[PATCH net] ipv6: take tb6_lock across the fib6 dump sernum snapshot

[PATCH net] ipv6: take tb6_lock across the fib6 dump sernum snapshot

[Jun Yang]

From: Jun Yang <junvyyang@tencent.com>

A resumable RTM_GETROUTE dump parks a raw struct fib6_info pointer in
w->leaf when the skb fills (fib6_dump_node()) and resumes from it on the
next recvmsg(). No reference is held across the suspension; the only
thing standing between the walker and a freed route is the fn_sernum
check in fib6_dump_table(), which restarts from the root when the tree
changed.

That check is done outside tb6_lock at both ends: the snapshot after the
first walk is taken after spin_unlock_bh(), and on continuation the
compare runs before spin_lock_bh(). A writer that slips in between, e.g.
RTM_NEWROUTE with NLM_F_REPLACE on the route the walker is parked on,
frees the route via fib6_info_release() (the replace path has no
FOR_WALKERS fixup, unlike fib6_del_route()) and bumps fn_sernum, yet the
dump records the new serial next to the stale w->leaf, or passes the
compare and only then takes the lock. Either way the restart is skipped
and fib6_walk_continue() hands the freed route to rt6_fill_node():

  BUG: KASAN: slab-use-after-free in rt6_fill_node (net/ipv6/route.c:5821)
  Read of size 4 at addr ffff88810adb0c64 by task fib6dump-repro/541
  CPU: 0 UID: 1000 PID: 541 Comm: fib6dump-repro Not tainted 7.3.0-rc6 #1 PREEMPT
  Call Trace:
   rt6_fill_node (net/ipv6/route.c:5821)
   rt6_dump_route (net/ipv6/route.c:6130)
   fib6_dump_node (net/ipv6/ip6_fib.c:534)
   fib6_walk_continue (net/ipv6/ip6_fib.c:2180)
   fib6_dump_table (net/ipv6/ip6_fib.c:620)
   inet6_dump_fib (net/ipv6/ip6_fib.c:714)
   rtnl_dumpit (net/core/rtnetlink.c:7006)
   netlink_dump (net/netlink/af_netlink.c:2350)
   netlink_recvmsg (net/netlink/af_netlink.c:2001)
   sock_recvmsg (net/socket.c:1173)
   __sys_recvfrom (net/socket.c:2331)

Take tb6_lock once at the top of fib6_dump_table() and hold it across
the snapshot, the comparison and the walker call, so fn_sernum is read
in the same critical section as the walk it describes.

Fixes: 2bec5a369ee7 ("ipv6: fib: fix crash when changing large fib while dumping it")
Cc: stable@kernel.org
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Assisted-by: tencentos-corvus-ai:hy4-preview
Signed-off-by: Jun Yang <junvyyang@tencent.com>
---
A KASAN reproducer for this issue is available if requested.

 net/ipv6/ip6_fib.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 9ff761962b45..70482381165f 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -592,14 +592,13 @@ static int fib6_dump_table(struct fib6_table *table, struct sk_buff *skb,
 	w = (void *)cb->args[2];
 	w->root = &table->tb6_root;
 
+	spin_lock_bh(&table->tb6_lock);
 	if (cb->args[4] == 0) {
 		w->count = 0;
 		w->skip = 0;
 		w->skip_in_node = 0;
 
-		spin_lock_bh(&table->tb6_lock);
 		res = fib6_walk(net, w);
-		spin_unlock_bh(&table->tb6_lock);
 		if (res > 0) {
 			cb->args[4] = 1;
 			cb->args[5] = READ_ONCE(w->root->fn_sernum);
@@ -616,14 +615,13 @@ static int fib6_dump_table(struct fib6_table *table, struct sk_buff *skb,
 		} else
 			w->skip = 0;
 
-		spin_lock_bh(&table->tb6_lock);
 		res = fib6_walk_continue(w);
-		spin_unlock_bh(&table->tb6_lock);
 		if (res <= 0) {
 			fib6_walker_unlink(net, w);
 			cb->args[4] = 0;
 		}
 	}
+	spin_unlock_bh(&table->tb6_lock);
 
 	return res;
 }

base-commit: a90ee4305c4a5df72c11b31dacfdc76e00fcf78a
-- 
2.43.7