mirror of
https://github.com/torvalds/linux.git
synced 2026-07-28 10:09:10 +02:00
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 a6dc643c69 ("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>
Link: https://patch.msgid.link/20260424-work-epoll-rework-v1-4-249ed00a20f3@kernel.org
Signed-off-by: Christian Brauner <brauner@kernel.org>
This commit is contained in:
parent
14af80c325
commit
a573cb40f9
|
|
@ -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;
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user