struct fib6_node's fn_sernum field can be read while other threads change it. Add READ_ONCE()/WRITE_ONCE() annotations. Do not change existing smp barriers in fib6_get_cookie_safe() and __fib6_update_sernum_upto_root() syzbot reported: BUG: KCSAN: data-race in fib6_clean_node / inet6_csk_route_socket write to 0xffff88813df62e2c of 4 bytes by task 1920 on cpu 1: fib6_clean_node+0xc2/0x260 net/ipv6/ip6_fib.c:2178 fib6_walk_continue+0x38e/0x430 net/ipv6/ip6_fib.c:2112 fib6_walk net/ipv6/ip6_fib.c:2160 [inline] fib6_clean_tree net/ipv6/ip6_fib.c:2240 [inline] __fib6_clean_all+0x1a9/0x2e0 net/ipv6/ip6_fib.c:2256 fib6_flush_trees+0x6c/0x80 net/ipv6/ip6_fib.c:2281 rt_genid_bump_ipv6 include/net/net_namespace.h:488 [inline] addrconf_dad_completed+0x57f/0x870 net/ipv6/addrconf.c:4230 addrconf_dad_work+0x908/0x1170 process_one_work+0x3f6/0x960 kernel/workqueue.c:2307 worker_thread+0x616/0xa70 kernel/workqueue.c:2454 kthread+0x1bf/0x1e0 kernel/kthread.c:359 ret_from_fork+0x1f/0x30 read to 0xffff88813df62e2c of 4 bytes by task 15701 on cpu 0: fib6_get_cookie_safe include/net/ip6_fib.h:285 [inline] rt6_get_cookie include/net/ip6_fib.h:306 [inline] ip6_dst_store include/net/ip6_route.h:234 [inline] inet6_csk_route_socket+0x352/0x3c0 net/ipv6/inet6_connection_sock.c:109 inet6_csk_xmit+0x91/0x1e0 net/ipv6/inet6_connection_sock.c:121 __tcp_transmit_skb+0x1323/0x1840 net/ipv4/tcp_output.c:1402 tcp_transmit_skb net/ipv4/tcp_output.c:1420 [inline] tcp_write_xmit+0x1450/0x4460 net/ipv4/tcp_output.c:2680 __tcp_push_pending_frames+0x68/0x1c0 net/ipv4/tcp_output.c:2864 tcp_push+0x2d9/0x2f0 net/ipv4/tcp.c:725 mptcp_push_release net/mptcp/protocol.c:1491 [inline] __mptcp_push_pending+0x46c/0x490 net/mptcp/protocol.c:1578 mptcp_sendmsg+0x9ec/0xa50 net/mptcp/protocol.c:1764 inet6_sendmsg+0x5f/0x80 net/ipv6/af_inet6.c:643 sock_sendmsg_nosec net/socket.c:705 [inline] sock_sendmsg net/socket.c:725 [inline] kernel_sendmsg+0x97/0xd0 net/socket.c:745 sock_no_sendpage+0x84/0xb0 net/core/sock.c:3086 inet_sendpage+0x9d/0xc0 net/ipv4/af_inet.c:834 kernel_sendpage+0x187/0x200 net/socket.c:3492 sock_sendpage+0x5a/0x70 net/socket.c:1007 pipe_to_sendpage+0x128/0x160 fs/splice.c:364 splice_from_pipe_feed fs/splice.c:418 [inline] __splice_from_pipe+0x207/0x500 fs/splice.c:562 splice_from_pipe fs/splice.c:597 [inline] generic_splice_sendpage+0x94/0xd0 fs/splice.c:746 do_splice_from fs/splice.c:767 [inline] direct_splice_actor+0x80/0xa0 fs/splice.c:936 splice_direct_to_actor+0x345/0x650 fs/splice.c:891 do_splice_direct+0x106/0x190 fs/splice.c:979 do_sendfile+0x675/0xc40 fs/read_write.c:1245 __do_sys_sendfile64 fs/read_write.c:1310 [inline] __se_sys_sendfile64 fs/read_write.c:1296 [inline] __x64_sys_sendfile64+0x102/0x140 fs/read_write.c:1296 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: 0x0000026f -> 0x00000271 Reported by Kernel Concurrency Sanitizer on: CPU: 0 PID: 15701 Comm: syz-executor.2 Not tainted 5.16.0-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 The Fixes tag I chose is probably arbitrary, I do not think we need to backport this patch to older kernels.
Unified · Split
| @@ -282,7 +282,7 @@ static inline bool fib6_get_cookie_safe(const struct fib6_info *f6i, · 281 unmodified lines | ||
| 282 | 282 | fn = rcu_dereference(f6i->fib6_node); |
| 283 | 283 | |
| 284 | 284 | if (fn) { |
| 285 | − *cookie = fn->fn_sernum; | |
| 285 | + *cookie = READ_ONCE(fn->fn_sernum); | |
| 286 | 286 | /* pairs with smp_wmb() in __fib6_update_sernum_upto_root() */ |
| 287 | 287 | smp_rmb(); |
| 288 | 288 | status = true; |
| @@ -112,7 +112,7 @@ void fib6_update_sernum(struct net *net, struct fib6_info *f6i) · 111 unmodified lines | ||
| 112 | 112 | fn = rcu_dereference_protected(f6i->fib6_node, |
| 113 | 113 | lockdep_is_held(&f6i->fib6_table->tb6_lock)); |
| 114 | 114 | if (fn) |
| 115 | − fn->fn_sernum = fib6_new_sernum(net); | |
| 115 | + WRITE_ONCE(fn->fn_sernum, fib6_new_sernum(net)); | |
| 116 | 116 | } |
| 117 | 117 | |
| 118 | 118 | /* |
| @@ -590,12 +590,13 @@ static int fib6_dump_table(struct fib6_table *table, struct sk_buff *skb, · 471 unmodified lines | ||
| 590 | 590 | spin_unlock_bh(&table->tb6_lock); |
| 591 | 591 | if (res > 0) { |
| 592 | 592 | cb->args[4] = 1; |
| 593 | − cb->args[5] = w->root->fn_sernum; | |
| 593 | + cb->args[5] = READ_ONCE(w->root->fn_sernum); | |
| 594 | 594 | } |
| 595 | 595 | } else { |
| 596 | − if (cb->args[5] != w->root->fn_sernum) { | |
| 596 | + int sernum = READ_ONCE(w->root->fn_sernum); | |
| 597 | + if (cb->args[5] != sernum) { | |
| 597 | 598 | /* Begin at the root if the tree changed */ |
| 598 | − cb->args[5] = w->root->fn_sernum; | |
| 599 | + cb->args[5] = sernum; | |
| 599 | 600 | w->state = FWS_INIT; |
| 600 | 601 | w->node = w->root; |
| 601 | 602 | w->skip = w->count; |
| @@ -1345,7 +1346,7 @@ static void __fib6_update_sernum_upto_root(struct fib6_info *rt, · 743 unmodified lines | ||
| 1345 | 1346 | /* paired with smp_rmb() in fib6_get_cookie_safe() */ |
| 1346 | 1347 | smp_wmb(); |
| 1347 | 1348 | while (fn) { |
| 1348 | − fn->fn_sernum = sernum; | |
| 1349 | + WRITE_ONCE(fn->fn_sernum, sernum); | |
| 1349 | 1350 | fn = rcu_dereference_protected(fn->parent, |
| 1350 | 1351 | lockdep_is_held(&rt->fib6_table->tb6_lock)); |
| 1351 | 1352 | } |
| @@ -2174,8 +2175,8 @@ static int fib6_clean_node(struct fib6_walker *w) · 822 unmodified lines | ||
| 2174 | 2175 | }; |
| 2175 | 2176 | |
| 2176 | 2177 | if (c->sernum != FIB6_NO_SERNUM_CHANGE && |
| 2177 | − w->node->fn_sernum != c->sernum) | |
| 2178 | − w->node->fn_sernum = c->sernum; | |
| 2178 | + READ_ONCE(w->node->fn_sernum) != c->sernum) | |
| 2179 | + WRITE_ONCE(w->node->fn_sernum, c->sernum); | |
| 2179 | 2180 | |
| 2180 | 2181 | if (!c->func) { |
| 2181 | 2182 | WARN_ON_ONCE(c->sernum == FIB6_NO_SERNUM_CHANGE); |
| @@ -2543,7 +2544,7 @@ static void ipv6_route_seq_setup_walk(struct ipv6_route_iter *iter, · 361 unmodified lines | ||
| 2543 | 2544 | iter->w.state = FWS_INIT; |
| 2544 | 2545 | iter->w.node = iter->w.root; |
| 2545 | 2546 | iter->w.args = iter; |
| 2546 | − iter->sernum = iter->w.root->fn_sernum; | |
| 2547 | + iter->sernum = READ_ONCE(iter->w.root->fn_sernum); | |
| 2547 | 2548 | INIT_LIST_HEAD(&iter->w.lh); |
| 2548 | 2549 | fib6_walker_link(net, &iter->w); |
| 2549 | 2550 | } |
| @@ -2571,8 +2572,10 @@ static struct fib6_table *ipv6_route_seq_next_table(struct fib6_table *tbl, · 21 unmodified lines | ||
| 2571 | 2572 | |
| 2572 | 2573 | static void ipv6_route_check_sernum(struct ipv6_route_iter *iter) |
| 2573 | 2574 | { |
| 2574 | − if (iter->sernum != iter->w.root->fn_sernum) { | |
| 2575 | − iter->sernum = iter->w.root->fn_sernum; | |
| 2575 | + int sernum = READ_ONCE(iter->w.root->fn_sernum); | |
| 2576 | + | |
| 2577 | + if (iter->sernum != sernum) { | |
| 2578 | + iter->sernum = sernum; | |
| 2576 | 2579 | iter->w.state = FWS_INIT; |
| 2577 | 2580 | iter->w.node = iter->w.root; |
| 2578 | 2581 | WARN_ON(iter->w.skip); |
| @@ -2802,7 +2802,7 @@ static void ip6_link_failure(struct sk_buff *skb) · 2801 unmodified lines | ||
| 2802 | 2802 | if (from) { |
| 2803 | 2803 | fn = rcu_dereference(from->fib6_node); |
| 2804 | 2804 | if (fn && (rt->rt6i_flags & RTF_DEFAULT)) |
| 2805 | − fn->fn_sernum = -1; | |
| 2805 | + WRITE_ONCE(fn->fn_sernum, -1); | |
| 2806 | 2806 | } |
| 2807 | 2807 | } |
| 2808 | 2808 | rcu_read_unlock(); |