Message ID | 20240515073644.32503-2-yenchia.chen@mediatek.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | netlink, fix issues caught by syzbot | expand |
On Wed, May 15, 2024 at 03:36:37PM +0800, Yenchia Chen wrote: > From: Eric Dumazet <edumazet@google.com> > > 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> > Link: https://lore.kernel.org/r/20230403214643.768555-1-edumazet@google.com > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > Signed-off-by: yenchia.chen <yenchia.chen@mediatek.com> > --- The conflict resolution looks good to me, thanks! Reviewed-by: Carlos Llamas <cmllamas@google.com>
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 82df02695bbd..56ba8a6396ca 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -1935,7 +1935,7 @@ static int netlink_recvmsg(struct socket *sock, struct msghdr *msg, size_t len, struct sock *sk = sock->sk; struct netlink_sock *nlk = nlk_sk(sk); int noblock = flags & MSG_DONTWAIT; - size_t copied; + size_t copied, max_recvmsg_len; struct sk_buff *skb, *data_skb; int err, ret; @@ -1968,9 +1968,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) { @@ -2219,6 +2220,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; @@ -2241,8 +2243,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);