Re: get_file() unsafe under epoll (was Re: [syzbot] [fs?] [io-uring?] general protection fault in __ep_remove)

From: Kees Cook
Date: Fri May 03 2024 - 16:28:52 EST


On Fri, May 03, 2024 at 12:59:52PM -0700, Kees Cook wrote:
> So, yeah, I can't figure out how eventpoll_release() and epoll_wait()
> are expected to behave safely for .poll handlers.
>
> Regardless, for the simple case: it seems like it's just totally illegal
> to use get_file() in a poll handler. Is this known/expected? And if so,
> how can dmabuf possibly deal with that?

Is this the right approach? It still feels to me like get_file() needs
to happen much earlier...

diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 882b89edc52a..c6c29facf228 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -991,9 +991,13 @@ static __poll_t ep_item_poll(const struct epitem *epi, poll_table *pt,
__poll_t res;

pt->_key = epi->event.events;
- if (!is_file_epoll(file))
- res = vfs_poll(file, pt);
- else
+ if (!is_file_epoll(file)) {
+ if (atomic_long_inc_not_zero(&file->f_count)) {
+ res = vfs_poll(file, pt);
+ fput(file);
+ } else
+ res = EPOLLERR;
+ } else
res = __ep_eventpoll_poll(file, pt, depth);
return res & epi->event.events;
}

--
Kees Cook