Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
From:   Haimin Zhang <tcs.kernel@gmail.com>
To:     Alexander Viro <viro@zeniv.linux.org.uk>,
        David Howells <dhowells@redhat.com>,
        "Fabio M. De Francesco" <fmdefrancesco@gmail.com>,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Cc:     Haimin Zhang <tcs_kernel@tencent.com>,
        TCS Robot <tcs_robot@tencent.com>
Subject: [PATCH] fs/pipe: Deinitialize the watch_queue when pipe is freed
Date:   Sat,  7 May 2022 19:56:05 +0800
Message-Id: <20220507115605.96775-1-tcs.kernel@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

From: Haimin Zhang <tcs_kernel@tencent.com>

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.

Reported-by: TCS Robot <tcs_robot@tencent.com>
Signed-off-by: Haimin Zhang <tcs_kernel@tencent.com>
---
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        | 16 ++++++++++++++++
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index e140ea150bbb..7e3f4df87c28 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) {
 		put_watch_queue(pipe->watch_queue);
+		watch_queue_deinit(pipe);
+	}
 #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..3396e60f14e8 100644
--- a/kernel/watch_queue.c
+++ b/kernel/watch_queue.c
@@ -663,3 +663,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