diff mbox series

[net] net: annotate data race around sk_ll_usec

Message ID 20210629141245.1278533-1-eric.dumazet@gmail.com (mailing list archive)
State Accepted
Commit 0dbffbb5335a1e3aa6855e4ee317e25e669dd302
Delegated to: Netdev Maintainers
Headers show
Series [net] net: annotate data race around sk_ll_usec | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present fail Series targets non-next tree, but doesn't contain any Fixes tags
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net
netdev/subject_prefix success Link
netdev/cc_maintainers warning 7 maintainers not CCed: linmiaohe@huawei.com xiangxia.m.yue@gmail.com keescook@chromium.org bjorn@kernel.org pabeni@redhat.com aahringo@redhat.com mathew.j.martineau@linux.intel.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 85 this patch: 85
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: Possible repeated word: 'Google'
netdev/build_allmodconfig_warn success Errors and warnings before: 85 this patch: 85
netdev/header_inline success Link

Commit Message

Eric Dumazet June 29, 2021, 2:12 p.m. UTC 
From: Eric Dumazet <edumazet@google.com>

sk_ll_usec is read locklessly from sk_can_busy_loop()
while another thread can change its value in sock_setsockopt()

This is correct but needs annotations.

BUG: KCSAN: data-race in __skb_try_recv_datagram / sock_setsockopt

write to 0xffff88814eb5f904 of 4 bytes by task 14011 on cpu 0:
 sock_setsockopt+0x1287/0x2090 net/core/sock.c:1175
 __sys_setsockopt+0x14f/0x200 net/socket.c:2100
 __do_sys_setsockopt net/socket.c:2115 [inline]
 __se_sys_setsockopt net/socket.c:2112 [inline]
 __x64_sys_setsockopt+0x62/0x70 net/socket.c:2112
 do_syscall_64+0x4a/0x90 arch/x86/entry/common.c:47
 entry_SYSCALL_64_after_hwframe+0x44/0xae

read to 0xffff88814eb5f904 of 4 bytes by task 14001 on cpu 1:
 sk_can_busy_loop include/net/busy_poll.h:41 [inline]
 __skb_try_recv_datagram+0x14f/0x320 net/core/datagram.c:273
 unix_dgram_recvmsg+0x14c/0x870 net/unix/af_unix.c:2101
 unix_seqpacket_recvmsg+0x5a/0x70 net/unix/af_unix.c:2067
 ____sys_recvmsg+0x15d/0x310 include/linux/uio.h:244
 ___sys_recvmsg net/socket.c:2598 [inline]
 do_recvmmsg+0x35c/0x9f0 net/socket.c:2692
 __sys_recvmmsg net/socket.c:2771 [inline]
 __do_sys_recvmmsg net/socket.c:2794 [inline]
 __se_sys_recvmmsg net/socket.c:2787 [inline]
 __x64_sys_recvmmsg+0xcf/0x150 net/socket.c:2787
 do_syscall_64+0x4a/0x90 arch/x86/entry/common.c:47
 entry_SYSCALL_64_after_hwframe+0x44/0xae

value changed: 0x00000000 -> 0x00000101

Reported by Kernel Concurrency Sanitizer on:
CPU: 1 PID: 14001 Comm: syz-executor.3 Not tainted 5.13.0-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
---
 include/net/busy_poll.h | 2 +-
 net/core/sock.c         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

patchwork-bot+netdevbpf@kernel.org July 1, 2021, 6:30 p.m. UTC | #1 
Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Tue, 29 Jun 2021 07:12:45 -0700 you wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> sk_ll_usec is read locklessly from sk_can_busy_loop()
> while another thread can change its value in sock_setsockopt()
> 
> This is correct but needs annotations.
> 
> [...]

Here is the summary with links:
  - [net] net: annotate data race around sk_ll_usec
    https://git.kernel.org/netdev/net/c/0dbffbb5335a

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
diff mbox series

Patch

diff --git a/include/net/busy_poll.h b/include/net/busy_poll.h
index 73af4a64a5999660127dab0a4d111e50eeadf1b5..40296ed976a9778ceb239b99ad783cb99b8b92ef 100644
--- a/include/net/busy_poll.h
+++ b/include/net/busy_poll.h
@@ -38,7 +38,7 @@  static inline bool net_busy_loop_on(void)
 
 static inline bool sk_can_busy_loop(const struct sock *sk)
 {
-	return sk->sk_ll_usec && !signal_pending(current);
+	return READ_ONCE(sk->sk_ll_usec) && !signal_pending(current);
 }
 
 bool sk_busy_loop_end(void *p, unsigned long start_time);
diff --git a/net/core/sock.c b/net/core/sock.c
index 946888afef880342cc08940e6bad1f295a985dd8..d27fdc2adf9c953fab3e25368150fb1412c7b249 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1172,7 +1172,7 @@  int sock_setsockopt(struct socket *sock, int level, int optname,
 			if (val < 0)
 				ret = -EINVAL;
 			else
-				sk->sk_ll_usec = val;
+				WRITE_ONCE(sk->sk_ll_usec, val);
 		}
 		break;
 	case SO_PREFER_BUSY_POLL: