| Message ID | 20240925-fungieren-anbauen-79b334b00542@brauner (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | epoll: annotate racy check | expand |
On Wed 25-09-24 11:05:16, Christian Brauner wrote: > Epoll relies on a racy fastpath check during __fput() in > eventpoll_release() to avoid the hit of pointlessly acquiring a > semaphore. Annotate that race by using WRITE_ONCE() and READ_ONCE(). > > Link: https://lore.kernel.org/r/66edfb3c.050a0220.3195df.001a.GAE@google.com > Reported-by: syzbot+3b6b32dc50537a49bb4a@syzkaller.appspotmail.com > Signed-off-by: Christian Brauner <brauner@kernel.org> > --- > fs/eventpoll.c | 3 ++- > include/linux/eventpoll.h | 2 +- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/fs/eventpoll.c b/fs/eventpoll.c > index f53ca4f7fced..fa766695f886 100644 > --- a/fs/eventpoll.c > +++ b/fs/eventpoll.c > @@ -823,7 +823,8 @@ static bool __ep_remove(struct eventpoll *ep, struct epitem *epi, bool force) > to_free = NULL; > head = file->f_ep; > if (head->first == &epi->fllink && !epi->fllink.next) { > - file->f_ep = NULL; > + /* See eventpoll_release() for details. */ > + WRITE_ONCE(file->f_ep, NULL); There's one more write to file->f_ep in attach_epitem() which needs WRITE_ONCE() as well to match the READ_ONCE() in other places. Otherwise feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > if (!is_file_epoll(file)) { > struct epitems_head *v; > v = container_of(head, struct epitems_head, epitems); > diff --git a/include/linux/eventpoll.h b/include/linux/eventpoll.h > index 3337745d81bd..0c0d00fcd131 100644 > --- a/include/linux/eventpoll.h > +++ b/include/linux/eventpoll.h > @@ -42,7 +42,7 @@ static inline void eventpoll_release(struct file *file) > * because the file in on the way to be removed and nobody ( but > * eventpoll ) has still a reference to this file. > */ > - if (likely(!file->f_ep)) > + if (likely(!READ_ONCE(file->f_ep))) > return; > > /* > -- > 2.45.2 >
On Wed, Sep 25, 2024 at 01:15:16PM GMT, Jan Kara wrote: > On Wed 25-09-24 11:05:16, Christian Brauner wrote: > > Epoll relies on a racy fastpath check during __fput() in > > eventpoll_release() to avoid the hit of pointlessly acquiring a > > semaphore. Annotate that race by using WRITE_ONCE() and READ_ONCE(). > > > > Link: https://lore.kernel.org/r/66edfb3c.050a0220.3195df.001a.GAE@google.com > > Reported-by: syzbot+3b6b32dc50537a49bb4a@syzkaller.appspotmail.com > > Signed-off-by: Christian Brauner <brauner@kernel.org> > > --- > > fs/eventpoll.c | 3 ++- > > include/linux/eventpoll.h | 2 +- > > 2 files changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/fs/eventpoll.c b/fs/eventpoll.c > > index f53ca4f7fced..fa766695f886 100644 > > --- a/fs/eventpoll.c > > +++ b/fs/eventpoll.c > > @@ -823,7 +823,8 @@ static bool __ep_remove(struct eventpoll *ep, struct epitem *epi, bool force) > > to_free = NULL; > > head = file->f_ep; > > if (head->first == &epi->fllink && !epi->fllink.next) { > > - file->f_ep = NULL; > > + /* See eventpoll_release() for details. */ > > + WRITE_ONCE(file->f_ep, NULL); > > There's one more write to file->f_ep in attach_epitem() which needs > WRITE_ONCE() as well to match the READ_ONCE() in other places. Otherwise Thanks, done! > feel free to add: > > Reviewed-by: Jan Kara <jack@suse.cz> Thanks!
diff --git a/fs/eventpoll.c b/fs/eventpoll.c index f53ca4f7fced..fa766695f886 100644 --- a/fs/eventpoll.c +++ b/fs/eventpoll.c @@ -823,7 +823,8 @@ static bool __ep_remove(struct eventpoll *ep, struct epitem *epi, bool force) to_free = NULL; head = file->f_ep; if (head->first == &epi->fllink && !epi->fllink.next) { - file->f_ep = NULL; + /* See eventpoll_release() for details. */ + WRITE_ONCE(file->f_ep, NULL); if (!is_file_epoll(file)) { struct epitems_head *v; v = container_of(head, struct epitems_head, epitems); diff --git a/include/linux/eventpoll.h b/include/linux/eventpoll.h index 3337745d81bd..0c0d00fcd131 100644 --- a/include/linux/eventpoll.h +++ b/include/linux/eventpoll.h @@ -42,7 +42,7 @@ static inline void eventpoll_release(struct file *file) * because the file in on the way to be removed and nobody ( but * eventpoll ) has still a reference to this file. */ - if (likely(!file->f_ep)) + if (likely(!READ_ONCE(file->f_ep))) return; /*
Epoll relies on a racy fastpath check during __fput() in eventpoll_release() to avoid the hit of pointlessly acquiring a semaphore. Annotate that race by using WRITE_ONCE() and READ_ONCE(). Link: https://lore.kernel.org/r/66edfb3c.050a0220.3195df.001a.GAE@google.com Reported-by: syzbot+3b6b32dc50537a49bb4a@syzkaller.appspotmail.com Signed-off-by: Christian Brauner <brauner@kernel.org> --- fs/eventpoll.c | 3 ++- include/linux/eventpoll.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-)