| Message ID | 20240227222259.4081489-1-edumazet@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 67ea41d19d2aad2da2bcaeb7992c4fff9dc28a8f |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] inet6: expand rcu_read_lock() scope in inet6_dump_addr() | expand |
Tue, Feb 27, 2024 at 11:22:59PM CET, edumazet@google.com wrote: >I missed that inet6_dump_addr() is calling in6_dump_addrs() >from two points. > >First one under RTNL protection, and second one under rcu_read_lock(). > >Since we want to remove RTNL use from inet6_dump_addr() very soon, >no longer assume in6_dump_addrs() is protected by RTNL (even >if this is still the case). > >Use rcu_read_lock() earlier to fix this lockdep splat: > >WARNING: suspicious RCU usage >6.8.0-rc5-syzkaller-01618-gf8cbf6bde4c8 #0 Not tainted > >net/ipv6/addrconf.c:5317 suspicious rcu_dereference_check() usage! > >other info that might help us debug this: > >rcu_scheduler_active = 2, debug_locks = 1 >3 locks held by syz-executor.2/8834: > #0: ffff88802f554678 (nlk_cb_mutex-ROUTE){+.+.}-{3:3}, at: __netlink_dump_start+0x119/0x780 net/netlink/af_netlink.c:2338 > #1: ffffffff8f377a88 (rtnl_mutex){+.+.}-{3:3}, at: netlink_dump+0x676/0xda0 net/netlink/af_netlink.c:2265 > #2: ffff88807e5f0580 (&ndev->lock){++--}-{2:2}, at: in6_dump_addrs+0xb8/0x1de0 net/ipv6/addrconf.c:5279 > >stack backtrace: >CPU: 1 PID: 8834 Comm: syz-executor.2 Not tainted 6.8.0-rc5-syzkaller-01618-gf8cbf6bde4c8 #0 >Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/25/2024 >Call Trace: > <TASK> > __dump_stack lib/dump_stack.c:88 [inline] > dump_stack_lvl+0x1e7/0x2e0 lib/dump_stack.c:106 > lockdep_rcu_suspicious+0x220/0x340 kernel/locking/lockdep.c:6712 > in6_dump_addrs+0x1b47/0x1de0 net/ipv6/addrconf.c:5317 > inet6_dump_addr+0x1597/0x1690 net/ipv6/addrconf.c:5428 > netlink_dump+0x6a6/0xda0 net/netlink/af_netlink.c:2266 > __netlink_dump_start+0x59d/0x780 net/netlink/af_netlink.c:2374 > netlink_dump_start include/linux/netlink.h:340 [inline] > rtnetlink_rcv_msg+0xcf7/0x10d0 net/core/rtnetlink.c:6555 > netlink_rcv_skb+0x1e3/0x430 net/netlink/af_netlink.c:2547 > netlink_unicast_kernel net/netlink/af_netlink.c:1335 [inline] > netlink_unicast+0x7ea/0x980 net/netlink/af_netlink.c:1361 > netlink_sendmsg+0x8e0/0xcb0 net/netlink/af_netlink.c:1902 > sock_sendmsg_nosec net/socket.c:730 [inline] > __sock_sendmsg+0x221/0x270 net/socket.c:745 > ____sys_sendmsg+0x525/0x7d0 net/socket.c:2584 > ___sys_sendmsg net/socket.c:2638 [inline] > __sys_sendmsg+0x2b0/0x3a0 net/socket.c:2667 > >Fixes: c3718936ec47 ("ipv6: anycast: complete RCU handling of struct ifacaddr6") >Reported-by: syzbot <syzkaller@googlegroups.com> >Signed-off-by: Eric Dumazet <edumazet@google.com> >Cc: Jiri Pirko <jiri@nvidia.com> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Tue, 27 Feb 2024 22:22:59 +0000 you wrote: > I missed that inet6_dump_addr() is calling in6_dump_addrs() > from two points. > > First one under RTNL protection, and second one under rcu_read_lock(). > > Since we want to remove RTNL use from inet6_dump_addr() very soon, > no longer assume in6_dump_addrs() is protected by RTNL (even > if this is still the case). > > [...] Here is the summary with links: - [net-next] inet6: expand rcu_read_lock() scope in inet6_dump_addr() https://git.kernel.org/netdev/net-next/c/67ea41d19d2a You are awesome, thank you!
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index e27069ad938ca68d758ef956b8c36cb85697eeb5..953a95898e4adce877d153f73ce4ec4a127e60e7 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -5300,9 +5300,9 @@ static int in6_dump_addrs(struct inet6_dev *idev, struct sk_buff *skb, fillargs->event = RTM_GETMULTICAST; /* multicast address */ - for (ifmca = rtnl_dereference(idev->mc_list); + for (ifmca = rcu_dereference(idev->mc_list); ifmca; - ifmca = rtnl_dereference(ifmca->next), ip_idx++) { + ifmca = rcu_dereference(ifmca->next), ip_idx++) { if (ip_idx < s_ip_idx) continue; err = inet6_fill_ifmcaddr(skb, ifmca, fillargs); @@ -5410,6 +5410,7 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb, s_idx = idx = cb->args[1]; s_ip_idx = cb->args[2]; + rcu_read_lock(); if (cb->strict_check) { err = inet6_valid_dump_ifaddr_req(nlh, &fillargs, &tgt_net, skb->sk, cb); @@ -5434,7 +5435,6 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb, } } - rcu_read_lock(); cb->seq = inet6_base_seq(tgt_net); for (h = s_h; h < NETDEV_HASHENTRIES; h++, s_idx = 0) { idx = 0; @@ -5456,10 +5456,10 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb, } } done: - rcu_read_unlock(); cb->args[0] = h; cb->args[1] = idx; put_tgt_net: + rcu_read_unlock(); if (fillargs.netnsid >= 0) put_net(tgt_net);
I missed that inet6_dump_addr() is calling in6_dump_addrs() from two points. First one under RTNL protection, and second one under rcu_read_lock(). Since we want to remove RTNL use from inet6_dump_addr() very soon, no longer assume in6_dump_addrs() is protected by RTNL (even if this is still the case). Use rcu_read_lock() earlier to fix this lockdep splat: WARNING: suspicious RCU usage 6.8.0-rc5-syzkaller-01618-gf8cbf6bde4c8 #0 Not tainted net/ipv6/addrconf.c:5317 suspicious rcu_dereference_check() usage! other info that might help us debug this: rcu_scheduler_active = 2, debug_locks = 1 3 locks held by syz-executor.2/8834: #0: ffff88802f554678 (nlk_cb_mutex-ROUTE){+.+.}-{3:3}, at: __netlink_dump_start+0x119/0x780 net/netlink/af_netlink.c:2338 #1: ffffffff8f377a88 (rtnl_mutex){+.+.}-{3:3}, at: netlink_dump+0x676/0xda0 net/netlink/af_netlink.c:2265 #2: ffff88807e5f0580 (&ndev->lock){++--}-{2:2}, at: in6_dump_addrs+0xb8/0x1de0 net/ipv6/addrconf.c:5279 stack backtrace: CPU: 1 PID: 8834 Comm: syz-executor.2 Not tainted 6.8.0-rc5-syzkaller-01618-gf8cbf6bde4c8 #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/25/2024 Call Trace: <TASK> __dump_stack lib/dump_stack.c:88 [inline] dump_stack_lvl+0x1e7/0x2e0 lib/dump_stack.c:106 lockdep_rcu_suspicious+0x220/0x340 kernel/locking/lockdep.c:6712 in6_dump_addrs+0x1b47/0x1de0 net/ipv6/addrconf.c:5317 inet6_dump_addr+0x1597/0x1690 net/ipv6/addrconf.c:5428 netlink_dump+0x6a6/0xda0 net/netlink/af_netlink.c:2266 __netlink_dump_start+0x59d/0x780 net/netlink/af_netlink.c:2374 netlink_dump_start include/linux/netlink.h:340 [inline] rtnetlink_rcv_msg+0xcf7/0x10d0 net/core/rtnetlink.c:6555 netlink_rcv_skb+0x1e3/0x430 net/netlink/af_netlink.c:2547 netlink_unicast_kernel net/netlink/af_netlink.c:1335 [inline] netlink_unicast+0x7ea/0x980 net/netlink/af_netlink.c:1361 netlink_sendmsg+0x8e0/0xcb0 net/netlink/af_netlink.c:1902 sock_sendmsg_nosec net/socket.c:730 [inline] __sock_sendmsg+0x221/0x270 net/socket.c:745 ____sys_sendmsg+0x525/0x7d0 net/socket.c:2584 ___sys_sendmsg net/socket.c:2638 [inline] __sys_sendmsg+0x2b0/0x3a0 net/socket.c:2667 Fixes: c3718936ec47 ("ipv6: anycast: complete RCU handling of struct ifacaddr6") Reported-by: syzbot <syzkaller@googlegroups.com> Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Jiri Pirko <jiri@nvidia.com> --- net/ipv6/addrconf.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)