eventpoll: don't decrement ep refcount while still holding the ep mutex
commit 8c2e52eb upstream. Jann Horn points out that epoll is decrementing the ep refcount and then doing a mutex_unlock(&ep->mtx); afterwards. That's very wrong, because it can lead to a use-after-free. That pattern is actually fine for the very last reference, because the code in question will delay the actual call to "ep_free(ep)" until after it has unlocked the mutex. But it's wrong for the much subtler "next to last" case when somebody *else* may also be dropping their reference and free the ep while we're still using the mutex. Note that this is true even if that other user is also using the same ep mutex: mutexes, unlike spinlocks, can not be used for object ownership, even if they guarantee mutual exclusion. A mutex "unlock" operation is not atomic, and as one user is still accessing the mutex as part of unlocking it, another user can come in and get the now released mutex and free the data structure while the first user is still cleaning up. See our mutex documentation in Documentation/locking/mutex-design.rst, in particular the section [1] about semantics: "mutex_unlock() may access the mutex structure even after it has internally released the lock already - so it's not safe for another context to acquire the mutex and assume that the mutex_unlock() context is not using the structure anymore" So if we drop our ep ref before the mutex unlock, but we weren't the last one, we may then unlock the mutex, another user comes in, drops _their_ reference and releases the 'ep' as it now has no users - all while the mutex_unlock() is still accessing it. Fix this by simply moving the ep refcount dropping to outside the mutex: the refcount itself is atomic, and doesn't need mutex protection (that's the whole _point_ of refcounts: unlike mutexes, they are inherently about object lifetimes). Reported-by:Jann Horn <jannh@google.com> Link: https://docs.kernel.org/locking/mutex-design.html#semantics [1] Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Christian Brauner <brauner@kernel.org> Cc: Jan Kara <jack@suse.cz> Signed-off-by:
Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-
mentioned in commit 474f586e
-
mentioned in commit 22f24914
-
mentioned in commit 44dcb2c3
-
mentioned in commit 3ebc58e2
-
mentioned in commit d82a8064
-
mentioned in commit f4c242f1
-
mentioned in commit 2d6ad23a
-
mentioned in commit e8a47175
-
mentioned in commit c27443ae
-
mentioned in commit 5bfe92d3
-
mentioned in commit c1de4747
-
mentioned in commit 4fac1bd0
-
mentioned in commit 304dd4f5
-
mentioned in commit 42d78647
-
mentioned in commit fe5a8edd
-
mentioned in commit 217c4b8d
-
mentioned in commit 8090e846
-
mentioned in commit 339dbba0
-
mentioned in commit 2face7dd
-
mentioned in commit ecae4d46
-
mentioned in commit 5b8315ef
-
mentioned in commit e23a0f94
-
mentioned in commit baad00f6
-
mentioned in commit 901dfa6d
-
mentioned in commit da1160c4
-
mentioned in commit e526f790
-
mentioned in commit 26cf3bf1
-
mentioned in commit d12def49