On Fri, Mar 23, 2018 at 7:57 AM Eric Dumazet <eduma...@google.com> wrote:
> syzbot reported a LOCKDEP splat [1] in rt6_age_examine_exception()
> rt6_age_examine_exception() is called while rt6_exception_lock is held.
> This lock is the lower one in the lock hierarchy, thus we can not
> call dst_neigh_lookup() function, as it can fallback to neigh_create()
> We should instead do a pure RCU lookup. As a bonus we avoid
> a pair of atomic operations on neigh refcount.
> [1]
> WARNING: possible circular locking dependency detected
> 4.16.0-rc4+ #277 Not tainted
> syz-executor7/4015 is trying to acquire lock:
> (&ndev->lock){++--}, at: [<00000000416dce19>]
__ipv6_dev_mc_dec+0x45/0x350 net/ipv6/mcast.c:928
> but task is already holding lock:
> (&tbl->lock){++-.}, at: [<00000000b5cb1d65>] neigh_ifdown+0x3d/0x250
net/core/neighbour.c:292
> which lock already depends on the new lock.
> the existing dependency chain (in reverse order) is:
> -> #3 (&tbl->lock){++-.}:
> __raw_write_lock_bh include/linux/rwlock_api_smp.h:203 [inline]
> _raw_write_lock_bh+0x31/0x40 kernel/locking/spinlock.c:312
> __neigh_create+0x87e/0x1d90 net/core/neighbour.c:528
> neigh_create include/net/neighbour.h:315 [inline]
> ip6_neigh_lookup+0x9a7/0xba0 net/ipv6/route.c:228
> dst_neigh_lookup include/net/dst.h:405 [inline]
> rt6_age_examine_exception net/ipv6/route.c:1609 [inline]
> rt6_age_exceptions+0x381/0x660 net/ipv6/route.c:1645
> fib6_age+0xfb/0x140 net/ipv6/ip6_fib.c:2033
> fib6_clean_node+0x389/0x580 net/ipv6/ip6_fib.c:1919
> fib6_walk_continue+0x46c/0x8a0 net/ipv6/ip6_fib.c:1845
> fib6_walk+0x91/0xf0 net/ipv6/ip6_fib.c:1893
> fib6_clean_tree+0x1e6/0x340 net/ipv6/ip6_fib.c:1970
> __fib6_clean_all+0x1f4/0x3a0 net/ipv6/ip6_fib.c:1986
> fib6_clean_all net/ipv6/ip6_fib.c:1997 [inline]
> fib6_run_gc+0x16b/0x3c0 net/ipv6/ip6_fib.c:2053
> ndisc_netdev_event+0x3c2/0x4a0 net/ipv6/ndisc.c:1781
> notifier_call_chain+0x136/0x2c0 kernel/notifier.c:93
> __raw_notifier_call_chain kernel/notifier.c:394 [inline]
> raw_notifier_call_chain+0x2d/0x40 kernel/notifier.c:401
> call_netdevice_notifiers_info+0x32/0x70 net/core/dev.c:1707
> call_netdevice_notifiers net/core/dev.c:1725 [inline]
> __dev_notify_flags+0x262/0x430 net/core/dev.c:6960
> dev_change_flags+0xf5/0x140 net/core/dev.c:6994
> devinet_ioctl+0x126a/0x1ac0 net/ipv4/devinet.c:1080
> inet_ioctl+0x184/0x310 net/ipv4/af_inet.c:919
> sock_do_ioctl+0xef/0x390 net/socket.c:957
> sock_ioctl+0x36b/0x610 net/socket.c:1081
> vfs_ioctl fs/ioctl.c:46 [inline]
> do_vfs_ioctl+0x1b1/0x1520 fs/ioctl.c:686
> SYSC_ioctl fs/ioctl.c:701 [inline]
> SyS_ioctl+0x8f/0xc0 fs/ioctl.c:692
> do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x42/0xb7
> -> #2 (rt6_exception_lock){+.-.}:
> __raw_spin_lock_bh include/linux/spinlock_api_smp.h:135 [inline]
> _raw_spin_lock_bh+0x31/0x40 kernel/locking/spinlock.c:168
> spin_lock_bh include/linux/spinlock.h:315 [inline]
> rt6_flush_exceptions+0x21/0x210 net/ipv6/route.c:1367
> fib6_del_route net/ipv6/ip6_fib.c:1677 [inline]
> fib6_del+0x624/0x12c0 net/ipv6/ip6_fib.c:1761
> __ip6_del_rt+0xc7/0x120 net/ipv6/route.c:2980
> ip6_del_rt+0x132/0x1a0 net/ipv6/route.c:2993
> __ipv6_dev_ac_dec+0x3b1/0x600 net/ipv6/anycast.c:332
> ipv6_dev_ac_dec net/ipv6/anycast.c:345 [inline]
> ipv6_sock_ac_close+0x2b4/0x3e0 net/ipv6/anycast.c:200
> inet6_release+0x48/0x70 net/ipv6/af_inet6.c:433
> sock_release+0x8d/0x1e0 net/socket.c:594
> sock_close+0x16/0x20 net/socket.c:1149
> __fput+0x327/0x7e0 fs/file_table.c:209
> ____fput+0x15/0x20 fs/file_table.c:243
> task_work_run+0x199/0x270 kernel/task_work.c:113
> exit_task_work include/linux/task_work.h:22 [inline]
> do_exit+0x9bb/0x1ad0 kernel/exit.c:865
> do_group_exit+0x149/0x400 kernel/exit.c:968
> get_signal+0x73a/0x16d0 kernel/signal.c:2469
> do_signal+0x90/0x1e90 arch/x86/kernel/signal.c:809
> exit_to_usermode_loop+0x258/0x2f0 arch/x86/entry/common.c:162
> prepare_exit_to_usermode arch/x86/entry/common.c:196 [inline]
> syscall_return_slowpath arch/x86/entry/common.c:265 [inline]
> do_syscall_64+0x6ec/0x940 arch/x86/entry/common.c:292
> entry_SYSCALL_64_after_hwframe+0x42/0xb7
> -> #1 (&(&tb->tb6_lock)->rlock){+.-.}:
> __raw_spin_lock_bh include/linux/spinlock_api_smp.h:135 [inline]
> _raw_spin_lock_bh+0x31/0x40 kernel/locking/spinlock.c:168
> spin_lock_bh include/linux/spinlock.h:315 [inline]
> __ip6_ins_rt+0x56/0x90 net/ipv6/route.c:1007
> ip6_route_add+0x141/0x190 net/ipv6/route.c:2955
> addrconf_prefix_route+0x44f/0x620 net/ipv6/addrconf.c:2359
> fixup_permanent_addr net/ipv6/addrconf.c:3368 [inline]
> addrconf_permanent_addr net/ipv6/addrconf.c:3391 [inline]
> addrconf_notify+0x1ad2/0x2310 net/ipv6/addrconf.c:3460
> notifier_call_chain+0x136/0x2c0 kernel/notifier.c:93
> __raw_notifier_call_chain kernel/notifier.c:394 [inline]
> raw_notifier_call_chain+0x2d/0x40 kernel/notifier.c:401
> call_netdevice_notifiers_info+0x32/0x70 net/core/dev.c:1707
> call_netdevice_notifiers net/core/dev.c:1725 [inline]
> __dev_notify_flags+0x15d/0x430 net/core/dev.c:6958
> dev_change_flags+0xf5/0x140 net/core/dev.c:6994
> do_setlink+0xa22/0x3bb0 net/core/rtnetlink.c:2357
> rtnl_newlink+0xf37/0x1a50 net/core/rtnetlink.c:2965
> rtnetlink_rcv_msg+0x57f/0xb10 net/core/rtnetlink.c:4641
> netlink_rcv_skb+0x14b/0x380 net/netlink/af_netlink.c:2444
> rtnetlink_rcv+0x1c/0x20 net/core/rtnetlink.c:4659
> netlink_unicast_kernel net/netlink/af_netlink.c:1308 [inline]
> netlink_unicast+0x4c4/0x6b0 net/netlink/af_netlink.c:1334
> netlink_sendmsg+0xa4a/0xe60 net/netlink/af_netlink.c:1897
> sock_sendmsg_nosec net/socket.c:629 [inline]
> sock_sendmsg+0xca/0x110 net/socket.c:639
> ___sys_sendmsg+0x767/0x8b0 net/socket.c:2047
> __sys_sendmsg+0xe5/0x210 net/socket.c:2081
> SYSC_sendmsg net/socket.c:2092 [inline]
> SyS_sendmsg+0x2d/0x50 net/socket.c:2088
> do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x42/0xb7
> -> #0 (&ndev->lock){++--}:
> lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:3920
> __raw_write_lock_bh include/linux/rwlock_api_smp.h:203 [inline]
> _raw_write_lock_bh+0x31/0x40 kernel/locking/spinlock.c:312
> __ipv6_dev_mc_dec+0x45/0x350 net/ipv6/mcast.c:928
> ipv6_dev_mc_dec+0x110/0x1f0 net/ipv6/mcast.c:961
> pndisc_destructor+0x21a/0x340 net/ipv6/ndisc.c:392
> pneigh_ifdown net/core/neighbour.c:695 [inline]
> neigh_ifdown+0x149/0x250 net/core/neighbour.c:294
> rt6_disable_ip+0x537/0x700 net/ipv6/route.c:3874
> addrconf_ifdown+0x14b/0x14f0 net/ipv6/addrconf.c:3633
> addrconf_notify+0x5f8/0x2310 net/ipv6/addrconf.c:3557
> notifier_call_chain+0x136/0x2c0 kernel/notifier.c:93
> __raw_notifier_call_chain kernel/notifier.c:394 [inline]
> raw_notifier_call_chain+0x2d/0x40 kernel/notifier.c:401
> call_netdevice_notifiers_info+0x32/0x70 net/core/dev.c:1707
> call_netdevice_notifiers net/core/dev.c:1725 [inline]
> __dev_notify_flags+0x262/0x430 net/core/dev.c:6960
> dev_change_flags+0xf5/0x140 net/core/dev.c:6994
> devinet_ioctl+0x126a/0x1ac0 net/ipv4/devinet.c:1080
> inet_ioctl+0x184/0x310 net/ipv4/af_inet.c:919
> packet_ioctl+0x1ff/0x310 net/packet/af_packet.c:4066
> sock_do_ioctl+0xef/0x390 net/socket.c:957
> sock_ioctl+0x36b/0x610 net/socket.c:1081
> vfs_ioctl fs/ioctl.c:46 [inline]
> do_vfs_ioctl+0x1b1/0x1520 fs/ioctl.c:686
> SYSC_ioctl fs/ioctl.c:701 [inline]
> SyS_ioctl+0x8f/0xc0 fs/ioctl.c:692
> do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x42/0xb7
> other info that might help us debug this:
> Chain exists of:
> &ndev->lock --> rt6_exception_lock --> &tbl->lock
> Possible unsafe locking scenario:
> CPU0 CPU1
> ---- ----
> lock(&tbl->lock);
> lock(rt6_exception_lock);
> lock(&tbl->lock);
> lock(&ndev->lock);
> *** DEADLOCK ***
> 2 locks held by syz-executor7/4015:
> #0: (rtnl_mutex){+.+.}, at: [<00000000a2f16daa>] rtnl_lock+0x17/0x20
net/core/rtnetlink.c:74
> #1: (&tbl->lock){++-.}, at: [<00000000b5cb1d65>]
neigh_ifdown+0x3d/0x250 net/core/neighbour.c:292
> stack backtrace:
> CPU: 0 PID: 4015 Comm: syz-executor7 Not tainted 4.16.0-rc4+ #277
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:17 [inline]
> dump_stack+0x194/0x24d lib/dump_stack.c:53
> print_circular_bug.isra.38+0x2cd/0x2dc kernel/locking/lockdep.c:1223
> check_prev_add kernel/locking/lockdep.c:1863 [inline]
> check_prevs_add kernel/locking/lockdep.c:1976 [inline]
> validate_chain kernel/locking/lockdep.c:2417 [inline]
> __lock_acquire+0x30a8/0x3e00 kernel/locking/lockdep.c:3431
> lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:3920
> __raw_write_lock_bh include/linux/rwlock_api_smp.h:203 [inline]
> _raw_write_lock_bh+0x31/0x40 kernel/locking/spinlock.c:312
> __ipv6_dev_mc_dec+0x45/0x350 net/ipv6/mcast.c:928
> ipv6_dev_mc_dec+0x110/0x1f0 net/ipv6/mcast.c:961
> pndisc_destructor+0x21a/0x340 net/ipv6/ndisc.c:392
> pneigh_ifdown net/core/neighbour.c:695 [inline]
> neigh_ifdown+0x149/0x250 net/core/neighbour.c:294
> rt6_disable_ip+0x537/0x700 net/ipv6/route.c:3874
> addrconf_ifdown+0x14b/0x14f0 net/ipv6/addrconf.c:3633
> addrconf_notify+0x5f8/0x2310 net/ipv6/addrconf.c:3557
> notifier_call_chain+0x136/0x2c0 kernel/notifier.c:93
> __raw_notifier_call_chain kernel/notifier.c:394 [inline]
> raw_notifier_call_chain+0x2d/0x40 kernel/notifier.c:401
> call_netdevice_notifiers_info+0x32/0x70 net/core/dev.c:1707
> call_netdevice_notifiers net/core/dev.c:1725 [inline]
> __dev_notify_flags+0x262/0x430 net/core/dev.c:6960
> dev_change_flags+0xf5/0x140 net/core/dev.c:6994
> devinet_ioctl+0x126a/0x1ac0 net/ipv4/devinet.c:1080
> inet_ioctl+0x184/0x310 net/ipv4/af_inet.c:919
> packet_ioctl+0x1ff/0x310 net/packet/af_packet.c:4066
> sock_do_ioctl+0xef/0x390 net/socket.c:957
> sock_ioctl+0x36b/0x610 net/socket.c:1081
> vfs_ioctl fs/ioctl.c:46 [inline]
> do_vfs_ioctl+0x1b1/0x1520 fs/ioctl.c:686
> SYSC_ioctl fs/ioctl.c:701 [inline]
> SyS_ioctl+0x8f/0xc0 fs/ioctl.c:692
> do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x42/0xb7
> Fixes: c757faa8bfa2 ("ipv6: prepare fib6_age() for exception table")
> Signed-off-by: Eric Dumazet <eduma...@google.com>
> Cc: Wei Wang <wei...@google.com>
> Cc: Martin KaFai Lau <ka...@fb.com>
> ---
Nice fix. Thanks Eric.
Acked-by: Wei Wang <wei...@google.com>
> net/ipv6/route.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index
b0d5c64e19780ce94feb112285ed1d85dbe07e9e..b33d057ac5eb2a85e19be59f0bceacf547cc9e59
100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1626,11 +1626,10 @@ static void rt6_age_examine_exception(struct
rt6_exception_bucket *bucket,
> struct neighbour *neigh;
> __u8 neigh_flags = 0;
> - neigh = dst_neigh_lookup(&rt->dst, &rt->rt6i_gateway);
> - if (neigh) {
> + neigh = __ipv6_neigh_lookup_noref(rt->dst.dev,
&rt->rt6i_gateway);
> + if (neigh)
> neigh_flags = neigh->flags;
> - neigh_release(neigh);
> - }
> +
> if (!(neigh_flags & NTF_ROUTER)) {
> RT6_TRACE("purging route %p via non-router but
gateway\n",
> rt);
> @@ -1654,7 +1653,8 @@ void rt6_age_exceptions(struct rt6_info *rt,
> if (!rcu_access_pointer(rt->rt6i_exception_bucket))
> return;
> - spin_lock_bh(&rt6_exception_lock);
> + rcu_read_lock_bh();
> + spin_lock(&rt6_exception_lock);
> bucket = rcu_dereference_protected(rt->rt6i_exception_bucket,
> lockdep_is_held(&rt6_exception_lock));
> @@ -1668,7 +1668,8 @@ void rt6_age_exceptions(struct rt6_info *rt,
> bucket++;
> }
> }
> - spin_unlock_bh(&rt6_exception_lock);
> + spin_unlock(&rt6_exception_lock);
> + rcu_read_unlock_bh();
> }
> struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
> --
> 2.17.0.rc0.231.g781580f067-goog
