linux-kernel - Re: [PATCH 1/8] epoll: check for events when removing a timed out thread from the wait queue

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:   Mon, 9 Nov 2020 22:05:03 -0800
From:   Davidlohr Bueso <dave@...olabs.net>
To:     Soheil Hassas Yeganeh <soheil.kdev@...il.com>
Cc:     torvalds@...ux-foundation.org, viro@...iv.linux.org.uk,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        akpm@...ux-foundation.org, edumazet@...gle.com, willemb@...gle.com,
        khazhy@...gle.com, guantaol@...gle.com,
        Soheil Hassas Yeganeh <soheil@...gle.com>
Subject: Re: [PATCH 1/8] epoll: check for events when removing a timed out
 thread from the wait queue

On Fri, 06 Nov 2020, Soheil Hassas Yeganeh wrote:

>From: Soheil Hassas Yeganeh <soheil@...gle.com>
>
>After abc610e01c66 ("fs/epoll: avoid barrier after an epoll_wait(2)
>timeout"), we break out of the ep_poll loop upon timeout, without checking
>whether there is any new events available.  Prior to that patch-series we
>always called ep_events_available() after exiting the loop.
>
>This can cause races and missed wakeups.  For example, consider the
>following scenario reported by Guantao Liu:
>
>Suppose we have an eventfd added using EPOLLET to an epollfd.
>
>Thread 1: Sleeps for just below 5ms and then writes to an eventfd.
>Thread 2: Calls epoll_wait with a timeout of 5 ms. If it sees an
>          event of the eventfd, it will write back on that fd.
>Thread 3: Calls epoll_wait with a negative timeout.
>
>Prior to abc610e01c66, it is guaranteed that Thread 3 will wake up either
>by Thread 1 or Thread 2.  After abc610e01c66, Thread 3 can be blocked
>indefinitely if Thread 2 sees a timeout right before the write to the
>eventfd by Thread 1.  Thread 2 will be woken up from
>schedule_hrtimeout_range and, with evail 0, it will not call
>ep_send_events().
>
>To fix this issue:
>1) Simplify the timed_out case as suggested by Linus.
>2) while holding the lock, recheck whether the thread was woken up
>   after its time out has reached.
>
>Note that (2) is different from Linus' original suggestion: It do not
>set "eavail = ep_events_available(ep)" to avoid unnecessary contention
>(when there are too many timed-out threads and a small number of events),
>as well as races mentioned in the discussion thread.
>
>This is the first patch in the series so that the backport to stable
>releases is straightforward.
>
>Link: https://lkml.kernel.org/r/CAHk-=wizk=OxUyQPbO8MS41w2Pag1kniUV5WdD5qWL-gq1kjDA@mail.gmail.com
>Fixes: abc610e01c66 ("fs/epoll: avoid barrier after an epoll_wait(2) timeout")
>Tested-by: Guantao Liu <guantaol@...gle.com>
>Suggested-by: Linus Torvalds <torvalds@...ux-foundation.org>
>Signed-off-by: Soheil Hassas Yeganeh <soheil@...gle.com>
>Reported-by: Guantao Liu <guantaol@...gle.com>
>Reviewed-by: Eric Dumazet <edumazet@...gle.com>
>Reviewed-by: Willem de Bruijn <willemb@...gle.com>
>Reviewed-by: Khazhismel Kumykov <khazhy@...gle.com>

Thanks for providing the fix and a testcase.

Reviewed-by: Davidlohr Bueso <dbueso@...e.de>