Skip to content

Commit 4c971d2

Browse files
edumazetdavem330
authored andcommitted
net: annotate races around sk->sk_bound_dev_if
UDP sendmsg() is lockless, and reads sk->sk_bound_dev_if while this field can be changed by another thread. Adds minimal annotations to avoid KCSAN splats for UDP. Following patches will add more annotations to potential lockless readers. BUG: KCSAN: data-race in __ip6_datagram_connect / udpv6_sendmsg write to 0xffff888136d47a94 of 4 bytes by task 7681 on cpu 0: __ip6_datagram_connect+0x6e2/0x930 net/ipv6/datagram.c:221 ip6_datagram_connect+0x2a/0x40 net/ipv6/datagram.c:272 inet_dgram_connect+0x107/0x190 net/ipv4/af_inet.c:576 __sys_connect_file net/socket.c:1900 [inline] __sys_connect+0x197/0x1b0 net/socket.c:1917 __do_sys_connect net/socket.c:1927 [inline] __se_sys_connect net/socket.c:1924 [inline] __x64_sys_connect+0x3d/0x50 net/socket.c:1924 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x2b/0x50 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae read to 0xffff888136d47a94 of 4 bytes by task 7670 on cpu 1: udpv6_sendmsg+0xc60/0x16e0 net/ipv6/udp.c:1436 inet6_sendmsg+0x5f/0x80 net/ipv6/af_inet6.c:652 sock_sendmsg_nosec net/socket.c:705 [inline] sock_sendmsg net/socket.c:725 [inline] ____sys_sendmsg+0x39a/0x510 net/socket.c:2413 ___sys_sendmsg net/socket.c:2467 [inline] __sys_sendmmsg+0x267/0x4c0 net/socket.c:2553 __do_sys_sendmmsg net/socket.c:2582 [inline] __se_sys_sendmmsg net/socket.c:2579 [inline] __x64_sys_sendmmsg+0x53/0x60 net/socket.c:2579 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x2b/0x50 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae value changed: 0x00000000 -> 0xffffff9b Reported by Kernel Concurrency Sanitizer on: CPU: 1 PID: 7670 Comm: syz-executor.3 Tainted: G W 5.18.0-rc1-syzkaller-dirty #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 I chose to not add Fixes: tag because race has minor consequences and stable teams busy enough. Signed-off-by: Eric Dumazet <edumazet@google.com> Reported-by: syzbot <syzkaller@googlegroups.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 7fa2e48 commit 4c971d2

File tree

4 files changed

+13
-11
lines changed

4 files changed

+13
-11
lines changed

include/net/ip.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ static inline void ipcm_init_sk(struct ipcm_cookie *ipcm,
9393

9494
ipcm->sockc.mark = inet->sk.sk_mark;
9595
ipcm->sockc.tsflags = inet->sk.sk_tsflags;
96-
ipcm->oif = inet->sk.sk_bound_dev_if;
96+
ipcm->oif = READ_ONCE(inet->sk.sk_bound_dev_if);
9797
ipcm->addr = inet->inet_saddr;
9898
}
9999

include/net/sock.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2875,13 +2875,14 @@ static inline void sk_pacing_shift_update(struct sock *sk, int val)
28752875
*/
28762876
static inline bool sk_dev_equal_l3scope(struct sock *sk, int dif)
28772877
{
2878+
int bound_dev_if = READ_ONCE(sk->sk_bound_dev_if);
28782879
int mdif;
28792880

2880-
if (!sk->sk_bound_dev_if || sk->sk_bound_dev_if == dif)
2881+
if (!bound_dev_if || bound_dev_if == dif)
28812882
return true;
28822883

28832884
mdif = l3mdev_master_ifindex_by_index(sock_net(sk), dif);
2884-
if (mdif && mdif == sk->sk_bound_dev_if)
2885+
if (mdif && mdif == bound_dev_if)
28852886
return true;
28862887

28872888
return false;

net/ipv6/datagram.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -218,11 +218,11 @@ int __ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr,
218218
err = -EINVAL;
219219
goto out;
220220
}
221-
sk->sk_bound_dev_if = usin->sin6_scope_id;
221+
WRITE_ONCE(sk->sk_bound_dev_if, usin->sin6_scope_id);
222222
}
223223

224224
if (!sk->sk_bound_dev_if && (addr_type & IPV6_ADDR_MULTICAST))
225-
sk->sk_bound_dev_if = np->mcast_oif;
225+
WRITE_ONCE(sk->sk_bound_dev_if, np->mcast_oif);
226226

227227
/* Connect to link-local address requires an interface */
228228
if (!sk->sk_bound_dev_if) {
@@ -798,7 +798,7 @@ int ip6_datagram_send_ctl(struct net *net, struct sock *sk,
798798
if (src_idx) {
799799
if (fl6->flowi6_oif &&
800800
src_idx != fl6->flowi6_oif &&
801-
(sk->sk_bound_dev_if != fl6->flowi6_oif ||
801+
(READ_ONCE(sk->sk_bound_dev_if) != fl6->flowi6_oif ||
802802
!sk_dev_equal_l3scope(sk, src_idx)))
803803
return -EINVAL;
804804
fl6->flowi6_oif = src_idx;

net/ipv6/udp.c

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ static int compute_score(struct sock *sk, struct net *net,
105105
const struct in6_addr *daddr, unsigned short hnum,
106106
int dif, int sdif)
107107
{
108-
int score;
108+
int bound_dev_if, score;
109109
struct inet_sock *inet;
110110
bool dev_match;
111111

@@ -132,10 +132,11 @@ static int compute_score(struct sock *sk, struct net *net,
132132
score++;
133133
}
134134

135-
dev_match = udp_sk_bound_dev_eq(net, sk->sk_bound_dev_if, dif, sdif);
135+
bound_dev_if = READ_ONCE(sk->sk_bound_dev_if);
136+
dev_match = udp_sk_bound_dev_eq(net, bound_dev_if, dif, sdif);
136137
if (!dev_match)
137138
return -1;
138-
if (sk->sk_bound_dev_if)
139+
if (bound_dev_if)
139140
score++;
140141

141142
if (READ_ONCE(sk->sk_incoming_cpu) == raw_smp_processor_id())
@@ -789,7 +790,7 @@ static bool __udp_v6_is_mcast_sock(struct net *net, struct sock *sk,
789790
(inet->inet_dport && inet->inet_dport != rmt_port) ||
790791
(!ipv6_addr_any(&sk->sk_v6_daddr) &&
791792
!ipv6_addr_equal(&sk->sk_v6_daddr, rmt_addr)) ||
792-
!udp_sk_bound_dev_eq(net, sk->sk_bound_dev_if, dif, sdif) ||
793+
!udp_sk_bound_dev_eq(net, READ_ONCE(sk->sk_bound_dev_if), dif, sdif) ||
793794
(!ipv6_addr_any(&sk->sk_v6_rcv_saddr) &&
794795
!ipv6_addr_equal(&sk->sk_v6_rcv_saddr, loc_addr)))
795796
return false;
@@ -1433,7 +1434,7 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
14331434
}
14341435

14351436
if (!fl6->flowi6_oif)
1436-
fl6->flowi6_oif = sk->sk_bound_dev_if;
1437+
fl6->flowi6_oif = READ_ONCE(sk->sk_bound_dev_if);
14371438

14381439
if (!fl6->flowi6_oif)
14391440
fl6->flowi6_oif = np->sticky_pktinfo.ipi6_ifindex;

0 commit comments

Comments
 (0)