Commit a793183c authored by Eric Dumazet's avatar Eric Dumazet Committed by David S. Miller

udp: fix data-race in udp_set_dev_scratch()

KCSAN reported a data-race in udp_set_dev_scratch() [1]

The issue here is that we must not write over skb fields
if skb is shared. A similar issue has been fixed in commit
89c22d8c ("net: Fix skb csum races when peeking")

While we are at it, use a helper only dealing with
udp_skb_scratch(skb)->csum_unnecessary, as this allows
udp_set_dev_scratch() to be called once and thus inlined.

[1]
BUG: KCSAN: data-race in udp_set_dev_scratch / udpv6_recvmsg

write to 0xffff888120278317 of 1 bytes by task 10411 on cpu 1:
 udp_set_dev_scratch+0xea/0x200 net/ipv4/udp.c:1308
 __first_packet_length+0x147/0x420 net/ipv4/udp.c:1556
 first_packet_length+0x68/0x2a0 net/ipv4/udp.c:1579
 udp_poll+0xea/0x110 net/ipv4/udp.c:2720
 sock_poll+0xed/0x250 net/socket.c:1256
 vfs_poll include/linux/poll.h:90 [inline]
 do_select+0x7d0/0x1020 fs/select.c:534
 core_sys_select+0x381/0x550 fs/select.c:677
 do_pselect.constprop.0+0x11d/0x160 fs/select.c:759
 __do_sys_pselect6 fs/select.c:784 [inline]
 __se_sys_pselect6 fs/select.c:769 [inline]
 __x64_sys_pselect6+0x12e/0x170 fs/select.c:769
 do_syscall_64+0xcc/0x370 arch/x86/entry/common.c:290
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

read to 0xffff888120278317 of 1 bytes by task 10413 on cpu 0:
 udp_skb_csum_unnecessary include/net/udp.h:358 [inline]
 udpv6_recvmsg+0x43e/0xe90 net/ipv6/udp.c:310
 inet6_recvmsg+0xbb/0x240 net/ipv6/af_inet6.c:592
 sock_recvmsg_nosec+0x5c/0x70 net/socket.c:871
 ___sys_recvmsg+0x1a0/0x3e0 net/socket.c:2480
 do_recvmmsg+0x19a/0x5c0 net/socket.c:2601
 __sys_recvmmsg+0x1ef/0x200 net/socket.c:2680
 __do_sys_recvmmsg net/socket.c:2703 [inline]
 __se_sys_recvmmsg net/socket.c:2696 [inline]
 __x64_sys_recvmmsg+0x89/0xb0 net/socket.c:2696
 do_syscall_64+0xcc/0x370 arch/x86/entry/common.c:290
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Reported by Kernel Concurrency Sanitizer on:
CPU: 0 PID: 10413 Comm: syz-executor.0 Not tainted 5.4.0-rc3+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011

Fixes: 2276f58a ("udp: use a separate rx queue for packet reception")
Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
Cc: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: default avatarPaolo Abeni <pabeni@redhat.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 7de4344f
...@@ -1316,6 +1316,20 @@ static void udp_set_dev_scratch(struct sk_buff *skb) ...@@ -1316,6 +1316,20 @@ static void udp_set_dev_scratch(struct sk_buff *skb)
scratch->_tsize_state |= UDP_SKB_IS_STATELESS; scratch->_tsize_state |= UDP_SKB_IS_STATELESS;
} }
static void udp_skb_csum_unnecessary_set(struct sk_buff *skb)
{
/* We come here after udp_lib_checksum_complete() returned 0.
* This means that __skb_checksum_complete() might have
* set skb->csum_valid to 1.
* On 64bit platforms, we can set csum_unnecessary
* to true, but only if the skb is not shared.
*/
#if BITS_PER_LONG == 64
if (!skb_shared(skb))
udp_skb_scratch(skb)->csum_unnecessary = true;
#endif
}
static int udp_skb_truesize(struct sk_buff *skb) static int udp_skb_truesize(struct sk_buff *skb)
{ {
return udp_skb_scratch(skb)->_tsize_state & ~UDP_SKB_IS_STATELESS; return udp_skb_scratch(skb)->_tsize_state & ~UDP_SKB_IS_STATELESS;
...@@ -1550,10 +1564,7 @@ static struct sk_buff *__first_packet_length(struct sock *sk, ...@@ -1550,10 +1564,7 @@ static struct sk_buff *__first_packet_length(struct sock *sk,
*total += skb->truesize; *total += skb->truesize;
kfree_skb(skb); kfree_skb(skb);
} else { } else {
/* the csum related bits could be changed, refresh udp_skb_csum_unnecessary_set(skb);
* the scratch area
*/
udp_set_dev_scratch(skb);
break; break;
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment