2020-01-31 14:52:07

by Qian Cai

[permalink] [raw]
Subject: [PATCH -next] locking/osq_lock: mark an intentional data race

node->next could be accessed concurrently as reported by KCSAN,

BUG: KCSAN: data-race in osq_lock / osq_unlock

write (marked) to 0xffff8bb2f1abbe40 of 8 bytes by task 1138 on cpu 44:
osq_lock+0x149/0x340 kernel/locking/osq_lock.c:143
__mutex_lock+0x277/0xd20 kernel/locking/mutex.c:657
mutex_lock_nested+0x31/0x40
kernfs_iop_getattr+0x58/0x90
vfs_getattr_nosec+0x11a/0x170
vfs_statx_fd+0x54/0x90
__do_sys_newfstat+0x40/0x90
__x64_sys_newfstat+0x3a/0x50
do_syscall_64+0x91/0xb47
entry_SYSCALL_64_after_hwframe+0x49/0xbe

read to 0xffff8bb2f1abbe40 of 8 bytes by task 1150 on cpu 29:
osq_unlock+0xee/0x170 kernel/locking/osq_lock.c:78
__mutex_lock+0xb68/0xd20 kernel/locking/mutex.c:686
mutex_lock_nested+0x31/0x40
kernfs_iop_getattr+0x58/0x90
vfs_getattr_nosec+0x11a/0x170
vfs_statx_fd+0x54/0x90
__do_sys_newfstat+0x40/0x90
__x64_sys_newfstat+0x3a/0x50
do_syscall_64+0x91/0xb47
entry_SYSCALL_64_after_hwframe+0x49/0xbe

This is a false positive. Since even if that load is shattered the code
will function correctly -- it checks for any !0 value, any byte
composite that is !0 is sufficient. Hence, mark it as an intentional
data race using the data_race() macro.

Signed-off-by: Qian Cai <[email protected]>
---
kernel/locking/osq_lock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/locking/osq_lock.c b/kernel/locking/osq_lock.c
index 1f7734949ac8..009bf18c2226 100644
--- a/kernel/locking/osq_lock.c
+++ b/kernel/locking/osq_lock.c
@@ -75,7 +75,7 @@ osq_wait_next(struct optimistic_spin_queue *lock,
* wait for either @lock to point to us, through its Step-B, or
* wait for a new @node->next from its Step-C.
*/
- if (node->next) {
+ if (data_race(node->next)) {
next = xchg(&node->next, NULL);
if (next)
break;
--
2.21.0 (Apple Git-122.2)