Re: [PATCH v3] kernel/watch_queue: Make pipe NULL while clearing watch_queue
From: Siddh Raman Pant <hidden>
Date: 2022-08-01 09:35:19
Also in:
linux-kernel-mentees, lkml
Hello Dipanjan,
It would be nice if you could test this patch and tell if it fixes the
issue on v5.10, as you had reported it earlier.
Please apply the following commits before applying this patch:
db8facfc9faf ("watch_queue, pipe: Free watchqueue state after clearing pipe ring")
353f7988dd84 ("watchqueue: make sure to serialize 'wqueue->defunct' properly")
I have tested locally on tag v5.10, using the reproducer available on
syzkaller dashboard. The crash occurred when the patches weren't applied,
and it no longer occurs after applying the three patches.
Thanks,
Siddh
quoted hunk ↗ jump to hunk
If not done, a reference to a freed pipe remains in the watch_queue, as this function is called before freeing a pipe in free_pipe_info() (see line 834 of fs/pipe.c). This causes a UAF when post_one_notification() tries to access the pipe on a key update, which is reported by syzbot. We also need to use READ_ONCE() in post_one_notification() to prevent the compiler from optimising and loading a non-NULL value from wqueue->pipe. Bug report: https://syzkaller.appspot.com/bug?id=1870dd7791ba05f2ea7f47f7cbdde701173973fc Reported-and-tested-by: syzbot+c70d87ac1d001f29a058@syzkaller.appspotmail.com Signed-off-by: Siddh Raman Pant <redacted> --- Changes in v3: - Restore the original unlock order, and clear before unlock. - Use READ_ONCE() in post path. This was explained by David Howells [off-list ref] in reply to v1. Not added Suggested-by since he didn't reply yet. Changes in v2: - Removed the superfluous ifdef guard. kernel/watch_queue.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)diff --git a/kernel/watch_queue.c b/kernel/watch_queue.c index bb9962b33f95..617425e34252 100644 --- a/kernel/watch_queue.c +++ b/kernel/watch_queue.c@@ -99,7 +99,7 @@ static bool post_one_notification(struct watch_queue *wqueue, struct watch_notification *n) { void *p; - struct pipe_inode_info *pipe = wqueue->pipe; + struct pipe_inode_info *pipe = READ_ONCE(wqueue->pipe); struct pipe_buffer *buf; struct page *page; unsigned int head, tail, mask, note, offset, len;@@ -637,6 +637,12 @@ void watch_queue_clear(struct watch_queue *wqueue) spin_lock_bh(&wqueue->lock); } + /* Clearing the watch queue, so we should clean the associated pipe. */ + if (wqueue->pipe) { + wqueue->pipe->watch_queue = NULL; + wqueue->pipe = NULL; + } + spin_unlock_bh(&wqueue->lock); rcu_read_unlock(); }-- 2.35.1