Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Qian Cai <cai@lca.pw>
To:     akpm@linux-foundation.org
Cc:     oleg@redhat.com, catalin.marinas@arm.com, elver@google.com,
        linux-kernel@vger.kernel.org, Qian Cai <cai@lca.pw>
Subject: [PATCH -next] signal: annotate data races in sys_rt_sigaction
Date:   Tue,  3 Mar 2020 12:20:49 -0500
Message-Id: <1583256049-15497-1-git-send-email-cai@lca.pw>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@lca.pw>
---
 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