linux-kernel - Re: [PATCH v2] epoll: be better about file lifetimes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sun, 5 May 2024 12:04:40 -0600
From: Jens Axboe <axboe@...nel.dk>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: brauner@...nel.org, christian.koenig@....com,
 dri-devel@...ts.freedesktop.org, io-uring@...r.kernel.org, jack@...e.cz,
 keescook@...omium.org, laura@...bott.name, linaro-mm-sig@...ts.linaro.org,
 linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-media@...r.kernel.org, minhquangbui99@...il.com,
 sumit.semwal@...aro.org,
 syzbot+045b454ab35fd82a35fb@...kaller.appspotmail.com,
 syzkaller-bugs@...glegroups.com, viro@...iv.linux.org.uk
Subject: Re: [PATCH v2] epoll: be better about file lifetimes

On 5/5/24 11:55 AM, Linus Torvalds wrote:
> epoll can call out to vfs_poll() with a file pointer that may race with
> the last 'fput()'. That would make f_count go down to zero, and while
> the ep->mtx locking means that the resulting file pointer tear-down will
> be blocked until the poll returns, it means that f_count is already
> dead, and any use of it won't actually get a reference to the file any
> more: it's dead regardless.
> 
> Make sure we have a valid ref on the file pointer before we call down to
> vfs_poll() from the epoll routines.
> 
> Link: https://lore.kernel.org/lkml/0000000000002d631f0615918f1e@google.com/
> Reported-by: syzbot+045b454ab35fd82a35fb@...kaller.appspotmail.com
> Reviewed-by: Jens Axboe <axboe@...nel.dk>
> Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>
> ---
> 
> Changes since v1:
> 
>  - add Link, Reported-by, and Jens' reviewed-by. And sign off on it
>    because it looks fine to me and we have some testing now.
> 
>  - move epi_fget() closer to the user
> 
>  - more comments about the background
> 
>  - remove the rcu_read_lock(), with the comment explaining why it's not
>    needed
> 
>  - note about returning zero rather than something like EPOLLERR|POLLHUP
>    for a file that is going away

I did look at that 0 return as well and agreed this is the right choice,
but adding the comment is a good idea.

Anyway, patch still looks fine to me. I'd word wrap the comment section
above epi_fget() wider, but that's just a stylistic choice...

-- 
Jens Axboe