amt_dev_stop() calls list_del_rcu() on each tunnel without holding amt->lock. amt_tunnel_expire() does the same under amt->lock. When both race on the same tunnel, the second list_del_rcu() hits LIST_POISON2 and panics the kernel. An unprivileged user can trigger this inside its own user/network namespace. list_del corruption, prev is LIST_POISON2 (dead000000000122) kernel BUG at lib/list_debug.c:59! RIP: 0010:__list_del_entry_valid_or_report+0x13a/0x200 Call Trace: amt_dev_stop+0x2c3/0x500 (drivers/net/amt.c:3097) __dev_close_many+0x17e/0x470 unregister_netdevice_many_notify+0x729/0x1f00 Fix: 1. Quiesce RX: clear sk_user_data and call synchronize_net() to ensure no RCU readers are traversing tunnel_list. This makes list_del_init() safe (it is not RCU-reader-safe unlike list_del_rcu()). 2. Hold amt->lock when unlinking tunnels in stop, using list_del_init() so amt_tunnel_expire() can detect already-claimed tunnels via list_empty() and skip them. 3. Use while/list_first_entry instead of list_for_each_entry_safe, because cancel_delayed_work_sync() can sleep and the cached next pointer may become stale. 4. Use disable_delayed_work_sync() instead of cancel_delayed_work_sync() to prevent amt_update_handler() from rearming the GC timer in a rare race where a packet arrives before the socket is fully released. Fixes: cbc21dc1cfe9 ("amt: add data plane of amt interface") Reported-by: AutonomousCodeSecurity@microsoft.com Reported-by: Xiang Mei (Microsoft) Reported-by: Cen Zhang (Microsoft) Cc: stable@vger.kernel.org Signed-off-by: Cen Zhang (Microsoft) --- drivers/net/amt.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/drivers/net/amt.c b/drivers/net/amt.c index bddc24e1856d..a5db02d81291 100644 --- a/drivers/net/amt.c +++ b/drivers/net/amt.c @@ -1348,6 +1348,11 @@ static void amt_tunnel_expire(struct work_struct *work) struct amt_dev *amt = tunnel->amt; spin_lock_bh(&amt->lock); + /* amt_dev_stop() marks tunnels it owns with list_del_init(). */ + if (list_empty(&tunnel->list)) { + spin_unlock_bh(&amt->lock); + return; + } rcu_read_lock(); list_del_rcu(&tunnel->list); amt->nr_tunnels--; @@ -3068,7 +3073,7 @@ static int amt_dev_open(struct net_device *dev) static int amt_dev_stop(struct net_device *dev) { struct amt_dev *amt = netdev_priv(dev); - struct amt_tunnel_list *tunnel, *tmp; + struct amt_tunnel_list *tunnel; struct sk_buff *skb; struct sock *sk; int i; @@ -3077,9 +3082,11 @@ static int amt_dev_stop(struct net_device *dev) disable_delayed_work_sync(&amt->discovery_wq); cancel_delayed_work_sync(&amt->secret_wq); - /* shutdown */ + /* Quiesce RX path before tearing down tunnels. */ sk = rtnl_dereference(amt->sk); RCU_INIT_POINTER(amt->sk, NULL); + if (sk) + rcu_assign_sk_user_data(sk, NULL); synchronize_net(); if (sk) udp_tunnel_sock_release(sk); @@ -3097,13 +3104,21 @@ static int amt_dev_stop(struct net_device *dev) amt->req_cnt = 0; WRITE_ONCE(amt->remote_ip, 0); - list_for_each_entry_safe(tunnel, tmp, &amt->tunnel_list, list) { - list_del_rcu(&tunnel->list); + spin_lock_bh(&amt->lock); + while (!list_empty(&amt->tunnel_list)) { + tunnel = list_first_entry(&amt->tunnel_list, + struct amt_tunnel_list, list); + list_del_init(&tunnel->list); amt->nr_tunnels--; - cancel_delayed_work_sync(&tunnel->gc_wq); + spin_unlock_bh(&amt->lock); + + disable_delayed_work_sync(&tunnel->gc_wq); amt_clear_groups(tunnel); kfree_rcu(tunnel, rcu); + + spin_lock_bh(&amt->lock); } + spin_unlock_bh(&amt->lock); return 0; } -- 2.55.0