Skip to content

Commit 8265792

Browse files
edumazetJakub Kicinski
authored andcommitted
net: silence KCSAN warnings around sk_add_backlog() calls
sk_add_backlog() callers usually read sk->sk_rcvbuf without owning the socket lock. This means sk_rcvbuf value can be changed by other cpus, and KCSAN complains. Add READ_ONCE() annotations to document the lockless nature of these reads. Note that writes over sk_rcvbuf should also use WRITE_ONCE(), but this will be done in separate patches to ease stable backports (if we decide this is relevant for stable trees). BUG: KCSAN: data-race in tcp_add_backlog / tcp_recvmsg write to 0xffff88812ab369f8 of 8 bytes by interrupt on cpu 1: __sk_add_backlog include/net/sock.h:902 [inline] sk_add_backlog include/net/sock.h:933 [inline] tcp_add_backlog+0x45a/0xcc0 net/ipv4/tcp_ipv4.c:1737 tcp_v4_rcv+0x1aba/0x1bf0 net/ipv4/tcp_ipv4.c:1925 ip_protocol_deliver_rcu+0x51/0x470 net/ipv4/ip_input.c:204 ip_local_deliver_finish+0x110/0x140 net/ipv4/ip_input.c:231 NF_HOOK include/linux/netfilter.h:305 [inline] NF_HOOK include/linux/netfilter.h:299 [inline] ip_local_deliver+0x133/0x210 net/ipv4/ip_input.c:252 dst_input include/net/dst.h:442 [inline] ip_rcv_finish+0x121/0x160 net/ipv4/ip_input.c:413 NF_HOOK include/linux/netfilter.h:305 [inline] NF_HOOK include/linux/netfilter.h:299 [inline] ip_rcv+0x18f/0x1a0 net/ipv4/ip_input.c:523 __netif_receive_skb_one_core+0xa7/0xe0 net/core/dev.c:5004 __netif_receive_skb+0x37/0xf0 net/core/dev.c:5118 netif_receive_skb_internal+0x59/0x190 net/core/dev.c:5208 napi_skb_finish net/core/dev.c:5671 [inline] napi_gro_receive+0x28f/0x330 net/core/dev.c:5704 receive_buf+0x284/0x30b0 drivers/net/virtio_net.c:1061 virtnet_receive drivers/net/virtio_net.c:1323 [inline] virtnet_poll+0x436/0x7d0 drivers/net/virtio_net.c:1428 napi_poll net/core/dev.c:6352 [inline] net_rx_action+0x3ae/0xa50 net/core/dev.c:6418 read to 0xffff88812ab369f8 of 8 bytes by task 7271 on cpu 0: tcp_recvmsg+0x470/0x1a30 net/ipv4/tcp.c:2047 inet_recvmsg+0xbb/0x250 net/ipv4/af_inet.c:838 sock_recvmsg_nosec net/socket.c:871 [inline] sock_recvmsg net/socket.c:889 [inline] sock_recvmsg+0x92/0xb0 net/socket.c:885 sock_read_iter+0x15f/0x1e0 net/socket.c:967 call_read_iter include/linux/fs.h:1864 [inline] new_sync_read+0x389/0x4f0 fs/read_write.c:414 __vfs_read+0xb1/0xc0 fs/read_write.c:427 vfs_read fs/read_write.c:461 [inline] vfs_read+0x143/0x2c0 fs/read_write.c:446 ksys_read+0xd5/0x1b0 fs/read_write.c:587 __do_sys_read fs/read_write.c:597 [inline] __se_sys_read fs/read_write.c:595 [inline] __x64_sys_read+0x4c/0x60 fs/read_write.c:595 do_syscall_64+0xcf/0x2f0 arch/x86/entry/common.c:296 entry_SYSCALL_64_after_hwframe+0x44/0xa9 Reported by Kernel Concurrency Sanitizer on: CPU: 0 PID: 7271 Comm: syz-fuzzer Not tainted 5.3.0+ #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Signed-off-by: Eric Dumazet <[email protected]> Reported-by: syzbot <[email protected]> Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 1f142c1 commit 8265792

File tree

6 files changed

+10
-10
lines changed

6 files changed

+10
-10
lines changed

net/core/sock.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,7 @@ int __sk_receive_skb(struct sock *sk, struct sk_buff *skb,
522522
rc = sk_backlog_rcv(sk, skb);
523523

524524
mutex_release(&sk->sk_lock.dep_map, 1, _RET_IP_);
525-
} else if (sk_add_backlog(sk, skb, sk->sk_rcvbuf)) {
525+
} else if (sk_add_backlog(sk, skb, READ_ONCE(sk->sk_rcvbuf))) {
526526
bh_unlock_sock(sk);
527527
atomic_inc(&sk->sk_drops);
528528
goto discard_and_relse;

net/ipv4/tcp_ipv4.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1644,7 +1644,7 @@ int tcp_v4_early_demux(struct sk_buff *skb)
16441644

16451645
bool tcp_add_backlog(struct sock *sk, struct sk_buff *skb)
16461646
{
1647-
u32 limit = sk->sk_rcvbuf + sk->sk_sndbuf;
1647+
u32 limit = READ_ONCE(sk->sk_rcvbuf) + READ_ONCE(sk->sk_sndbuf);
16481648
struct skb_shared_info *shinfo;
16491649
const struct tcphdr *th;
16501650
struct tcphdr *thtail;

net/llc/llc_conn.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -813,7 +813,7 @@ void llc_conn_handler(struct llc_sap *sap, struct sk_buff *skb)
813813
else {
814814
dprintk("%s: adding to backlog...\n", __func__);
815815
llc_set_backlog_type(skb, LLC_PACKET);
816-
if (sk_add_backlog(sk, skb, sk->sk_rcvbuf))
816+
if (sk_add_backlog(sk, skb, READ_ONCE(sk->sk_rcvbuf)))
817817
goto drop_unlock;
818818
}
819819
out:

net/sctp/input.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ int sctp_backlog_rcv(struct sock *sk, struct sk_buff *skb)
322322
bh_lock_sock(sk);
323323

324324
if (sock_owned_by_user(sk) || !sctp_newsk_ready(sk)) {
325-
if (sk_add_backlog(sk, skb, sk->sk_rcvbuf))
325+
if (sk_add_backlog(sk, skb, READ_ONCE(sk->sk_rcvbuf)))
326326
sctp_chunk_free(chunk);
327327
else
328328
backloged = 1;
@@ -337,7 +337,7 @@ int sctp_backlog_rcv(struct sock *sk, struct sk_buff *skb)
337337
return 0;
338338
} else {
339339
if (!sctp_newsk_ready(sk)) {
340-
if (!sk_add_backlog(sk, skb, sk->sk_rcvbuf))
340+
if (!sk_add_backlog(sk, skb, READ_ONCE(sk->sk_rcvbuf)))
341341
return 0;
342342
sctp_chunk_free(chunk);
343343
} else {
@@ -364,7 +364,7 @@ static int sctp_add_backlog(struct sock *sk, struct sk_buff *skb)
364364
struct sctp_ep_common *rcvr = chunk->rcvr;
365365
int ret;
366366

367-
ret = sk_add_backlog(sk, skb, sk->sk_rcvbuf);
367+
ret = sk_add_backlog(sk, skb, READ_ONCE(sk->sk_rcvbuf));
368368
if (!ret) {
369369
/* Hold the assoc/ep while hanging on the backlog queue.
370370
* This way, we know structures we need will not disappear

net/tipc/socket.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2119,13 +2119,13 @@ static unsigned int rcvbuf_limit(struct sock *sk, struct sk_buff *skb)
21192119
struct tipc_msg *hdr = buf_msg(skb);
21202120

21212121
if (unlikely(msg_in_group(hdr)))
2122-
return sk->sk_rcvbuf;
2122+
return READ_ONCE(sk->sk_rcvbuf);
21232123

21242124
if (unlikely(!msg_connected(hdr)))
2125-
return sk->sk_rcvbuf << msg_importance(hdr);
2125+
return READ_ONCE(sk->sk_rcvbuf) << msg_importance(hdr);
21262126

21272127
if (likely(tsk->peer_caps & TIPC_BLOCK_FLOWCTL))
2128-
return sk->sk_rcvbuf;
2128+
return READ_ONCE(sk->sk_rcvbuf);
21292129

21302130
return FLOWCTL_MSG_LIM;
21312131
}

net/x25/x25_dev.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ static int x25_receive_data(struct sk_buff *skb, struct x25_neigh *nb)
5555
if (!sock_owned_by_user(sk)) {
5656
queued = x25_process_rx_frame(sk, skb);
5757
} else {
58-
queued = !sk_add_backlog(sk, skb, sk->sk_rcvbuf);
58+
queued = !sk_add_backlog(sk, skb, READ_ONCE(sk->sk_rcvbuf));
5959
}
6060
bh_unlock_sock(sk);
6161
sock_put(sk);

0 commit comments

Comments
 (0)