lkml.org 
[lkml]   [2009]   [Feb]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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/

    \
     
     \ /
      Last update: 2009-02-24 19:15    [W:5.048 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site