lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Date:   Tue,  8 Mar 2022 02:58:20 +0000
From:   cgel.zte@...il.com
To:     viro@...iv.linux.org.uk
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        lv.ruyi@....com.cn, zealci@....com.cn
Subject: [PATCH] pipe: fix data race in pipe_poll / pipe_release

From: Lv Ruyi (CGEL ZTE) <lv.ruyi@....com.cn>

pipe_poll is reading pipe->writers without any synchronization while
pipe_release can write it with holding pipe lock. It is same to
pipe->readers.

Reading pipe state only, no need for acquiring the semaphore. Use
READ_ONCE to document them.

BUG: KCSAN: data-race in pipe_poll / pipe_release
write to 0xff1100003f332de8 of 4 bytes by task 31893 on cpu 0:
 pipe_release+0xc3/0x1f0 fs/pipe.c:721
 __fput+0x29b/0x530 fs/file_table.c:280
 ____fput+0x11/0x20 fs/file_table.c:313
 task_work_run+0x94/0x120 kernel/task_work.c:164
 exit_task_work include/linux/task_work.h:32 [inline]
 do_exit+0x675/0x1720 kernel/exit.c:832
 do_group_exit+0xa4/0x180 kernel/exit.c:929
 __do_sys_exit_group+0xb/0x10 kernel/exit.c:940
 __se_sys_exit_group+0x5/0x10 kernel/exit.c:938
 __x64_sys_exit_group+0x16/0x20 kernel/exit.c:938
 do_syscall_x64 arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x4a/0xb0 arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x44/0xae
read to 0xff1100003f332de8 of 4 bytes by task 31891 on cpu 3:
 pipe_poll+0x18b/0x270 fs/pipe.c:679
 vfs_poll include/linux/poll.h:90 [inline]
 do_select+0x7bb/0xec0 fs/select.c:537
 core_sys_select+0x42a/0x6a0 fs/select.c:680
 kern_select fs/select.c:721 [inline]
 __do_sys_select fs/select.c:728 [inline]
 __se_sys_select+0x1a7/0x1e0 fs/select.c:725
 __x64_sys_select+0x63/0x70 fs/select.c:725
 do_syscall_x64 arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x4a/0xb0 arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x44/0xae
value changed: 0x00000001 -> 0x00000000
Reported by Kernel Concurrency Sanitizer on:
CPU: 3 PID: 31891 Comm: sshd Not tainted 5.16.10 #3
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.0-0-ga698c8995f-prebuilt.qemu.org 04/01/2014
==================================================================

kernel version : linux_5.16.10/linux-stable

(gdb) l *pipe_release+0xc3
0xffffffff8144f2d3 is in pipe_release (fs/pipe.c:721).
716
717             __pipe_lock(pipe);
718             if (file->f_mode & FMODE_READ)
719                     pipe->readers--;
720             if (file->f_mode & FMODE_WRITE)
721                     pipe->writers--;
722
723             /* Was that the last reader or writer, but not the other side? */
724             if (!pipe->readers != !pipe->writers) {
725                     wake_up_interruptible_all(&pipe->rd_wait);
(gdb) l *pipe_poll+0x18b
0xffffffff8144e84b is in pipe_poll (fs/pipe.c:679).
674
675             mask = 0;
676             if (filp->f_mode & FMODE_READ) {
677                     if (!pipe_empty(head, tail))
678                             mask |= EPOLLIN | EPOLLRDNORM;
679                     if (!pipe->writers && filp->f_version != pipe->w_counter)
680                             mask |= EPOLLHUP;
681             }
682
683             if (filp->f_mode & FMODE_WRITE) {

Reported-by: Zeal Robot <zealci@....com.cn>
Signed-off-by: Lv Ruyi (CGEL ZTE) <lv.ruyi@....com.cn>
---
 fs/pipe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 71946832e33f..ddc8050f97ca 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -677,7 +677,7 @@ pipe_poll(struct file *filp, poll_table *wait)
 	if (filp->f_mode & FMODE_READ) {
 		if (!pipe_empty(head, tail))
 			mask |= EPOLLIN | EPOLLRDNORM;
-		if (!pipe->writers && filp->f_version != pipe->w_counter)
+		if (!READ_ONCE(pipe->writers) && filp->f_version != pipe->w_counter)
 			mask |= EPOLLHUP;
 	}
 
@@ -688,7 +688,7 @@ pipe_poll(struct file *filp, poll_table *wait)
 		 * Most Unices do not set EPOLLERR for FIFOs but on Linux they
 		 * behave exactly like pipes for poll().
 		 */
-		if (!pipe->readers)
+		if (!READ_ONCE(pipe->readers))
 			mask |= EPOLLERR;
 	}
 
-- 
2.25.1

Powered by blists - more mailing lists