| Message ID | 20230403214643.768555-1-edumazet@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a1865f2e7d10dde00d35a2122b38d2e469ae67ed |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] netlink: annotate lockless accesses to nlk->max_recvmsg_len | expand |
On Mon, Apr 03, 2023 at 09:46:43PM +0000, Eric Dumazet wrote: > syzbot reported a data-race in data-race in netlink_recvmsg() [1] > > Indeed, netlink_recvmsg() can be run concurrently, > and netlink_dump() also needs protection. > > [1] > BUG: KCSAN: data-race in netlink_recvmsg / netlink_recvmsg > > read to 0xffff888141840b38 of 8 bytes by task 23057 on cpu 0: > netlink_recvmsg+0xea/0x730 net/netlink/af_netlink.c:1988 > sock_recvmsg_nosec net/socket.c:1017 [inline] > sock_recvmsg net/socket.c:1038 [inline] > __sys_recvfrom+0x1ee/0x2e0 net/socket.c:2194 > __do_sys_recvfrom net/socket.c:2212 [inline] > __se_sys_recvfrom net/socket.c:2208 [inline] > __x64_sys_recvfrom+0x78/0x90 net/socket.c:2208 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > write to 0xffff888141840b38 of 8 bytes by task 23037 on cpu 1: > netlink_recvmsg+0x114/0x730 net/netlink/af_netlink.c:1989 > sock_recvmsg_nosec net/socket.c:1017 [inline] > sock_recvmsg net/socket.c:1038 [inline] > ____sys_recvmsg+0x156/0x310 net/socket.c:2720 > ___sys_recvmsg net/socket.c:2762 [inline] > do_recvmmsg+0x2e5/0x710 net/socket.c:2856 > __sys_recvmmsg net/socket.c:2935 [inline] > __do_sys_recvmmsg net/socket.c:2958 [inline] > __se_sys_recvmmsg net/socket.c:2951 [inline] > __x64_sys_recvmmsg+0xe2/0x160 net/socket.c:2951 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > value changed: 0x0000000000000000 -> 0x0000000000001000 > > Reported by Kernel Concurrency Sanitizer on: > CPU: 1 PID: 23037 Comm: syz-executor.2 Not tainted 6.3.0-rc4-syzkaller-00195-g5a57b48fdfcb #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/02/2023 > > Fixes: 9063e21fb026 ("netlink: autosize skb lengthes") > Reported-by: syzbot <syzkaller@googlegroups.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> Reviewed-by: Simon Horman <simon.horman@corigine.com>
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Mon, 3 Apr 2023 21:46:43 +0000 you wrote: > syzbot reported a data-race in data-race in netlink_recvmsg() [1] > > Indeed, netlink_recvmsg() can be run concurrently, > and netlink_dump() also needs protection. > > [1] > BUG: KCSAN: data-race in netlink_recvmsg / netlink_recvmsg > > [...] Here is the summary with links: - [net] netlink: annotate lockless accesses to nlk->max_recvmsg_len https://git.kernel.org/netdev/net/c/a1865f2e7d10 You are awesome, thank you!
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index c6427765975318b4c7fe3d5291dc4d69988f5249..f365dfdd672d7576b02bd5ae2871cf317e879248 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -1952,7 +1952,7 @@ static int netlink_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, struct scm_cookie scm; struct sock *sk = sock->sk; struct netlink_sock *nlk = nlk_sk(sk); - size_t copied; + size_t copied, max_recvmsg_len; struct sk_buff *skb, *data_skb; int err, ret; @@ -1985,9 +1985,10 @@ static int netlink_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, #endif /* Record the max length of recvmsg() calls for future allocations */ - nlk->max_recvmsg_len = max(nlk->max_recvmsg_len, len); - nlk->max_recvmsg_len = min_t(size_t, nlk->max_recvmsg_len, - SKB_WITH_OVERHEAD(32768)); + max_recvmsg_len = max(READ_ONCE(nlk->max_recvmsg_len), len); + max_recvmsg_len = min_t(size_t, max_recvmsg_len, + SKB_WITH_OVERHEAD(32768)); + WRITE_ONCE(nlk->max_recvmsg_len, max_recvmsg_len); copied = data_skb->len; if (len < copied) { @@ -2236,6 +2237,7 @@ static int netlink_dump(struct sock *sk) struct netlink_ext_ack extack = {}; struct netlink_callback *cb; struct sk_buff *skb = NULL; + size_t max_recvmsg_len; struct module *module; int err = -ENOBUFS; int alloc_min_size; @@ -2258,8 +2260,9 @@ static int netlink_dump(struct sock *sk) cb = &nlk->cb; alloc_min_size = max_t(int, cb->min_dump_alloc, NLMSG_GOODSIZE); - if (alloc_min_size < nlk->max_recvmsg_len) { - alloc_size = nlk->max_recvmsg_len; + max_recvmsg_len = READ_ONCE(nlk->max_recvmsg_len); + if (alloc_min_size < max_recvmsg_len) { + alloc_size = max_recvmsg_len; skb = alloc_skb(alloc_size, (GFP_KERNEL & ~__GFP_DIRECT_RECLAIM) | __GFP_NOWARN | __GFP_NORETRY);
syzbot reported a data-race in data-race in netlink_recvmsg() [1] Indeed, netlink_recvmsg() can be run concurrently, and netlink_dump() also needs protection. [1] BUG: KCSAN: data-race in netlink_recvmsg / netlink_recvmsg read to 0xffff888141840b38 of 8 bytes by task 23057 on cpu 0: netlink_recvmsg+0xea/0x730 net/netlink/af_netlink.c:1988 sock_recvmsg_nosec net/socket.c:1017 [inline] sock_recvmsg net/socket.c:1038 [inline] __sys_recvfrom+0x1ee/0x2e0 net/socket.c:2194 __do_sys_recvfrom net/socket.c:2212 [inline] __se_sys_recvfrom net/socket.c:2208 [inline] __x64_sys_recvfrom+0x78/0x90 net/socket.c:2208 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x63/0xcd write to 0xffff888141840b38 of 8 bytes by task 23037 on cpu 1: netlink_recvmsg+0x114/0x730 net/netlink/af_netlink.c:1989 sock_recvmsg_nosec net/socket.c:1017 [inline] sock_recvmsg net/socket.c:1038 [inline] ____sys_recvmsg+0x156/0x310 net/socket.c:2720 ___sys_recvmsg net/socket.c:2762 [inline] do_recvmmsg+0x2e5/0x710 net/socket.c:2856 __sys_recvmmsg net/socket.c:2935 [inline] __do_sys_recvmmsg net/socket.c:2958 [inline] __se_sys_recvmmsg net/socket.c:2951 [inline] __x64_sys_recvmmsg+0xe2/0x160 net/socket.c:2951 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x63/0xcd value changed: 0x0000000000000000 -> 0x0000000000001000 Reported by Kernel Concurrency Sanitizer on: CPU: 1 PID: 23037 Comm: syz-executor.2 Not tainted 6.3.0-rc4-syzkaller-00195-g5a57b48fdfcb #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/02/2023 Fixes: 9063e21fb026 ("netlink: autosize skb lengthes") Reported-by: syzbot <syzkaller@googlegroups.com> Signed-off-by: Eric Dumazet <edumazet@google.com> --- net/netlink/af_netlink.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)