Skip to content

Commit 086d490

Browse files
edumazetdavem330
authored andcommitted
ipv6: annotate some data-races around sk->sk_prot
IPv6 has this hack changing sk->sk_prot when an IPv6 socket is 'converted' to an IPv4 one with IPV6_ADDRFORM option. This operation is only performed for TCP and UDP, knowing their 'struct proto' for the two network families are populated in the same way, and can not disappear while a reader might use and dereference sk->sk_prot. If we think about it all reads of sk->sk_prot while either socket lock or RTNL is not acquired should be using READ_ONCE(). Also note that other layers like MPTCP, XFRM, CHELSIO_TLS also write over sk->sk_prot. BUG: KCSAN: data-race in inet6_recvmsg / ipv6_setsockopt write to 0xffff8881386f7aa8 of 8 bytes by task 26932 on cpu 0: do_ipv6_setsockopt net/ipv6/ipv6_sockglue.c:492 [inline] ipv6_setsockopt+0x3758/0x3910 net/ipv6/ipv6_sockglue.c:1019 udpv6_setsockopt+0x85/0x90 net/ipv6/udp.c:1649 sock_common_setsockopt+0x5d/0x70 net/core/sock.c:3489 __sys_setsockopt+0x209/0x2a0 net/socket.c:2180 __do_sys_setsockopt net/socket.c:2191 [inline] __se_sys_setsockopt net/socket.c:2188 [inline] __x64_sys_setsockopt+0x62/0x70 net/socket.c:2188 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x44/0xd0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae read to 0xffff8881386f7aa8 of 8 bytes by task 26911 on cpu 1: inet6_recvmsg+0x7a/0x210 net/ipv6/af_inet6.c:659 ____sys_recvmsg+0x16c/0x320 ___sys_recvmsg net/socket.c:2674 [inline] do_recvmmsg+0x3f5/0xae0 net/socket.c:2768 __sys_recvmmsg net/socket.c:2847 [inline] __do_sys_recvmmsg net/socket.c:2870 [inline] __se_sys_recvmmsg net/socket.c:2863 [inline] __x64_sys_recvmmsg+0xde/0x160 net/socket.c:2863 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x44/0xd0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x44/0xae value changed: 0xffffffff85e0e980 -> 0xffffffff85e01580 Reported by Kernel Concurrency Sanitizer on: CPU: 1 PID: 26911 Comm: syz-executor.3 Not tainted 5.17.0-rc2-syzkaller-00316-g0457e5153e0e-dirty #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Reported-by: syzbot <syzkaller@googlegroups.com> Signed-off-by: Eric Dumazet <edumazet@google.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 7ea0c16 commit 086d490

File tree

2 files changed

+22
-8
lines changed

2 files changed

+22
-8
lines changed

net/ipv6/af_inet6.c

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -441,11 +441,14 @@ int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
441441
{
442442
struct sock *sk = sock->sk;
443443
u32 flags = BIND_WITH_LOCK;
444+
const struct proto *prot;
444445
int err = 0;
445446

447+
/* IPV6_ADDRFORM can change sk->sk_prot under us. */
448+
prot = READ_ONCE(sk->sk_prot);
446449
/* If the socket has its own bind function then use it. */
447-
if (sk->sk_prot->bind)
448-
return sk->sk_prot->bind(sk, uaddr, addr_len);
450+
if (prot->bind)
451+
return prot->bind(sk, uaddr, addr_len);
449452

450453
if (addr_len < SIN6_LEN_RFC2133)
451454
return -EINVAL;
@@ -555,6 +558,7 @@ int inet6_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
555558
void __user *argp = (void __user *)arg;
556559
struct sock *sk = sock->sk;
557560
struct net *net = sock_net(sk);
561+
const struct proto *prot;
558562

559563
switch (cmd) {
560564
case SIOCADDRT:
@@ -572,9 +576,11 @@ int inet6_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
572576
case SIOCSIFDSTADDR:
573577
return addrconf_set_dstaddr(net, argp);
574578
default:
575-
if (!sk->sk_prot->ioctl)
579+
/* IPV6_ADDRFORM can change sk->sk_prot under us. */
580+
prot = READ_ONCE(sk->sk_prot);
581+
if (!prot->ioctl)
576582
return -ENOIOCTLCMD;
577-
return sk->sk_prot->ioctl(sk, cmd, arg);
583+
return prot->ioctl(sk, cmd, arg);
578584
}
579585
/*NOTREACHED*/
580586
return 0;
@@ -636,11 +642,14 @@ INDIRECT_CALLABLE_DECLARE(int udpv6_sendmsg(struct sock *, struct msghdr *,
636642
int inet6_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
637643
{
638644
struct sock *sk = sock->sk;
645+
const struct proto *prot;
639646

640647
if (unlikely(inet_send_prepare(sk)))
641648
return -EAGAIN;
642649

643-
return INDIRECT_CALL_2(sk->sk_prot->sendmsg, tcp_sendmsg, udpv6_sendmsg,
650+
/* IPV6_ADDRFORM can change sk->sk_prot under us. */
651+
prot = READ_ONCE(sk->sk_prot);
652+
return INDIRECT_CALL_2(prot->sendmsg, tcp_sendmsg, udpv6_sendmsg,
644653
sk, msg, size);
645654
}
646655

@@ -650,13 +659,16 @@ int inet6_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,
650659
int flags)
651660
{
652661
struct sock *sk = sock->sk;
662+
const struct proto *prot;
653663
int addr_len = 0;
654664
int err;
655665

656666
if (likely(!(flags & MSG_ERRQUEUE)))
657667
sock_rps_record_flow(sk);
658668

659-
err = INDIRECT_CALL_2(sk->sk_prot->recvmsg, tcp_recvmsg, udpv6_recvmsg,
669+
/* IPV6_ADDRFORM can change sk->sk_prot under us. */
670+
prot = READ_ONCE(sk->sk_prot);
671+
err = INDIRECT_CALL_2(prot->recvmsg, tcp_recvmsg, udpv6_recvmsg,
660672
sk, msg, size, flags & MSG_DONTWAIT,
661673
flags & ~MSG_DONTWAIT, &addr_len);
662674
if (err >= 0)

net/ipv6/ipv6_sockglue.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,8 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
475475
sock_prot_inuse_add(net, sk->sk_prot, -1);
476476
sock_prot_inuse_add(net, &tcp_prot, 1);
477477

478-
sk->sk_prot = &tcp_prot;
478+
/* Paired with READ_ONCE(sk->sk_prot) in net/ipv6/af_inet6.c */
479+
WRITE_ONCE(sk->sk_prot, &tcp_prot);
479480
icsk->icsk_af_ops = &ipv4_specific;
480481
sk->sk_socket->ops = &inet_stream_ops;
481482
sk->sk_family = PF_INET;
@@ -489,7 +490,8 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
489490
sock_prot_inuse_add(net, sk->sk_prot, -1);
490491
sock_prot_inuse_add(net, prot, 1);
491492

492-
sk->sk_prot = prot;
493+
/* Paired with READ_ONCE(sk->sk_prot) in net/ipv6/af_inet6.c */
494+
WRITE_ONCE(sk->sk_prot, prot);
493495
sk->sk_socket->ops = &inet_dgram_ops;
494496
sk->sk_family = PF_INET;
495497
}

0 commit comments

Comments
 (0)