[PATCH -next] signal: annotate data races in sys_rt_sigaction

From: Qian Cai
Date: Tue Mar 03 2020 - 12:21:02 EST


Kmemleak could scan task stacks while plain writes happens to those
stack variables which could results in data races. For example, in
sys_rt_sigaction and do_sigaction(), it could have plain writes in
a 32-byte size. Since the kmemleak does not care about the actual values
of a non-pointer and all do_sigaction() call sites only copy to stack
variables, annotate them as intentional data races using the
data_race() macro. The data races were reported by KCSAN,

BUG: KCSAN: data-race in _copy_from_user / scan_block

read to 0xffffb3074e61fe58 of 8 bytes by task 356 on cpu 19:
scan_block+0x6e/0x1a0
scan_block at mm/kmemleak.c:1251
kmemleak_scan+0xbea/0xd20
kmemleak_scan at mm/kmemleak.c:1482
kmemleak_scan_thread+0xcc/0xfa
kthread+0x1cd/0x1f0
ret_from_fork+0x3a/0x50

write to 0xffffb3074e61fe58 of 32 bytes by task 30208 on cpu 2:
_copy_from_user+0xb2/0xe0
copy_user_generic at arch/x86/include/asm/uaccess_64.h:37
(inlined by) raw_copy_from_user at arch/x86/include/asm/uaccess_64.h:71
(inlined by) _copy_from_user at lib/usercopy.c:15
__x64_sys_rt_sigaction+0x83/0x140
__do_sys_rt_sigaction at kernel/signal.c:4245
(inlined by) __se_sys_rt_sigaction at kernel/signal.c:4233
(inlined by) __x64_sys_rt_sigaction at kernel/signal.c:4233
do_syscall_64+0x91/0xb05
entry_SYSCALL_64_after_hwframe+0x49/0xbe

Signed-off-by: Qian Cai <cai@xxxxxx>
---
kernel/signal.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 5b2396350dd1..bf39078c8be1 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3964,14 +3964,15 @@ int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)

spin_lock_irq(&p->sighand->siglock);
if (oact)
- *oact = *k;
+ /* Kmemleak could scan the task stack. */
+ data_race(*oact = *k);

sigaction_compat_abi(act, oact);

if (act) {
sigdelsetmask(&act->sa.sa_mask,
sigmask(SIGKILL) | sigmask(SIGSTOP));
- *k = *act;
+ data_race(*k = *act);
/*
* POSIX 3.3.1.3:
* "Setting a signal action to SIG_IGN for a signal that is
@@ -4242,7 +4243,9 @@ int __compat_save_altstack(compat_stack_t __user *uss, unsigned long sp)
if (sigsetsize != sizeof(sigset_t))
return -EINVAL;

- if (act && copy_from_user(&new_sa.sa, act, sizeof(new_sa.sa)))
+ if (act &&
+ /* Kmemleak could scan the task stack. */
+ data_race(copy_from_user(&new_sa.sa, act, sizeof(new_sa.sa))))
return -EFAULT;

ret = do_sigaction(sig, act ? &new_sa : NULL, oact ? &old_sa : NULL);
--
1.8.3.1