Commit 4efaa5ac authored by Linus Torvalds's avatar Linus Torvalds

epoll: be better about file lifetimes

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@syzkaller.appspotmail.com
Reviewed-by: default avatarJens Axboe <axboe@kernel.dk>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent f462ae0e
...@@ -979,6 +979,34 @@ static __poll_t __ep_eventpoll_poll(struct file *file, poll_table *wait, int dep ...@@ -979,6 +979,34 @@ static __poll_t __ep_eventpoll_poll(struct file *file, poll_table *wait, int dep
return res; return res;
} }
/*
* 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.
*
* 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).
*
* 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).
*/
static struct file *epi_fget(const struct epitem *epi)
{
struct file *file;
file = epi->ffd.file;
if (!atomic_long_inc_not_zero(&file->f_count))
file = NULL;
return file;
}
/* /*
* Differs from ep_eventpoll_poll() in that internal callers already have * Differs from ep_eventpoll_poll() in that internal callers already have
* the ep->mtx so we need to start from depth=1, such that mutex_lock_nested() * the ep->mtx so we need to start from depth=1, such that mutex_lock_nested()
...@@ -987,14 +1015,22 @@ static __poll_t __ep_eventpoll_poll(struct file *file, poll_table *wait, int dep ...@@ -987,14 +1015,22 @@ static __poll_t __ep_eventpoll_poll(struct file *file, poll_table *wait, int dep
static __poll_t ep_item_poll(const struct epitem *epi, poll_table *pt, static __poll_t ep_item_poll(const struct epitem *epi, poll_table *pt,
int depth) int depth)
{ {
struct file *file = epi->ffd.file; struct file *file = epi_fget(epi);
__poll_t res; __poll_t res;
/*
* We could return EPOLLERR | EPOLLHUP or something, but let's
* treat this more as "file doesn't exist, poll didn't happen".
*/
if (!file)
return 0;
pt->_key = epi->event.events; pt->_key = epi->event.events;
if (!is_file_epoll(file)) if (!is_file_epoll(file))
res = vfs_poll(file, pt); res = vfs_poll(file, pt);
else else
res = __ep_eventpoll_poll(file, pt, depth); res = __ep_eventpoll_poll(file, pt, depth);
fput(file);
return res & epi->event.events; return res & epi->event.events;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment