[PATCH net] amt: fix tunnel list corruption on device stop

"Cen Zhang (Microsoft)" <[email protected]>
Newsgroups org.kernel.vger.netdev,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <[email protected]>
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: [email protected]
Reported-by: Xiang Mei (Microsoft) <[email protected]>
Reported-by: Cen Zhang (Microsoft) <[email protected]>
Cc: [email protected]
Signed-off-by: Cen Zhang (Microsoft) <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.