From: Christian Brauner Date: Fri, 24 Apr 2026 13:46:35 +0000 (+0200) Subject: eventpoll: refresh epi_fget() / ep_remove_file() comments X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=a573cb40f9819c3fe81a43eb10170f8fc8eddc5e;p=thirdparty%2Fkernel%2Flinux.git eventpoll: refresh epi_fget() / ep_remove_file() comments 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) Link: https://patch.msgid.link/20260424-work-epoll-rework-v1-4-249ed00a20f3@kernel.org Signed-off-by: Christian Brauner --- 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;