All of lore.kernel.org
 help / color / mirror / Atom feed
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 v2] fs/pipe: Deinitialize the watch_queue when pipe is freed
Date: Mon,  9 May 2022 21:17:26 +0800	[thread overview]
Message-ID: <20220509131726.59664-1-tcs.kernel@gmail.com> (raw)

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.
Check wqueu->defunct before pipe check since pipe becomes invalid once all
watch queues were cleared.

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        | 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


             reply	other threads:[~2022-05-09 13:18 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-09 13:17 Haimin Zhang [this message]
2022-05-09 20:50 ` [PATCH v2] fs/pipe: Deinitialize the watch_queue when pipe is freed Eric Biggers
2022-07-06  9:39   ` Lee Jones
2022-07-19 14:04     ` Lee Jones
2022-08-02 21:56       ` Eric Biggers
2022-08-09 15:38         ` Lee Jones
2022-08-09 18:06           ` Eric Biggers
2022-08-09 18:17             ` Lee Jones
2022-05-10  0:34 ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220509131726.59664-1-tcs.kernel@gmail.com \
    --to=tcs.kernel@gmail.com \
    --cc=dhowells@redhat.com \
    --cc=fmdefrancesco@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tcs_kernel@tencent.com \
    --cc=tcs_robot@tencent.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.