watch_queue: Fix lack of barrier/sync/lock between post and read
authorDavid Howells <dhowells@redhat.com>
Fri, 11 Mar 2022 13:24:36 +0000 (13:24 +0000)
committerLinus Torvalds <torvalds@linux-foundation.org>
Fri, 11 Mar 2022 18:17:13 +0000 (10:17 -0800)
There's nothing to synchronise post_one_notification() versus
pipe_read().  Whilst posting is done under pipe->rd_wait.lock, the
reader only takes pipe->mutex which cannot bar notification posting as
that may need to be made from contexts that cannot sleep.

Fix this by setting pipe->head with a barrier in post_one_notification()
and reading pipe->head with a barrier in pipe_read().

If that's not sufficient, the rd_wait.lock will need to be taken,
possibly in a ->confirm() op so that it only applies to notifications.
The lock would, however, have to be dropped before copy_page_to_iter()
is invoked.

Fixes: c73be61cede5 ("pipe: Add general notification queue support")
Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
fs/pipe.c
kernel/watch_queue.c

index 4eb88bc138bbc9d31ef294c21c995669972a965b..2667db9506e2f2853f46e551332562dec10618af 100644 (file)
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -253,7 +253,8 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
         */
        was_full = pipe_full(pipe->head, pipe->tail, pipe->max_usage);
        for (;;) {
-               unsigned int head = pipe->head;
+               /* Read ->head with a barrier vs post_one_notification() */
+               unsigned int head = smp_load_acquire(&pipe->head);
                unsigned int tail = pipe->tail;
                unsigned int mask = pipe->ring_size - 1;
 
index c12267ccc70e1f30982d4fd8977b7118dfb25761..37bcd900fd77d9025ea9b602a95a10f6c1aaa33f 100644 (file)
@@ -113,7 +113,7 @@ static bool post_one_notification(struct watch_queue *wqueue,
        buf->offset = offset;
        buf->len = len;
        buf->flags = PIPE_BUF_FLAG_WHOLE;
-       pipe->head = head + 1;
+       smp_store_release(&pipe->head, head + 1); /* vs pipe_read() */
 
        if (!test_and_clear_bit(note, wqueue->notes_bitmap)) {
                spin_unlock_irq(&pipe->rd_wait.lock);