From: Haimin Zhang <tcs_kernel@xxxxxxxxxxx>
Add a new function call to deinitialize the watch_queue of a freed pipe.
When a pipe node is freed, it doesn't make pipe->watch_queue->pipe null.
Later when function post_one_notification is called, it will use this
field, but it has been freed and watch_queue->pipe is a dangling pointer.
It makes a uaf issue.
Check wqueu->defunct before pipe check since pipe becomes invalid once all
watch queues were cleared.
Reported-by: TCS Robot <tcs_robot@xxxxxxxxxxx>
Signed-off-by: Haimin Zhang <tcs_kernel@xxxxxxxxxxx>
---
The following is the callstacks:
1. The pipe was created as follows:
```
kmalloc build/../include/linux/slab.h:581 [inline]
kzalloc build/../include/linux/slab.h:714 [inline]
alloc_pipe_info+0x105/0x590 build/../fs/pipe.c:790
get_pipe_inode build/../fs/pipe.c:881 [inline]
create_pipe_files+0x8d/0x880 build/../fs/pipe.c:913
__do_pipe_flags build/../fs/pipe.c:962 [inline]
do_pipe2+0x96/0x1b0 build/../fs/pipe.c:1010
__do_sys_pipe2 build/../fs/pipe.c:1028 [inline]
__se_sys_pipe2 build/../fs/pipe.c:1026 [inline]
__x64_sys_pipe2+0x50/0x70 build/../fs/pipe.c:1026
do_syscall_x64 build/../arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0x80 build/../arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
```
2. The pipe was freed as follows:
```
kfree+0xd6/0x4d0 build/../mm/slub.c:4552
put_pipe_info build/../fs/pipe.c:711 [inline]
pipe_release+0x2b6/0x310 build/../fs/pipe.c:734
__fput+0x277/0x9d0 build/../fs/file_table.c:317
task_work_run+0xdd/0x1a0 build/../kernel/task_work.c:164
resume_user_mode_work build/../include/linux/resume_user_mode.h: 49 [inline]
exit_to_user_mode_loop build/../kernel/entry/common.c:169 [inline]
exit_to_user_mode_prepare+0x23c/0x250 build/../kernel/entry/common.c:201
__syscall_exit_to_user_mode_work build/../kernel/entry/common.c:283 [inline]
syscall_exit_to_user_mode+0x19/0x60 build/../kernel/entry/common.c:294
do_syscall_64+0x42/0x80 build/../arch/x86/entry/common.c:86
entry_SYSCALL_64_after_hwframe+0x44/0xae
```
3. The dangling pointer was used:
```
__lock_acquire+0x3eb0/0x56c0 build/../kernel/locking/lockdep.c:4899
lock_acquire build/../kernel/locking/lockdep.c:5641 [inline]
lock_acquire+0x1ab/0x510 build/../kernel/locking/lockdep.c:5606
__raw_spin_lock_irq build/../include/linux/spinlock_api_smp.h:119 [inline]
_raw_spin_lock_irq+0x32/0x50 build/../kernel/locking/spinlock.c:170
spin_lock_irq build/../include/linux/spinlock.h:374 [inline]
post_one_notification.isra.0+0x59/0x990 build/../kernel/watch_queue.c:86
remove_watch_from_object+0x35a/0x9d0 build/../kernel/watch_queue.c:527
remove_watch_list build/../include/linux/watch_queue.h:115 [inline]
key_gc_unused_keys.constprop.0+0x2e5/0x600 build/../security/keys/gc.c:135
key_garbage_collector+0x3d7/0x920 build/../security/keys/gc.c:297
process_one_work+0x996/0x1610 build/../kernel/workqueue.c:2289
worker_thread+0x665/0x1080 build/../kernel/workqueue.c:2436
kthread+0x2e9/0x3a0 build/../kernel/kthread.c:376
ret_from_fork+0x1f/0x30 build/../arch/x86/entry/entry_64.S:298
```
fs/pipe.c | 4 +++-
include/linux/watch_queue.h | 5 +++++
kernel/watch_queue.c | 34 ++++++++++++++++++++++++++++++----
3 files changed, 38 insertions(+), 5 deletions(-)
diff --git a/fs/pipe.c b/fs/pipe.c
index e140ea150bbb..d0e9d8697810 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -844,8 +844,10 @@ void free_pipe_info(struct pipe_inode_info *pipe)
pipe_buf_release(pipe, buf);
}
#ifdef CONFIG_WATCH_QUEUE
- if (pipe->watch_queue)
+ if (pipe->watch_queue) {
+ watch_queue_deinit(pipe);
put_watch_queue(pipe->watch_queue);
+ }
#endif
if (pipe->tmp_page)
__free_page(pipe->tmp_page);
diff --git a/include/linux/watch_queue.h b/include/linux/watch_queue.h
index 3b9a40ae8bdb..e5086b195fb7 100644
--- a/include/linux/watch_queue.h
+++ b/include/linux/watch_queue.h
@@ -90,6 +90,7 @@ extern long watch_queue_set_size(struct pipe_inode_info *, unsigned int);
extern long watch_queue_set_filter(struct pipe_inode_info *,
struct watch_notification_filter __user *);
extern int watch_queue_init(struct pipe_inode_info *);
+extern int watch_queue_deinit(struct pipe_inode_info *);
extern void watch_queue_clear(struct watch_queue *);
static inline void init_watch_list(struct watch_list *wlist,
@@ -129,6 +130,10 @@ static inline int watch_queue_init(struct pipe_inode_info *pipe)
return -ENOPKG;
}
+static inline int watch_queue_deinit(struct pipe_inode_info *pipe)
+{
+ return -ENOPKG;
+}
#endif
#endif /* _LINUX_WATCH_QUEUE_H */
diff --git a/kernel/watch_queue.c b/kernel/watch_queue.c
index 230038d4f908..4357511880f5 100644
--- a/kernel/watch_queue.c
+++ b/kernel/watch_queue.c
@@ -80,13 +80,22 @@ static bool post_one_notification(struct watch_queue *wqueue,
unsigned int head, tail, mask, note, offset, len;
bool done = false;
- if (!pipe)
+ if (!kref_get_unless_zero(&wqueue->usage))
return false;
- spin_lock_irq(&pipe->rd_wait.lock);
-
- if (wqueue->defunct)
+ spin_lock_bh(&wqueue->lock);
+ if (wqueue->defunct) {
+ spin_unlock_bh(&wqueue->lock);
goto out;
+ }
+ spin_unlock_bh(&wqueue->lock);
+
+ if (!pipe) {
+ put_watch_queue(wqueue);
+ return false;
+ }
+
+ spin_lock_irq(&pipe->rd_wait.lock);
mask = pipe->ring_size - 1;
head = pipe->head;
@@ -123,6 +132,7 @@ static bool post_one_notification(struct watch_queue *wqueue,
done = true;
out:
+ put_watch_queue(wqueue);
spin_unlock_irq(&pipe->rd_wait.lock);
if (done)
kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN);
@@ -663,3 +673,19 @@ int watch_queue_init(struct pipe_inode_info *pipe)
pipe->watch_queue = wqueue;
return 0;
}
+
+/*
+ * Deinitialise a watch queue
+ */
+int watch_queue_deinit(struct pipe_inode_info *pipe)
+{
+ struct watch_queue *wqueue;
+
+ if (pipe) {
+ wqueue = pipe->watch_queue;
+ if (wqueue)
+ wqueue->pipe = NULL;
+ pipe->watch_queue = NULL;
+ }
+ return 0;
+}
--
2.27.0
