aboutsummaryrefslogtreecommitdiff
path: root/fs
diff options
context:
space:
mode:
authorLinus Torvalds2019-12-07 10:41:17 -0800
committerLinus Torvalds2019-12-07 10:41:17 -0800
commitad910e36da4ca3a1bd436989f632d062dda0c921 (patch)
tree602909bcdadd7b61ea2dbca372db6ec3a81bc3b8 /fs
parenteea2d5da29e396b6cc1fb35e36bcbf5f57731015 (diff)
pipe: fix poll/select race introduced by the pipe rework
The kernel wait queues have a basic rule to them: you add yourself to the wait-queue first, and then you check the things that you're going to wait on. That avoids the races with the event you're waiting for. The same goes for poll/select logic: the "poll_wait()" goes first, and then you check the things you're polling for. Of course, if you use locking, the ordering doesn't matter since the lock will serialize with anything that changes the state you're looking at. That's not the case here, though. So move the poll_wait() first in pipe_poll(), before you start looking at the pipe state. Fixes: 8cefc107ca54 ("pipe: Use head and tail pointers for the ring, not cursor and length") Cc: David Howells <dhowells@redhat.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'fs')
-rw-r--r--fs/pipe.c18
1 files changed, 15 insertions, 3 deletions
diff --git a/fs/pipe.c b/fs/pipe.c
index b901c8eefafd..3e8b11e3b764 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -574,12 +574,24 @@ pipe_poll(struct file *filp, poll_table *wait)
{
__poll_t mask;
struct pipe_inode_info *pipe = filp->private_data;
- unsigned int head = READ_ONCE(pipe->head);
- unsigned int tail = READ_ONCE(pipe->tail);
+ unsigned int head, tail;
+ /*
+ * Reading only -- no need for acquiring the semaphore.
+ *
+ * But because this is racy, the code has to add the
+ * entry to the poll table _first_ ..
+ */
poll_wait(filp, &pipe->wait, wait);
- /* Reading only -- no need for acquiring the semaphore. */
+ /*
+ * .. and only then can you do the racy tests. That way,
+ * if something changes and you got it wrong, the poll
+ * table entry will wake you up and fix it.
+ */
+ head = READ_ONCE(pipe->head);
+ tail = READ_ONCE(pipe->tail);
+
mask = 0;
if (filp->f_mode & FMODE_READ) {
if (!pipe_empty(head, tail))