Messages in this thread | | | Date | Tue, 24 Feb 2009 19:12:13 +0100 | From | Eric Dumazet <> | Subject | Re: [PATCH 1/6] [2.6.29] epoll: fix for epoll_wait sometimes returning events on closed fds |
| |
Tony Battersby a écrit : >>From the epoll manpage: > Q: Will closing a file descriptor cause it to be removed from all > epoll sets automatically? > A: Yes > > sys_close() calls filp_close(), which calls fput(). If no one else > holds a reference to the file, then fput() calls __fput(), and __fput() > calls eventpoll_release(), which prevents epoll_wait() from returning > events on the fd to userspace. In the rare case that sys_close() > doesn't call __fput() because someone else has a reference to the file, > a subsequent epoll_wait() may still unexpectedly return events on the > fd after it has been closed. This can end up confusing or crashing > a userspace program that doesn't do epoll_ctl(EPOLL_CTL_DEL) before > closing the fd. I have reports of this actually happening under > heavy load with a program using epoll with network sockets on 2.6.27. > > This patch fixes the problem by calling eventpoll_release_file() > from filp_close() instead of from __fput(). > > The locking in eventpoll_release() and eventpoll_release_file() needs > to be changed because previously it relied on the fact that no one > else could have a reference to the file when called from __fput(), > and this is no longer true. The new locking is admittedly ugly, > but I believe it works. > > ep_insert() now also needs to check if the file has been closed > to avoid races in multi-threaded programs where one thread is doing > epoll_ctl(EPOLL_CTL_ADD) and another thread is closing the fd. This is > done by checking that fget_light still returns the same struct file * > as before. > > Note that the list_del_init(&epi->fllink) previously done in > eventpoll_release_file() was unnecessary because it is also done > by ep_remove(). > > Userspace programs that might run on kernels with this bug can work > around the problem by doing epoll_ctl(EPOLL_CTL_DEL) before close(). > > Signed-off-by: Tony Battersby <tonyb@cybernetics.com> > CC: <stable@kernel.org>
Your patch may solve part of the problem. In your programs, maybe you have one thread doing all epoll_wait() and close() syscalls, but what of other programs ?
What prevents a thread doing close(fd) right after an other thread got this fd from epoll_wait() ? Nothing, and application may strangely react.
The moment you have several threads doing read()/write()/close() syscalls on the same fd at the same time, you obviously get problems, not only with epoll.
In a typical epoll driven application, with a pool of N worker threads all doing :
while (1) { fd = epoll_wait(epoll_fd); work_on_fd(fd); /* possibly calling close(fd); */ }
Then, you must be prepared to get a *false* event, ie an fd that another worker already closed (and eventually reopened)
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |