[PATCH 04/17] eventpoll: refresh epi_fget() / ep_remove_file() comments

[Christian Brauner <brauner@kernel.org>]

Two comments drifted from the code they sit on.

epi_fget()'s block comment still referenced atomic_long_inc_not_zero,
which has been file_ref_get() for a while, and described only one of
the function's two roles: safe dereference of epi->ffd.file under
ep->mtx. Since commit a6dc643c6931 ("eventpoll: fix ep_remove struct
eventpoll / struct file UAF") the refcount bump also serves as a pin
that blocks __fput() from starting, which is what lets ep_remove()
touch file->f_lock and file->f_ep without racing
eventpoll_release_file(). Update the block to name both roles and the
commit that introduced the pin role.

ep_remove_file()'s one-line "See eventpoll_release() for details"
pointed at an inline in include/linux/eventpoll.h but said nothing
about what those details were. Replace it with a short explanation:
we publish NULL so the eventpoll_release() fastpath can skip the slow
path, and this is safe because every f_ep writer either holds a pin
via epi_fget() or is __fput() itself.

Comment-only; no functional change.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/eventpoll.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 1d1fd6464c38..1039d9737ce9 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -991,22 +991,23 @@ static void ep_free(struct eventpoll *ep)
 }
 
 /*
- * The ffd.file pointer may be in the process of being torn down due to
- * being closed, but we may not have finished eventpoll_release() yet.
+ * Pin @epi->ffd.file for operations that require both safe dereference
+ * and exclusion from __fput().
  *
- * Normally, even with the atomic_long_inc_not_zero, the file may have
- * been free'd and then gotten re-allocated to something else (since
- * files are not RCU-delayed, they are SLAB_TYPESAFE_BY_RCU).
+ * struct file uses SLAB_TYPESAFE_BY_RCU, so a freed slot can be
+ * reassigned at any time. The bare load of epi->ffd.file is safe here
+ * because the caller holds ep->mtx and eventpoll_release_file() blocks
+ * on that mutex while tearing down the epi, so the backing file
+ * allocation cannot be freed and reused under us. An rcu_read_lock()
+ * is therefore unnecessary for the load.
  *
- * But for epoll, users hold the ep->mtx mutex, and as such any file in
- * the process of being free'd will block in eventpoll_release_file()
- * and thus the underlying file allocation will not be free'd, and the
- * file re-use cannot happen.
- *
- * For the same reason we can avoid a rcu_read_lock() around the
- * operation - 'ffd.file' cannot go away even if the refcount has
- * reached zero (but we must still not call out to ->poll() functions
- * etc).
+ * A successful file_ref_get() additionally blocks __fput() from
+ * starting on this file: once the refcount has reached zero it cannot
+ * come back. ep_remove() relies on that to touch file->f_lock and
+ * file->f_ep without racing eventpoll_release_file() (see commit
+ * a6dc643c6931). A NULL return means __fput() is already in flight;
+ * the caller must bail without touching the file, and
+ * eventpoll_release_file() will clean the epi up from its side.
  */
 static struct file *epi_fget(const struct epitem *epi)
 {
@@ -1032,7 +1033,13 @@ static void ep_remove_file(struct eventpoll *ep, struct epitem *epi,
 	spin_lock(&file->f_lock);
 	head = file->f_ep;
 	if (hlist_is_singular_node(&epi->fllink, head)) {
-		/* See eventpoll_release() for details. */
+		/*
+		 * Last watcher: publish NULL so the eventpoll_release()
+		 * fastpath in include/linux/eventpoll.h can skip the slow
+		 * path on a future __fput(). Safe because every f_ep writer
+		 * either holds a pin on @file via epi_fget() or is __fput()
+		 * itself -- see the comment in eventpoll_release().
+		 */
 		WRITE_ONCE(file->f_ep, NULL);
 		if (!is_file_epoll(file)) {
 			struct epitems_head *v;

-- 
2.47.3