[PATCH 6.1 266/313] netlink: annotate data races around nlk->portid

[Greg Kroah-Hartman]

From: Eric Dumazet <edum...@google.com>

[ Upstream commit c1bb9484e3b05166880da8574504156ccbd0549e ]

syzbot reminds us netlink_getname() runs locklessly [1]

This first patch annotates the race against nlk->portid.

Following patches take care of the remaining races.

[1]
BUG: KCSAN: data-race in netlink_getname / netlink_insert

write to 0xffff88814176d310 of 4 bytes by task 2315 on cpu 1:
netlink_insert+0xf1/0x9a0 net/netlink/af_netlink.c:583
netlink_autobind+0xae/0x180 net/netlink/af_netlink.c:856
netlink_sendmsg+0x444/0x760 net/netlink/af_netlink.c:1895
sock_sendmsg_nosec net/socket.c:714 [inline]
sock_sendmsg net/socket.c:734 [inline]
____sys_sendmsg+0x38f/0x500 net/socket.c:2476
___sys_sendmsg net/socket.c:2530 [inline]
__sys_sendmsg+0x19a/0x230 net/socket.c:2559
__do_sys_sendmsg net/socket.c:2568 [inline]
__se_sys_sendmsg net/socket.c:2566 [inline]
__x64_sys_sendmsg+0x42/0x50 net/socket.c:2566
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x2b/0x70 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd

read to 0xffff88814176d310 of 4 bytes by task 2316 on cpu 0:
netlink_getname+0xcd/0x1a0 net/netlink/af_netlink.c:1144
__sys_getsockname+0x11d/0x1b0 net/socket.c:2026
__do_sys_getsockname net/socket.c:2041 [inline]
__se_sys_getsockname net/socket.c:2038 [inline]
__x64_sys_getsockname+0x3e/0x50 net/socket.c:2038
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x2b/0x70 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd

value changed: 0x00000000 -> 0xc9a49780

Reported by Kernel Concurrency Sanitizer on:
CPU: 0 PID: 2316 Comm: syz-executor.2 Not tainted 6.2.0-rc3-syzkaller-00030-ge8f60cd7db24-dirty #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/26/2022

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edum...@google.com>
Reported-by: syzbot <syzk...@googlegroups.com>
Signed-off-by: Jakub Kicinski <ku...@kernel.org>
Signed-off-by: Sasha Levin <sas...@kernel.org>
---
net/netlink/af_netlink.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index a662e8a5ff84..11a6309f17a3 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -580,7 +580,9 @@ static int netlink_insert(struct sock *sk, u32 portid)
if (nlk_sk(sk)->bound)
goto err;

- nlk_sk(sk)->portid = portid;
+ /* portid can be read locklessly from netlink_getname(). */
+ WRITE_ONCE(nlk_sk(sk)->portid, portid);
+
sock_hold(sk);

err = __netlink_insert(table, sk);
@@ -1130,7 +1132,8 @@ static int netlink_getname(struct socket *sock, struct sockaddr *addr,
nladdr->nl_pid = nlk->dst_portid;
nladdr->nl_groups = netlink_group_mask(nlk->dst_group);
} else {
- nladdr->nl_pid = nlk->portid;
+ /* Paired with WRITE_ONCE() in netlink_insert() */
+ nladdr->nl_pid = READ_ONCE(nlk->portid);
netlink_lock_table();
nladdr->nl_groups = nlk->groups ? nlk->groups[0] : 0;
netlink_unlock_table();
--
2.39.0

[Greg Kroah-Hartman]

index 974d32632ef4..1eab80af5112 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -578,7 +578,9 @@ static int netlink_insert(struct sock *sk, u32 portid)

if (nlk_sk(sk)->bound)
goto err;

- nlk_sk(sk)->portid = portid;
+ /* portid can be read locklessly from netlink_getname(). */
+ WRITE_ONCE(nlk_sk(sk)->portid, portid);
+
sock_hold(sk);

err = __netlink_insert(table, sk);

@@ -1132,7 +1134,8 @@ static int netlink_getname(struct socket *sock, struct sockaddr *addr,

[Greg Kroah-Hartman]

index d96a610929d9..438f45222ca5 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -570,7 +570,9 @@ static int netlink_insert(struct sock *sk, u32 portid)

if (nlk_sk(sk)->bound)
goto err;

- nlk_sk(sk)->portid = portid;
+ /* portid can be read locklessly from netlink_getname(). */
+ WRITE_ONCE(nlk_sk(sk)->portid, portid);
+
sock_hold(sk);

err = __netlink_insert(table, sk);

@@ -1124,7 +1126,8 @@ static int netlink_getname(struct socket *sock, struct sockaddr *addr,

[Greg Kroah-Hartman]

index 966c709c3831..52bf99ed7093 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -578,7 +578,9 @@ static int netlink_insert(struct sock *sk, u32 portid)

if (nlk_sk(sk)->bound)
goto err;

- nlk_sk(sk)->portid = portid;
+ /* portid can be read locklessly from netlink_getname(). */
+ WRITE_ONCE(nlk_sk(sk)->portid, portid);
+
sock_hold(sk);

err = __netlink_insert(table, sk);

@@ -1133,7 +1135,8 @@ static int netlink_getname(struct socket *sock, struct sockaddr *addr,

[Greg Kroah-Hartman]

index 86b70385dce3..f6b985877d9c 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -569,7 +569,9 @@ static int netlink_insert(struct sock *sk, u32 portid)

if (nlk_sk(sk)->bound)
goto err;

- nlk_sk(sk)->portid = portid;
+ /* portid can be read locklessly from netlink_getname(). */
+ WRITE_ONCE(nlk_sk(sk)->portid, portid);
+
sock_hold(sk);

err = __netlink_insert(table, sk);

@@ -1123,7 +1125,8 @@ static int netlink_getname(struct socket *sock, struct sockaddr *addr,