Skip to contentSkip to conversation

xfrm: hold skb->dev across async IPv6 transport reinject

PATCH New Qi Tang 1 participant 1 message Mar 19

Subsystems (3)
lore
Share & tools
lore

Qi Tang

author
#0 lore raw

PATCH 1/1 xfrm: hold skb->dev across async IPv6 transport reinject

xfrm_trans_queue() queues transport-mode packets for async reinject via xfrm_trans_reinject(). The queued skb keeps only a bare skb->dev pointer. If the originating netns is torn down before the workqueue callback runs, ip6_rcv_finish() can still dereference skb->dev after the device has already been released by default_device_exit_batch().

Fix this by taking a netdev reference when queueing the skb and dropping it after the reinject callback completes.

This was reproduced with KASAN under QEMU:

BUG: KASAN: slab-use-after-free in ip6_rcv_finish+0x17c/0x1b0
Workqueue: events xfrm_trans_reinject
Call Trace:
 ip6_rcv_finish+0x17c/0x1b0
 xfrm_trans_reinject+0x292/0x440
 process_one_work+0x63c/0x1100
 worker_thread+0x62d/0xef0
 kthread+0x368/0x480
 ret_from_fork+0x529/0x750

Allocated by task 112:
 alloc_netdev_mqs+0x82/0x1180
 rtnl_create_link+0xaa4/0xe30
 rtnl_newlink+0xa98/0x1f90

Freed by task 12:
 device_release+0x9b/0x210
 netdev_run_todo+0x497/0xcf0
 default_device_exit_batch+0x735/0xab0
 cleanup_net+0x3c7/0x860

The issue can be reproduced by repeatedly:
- creating an IPv6 veth pair and network namespace,
- installing IPv6 transport-mode XFRM state and policy,
- sending IPv6 traffic to queue async reinject work, and
- tearing the namespace and device down in parallel.

When unprivileged user namespaces are enabled, this can be triggered by a non-root user after entering its own user/net namespace with unshare -Urn.

Fixes acf568ee859f ("xfrm: Reinject transport-mode packets through tasklet")
---
 net/xfrm/xfrm_input.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)
net/xfrm/xfrm_input.c +10 -3
index 4ed346e682c7..4b5147cb44b7 100644
@@ -40,6 +40,7 @@ struct xfrm_trans_cb {
 	} header;
 	int (*finish)(struct net *net, struct sock *sk, struct sk_buff *skb);
 	struct net *net;
+	struct net_device *dev;
 };
 
 #define XFRM_TRANS_SKB_CB(__skb) ((struct xfrm_trans_cb *)&((__skb)->cb[0]))
@@ -784,9 +785,13 @@ static void xfrm_trans_reinject(struct work_struct *work)
 	spin_unlock_bh(&trans->queue_lock);
 
 	local_bh_disable();
-	while ((skb = __skb_dequeue(&queue)))
-		XFRM_TRANS_SKB_CB(skb)->finish(XFRM_TRANS_SKB_CB(skb)->net,
-					       NULL, skb);
+	while ((skb = __skb_dequeue(&queue))) {
+		struct xfrm_trans_cb *cb = XFRM_TRANS_SKB_CB(skb);
+		struct net_device *dev = cb->dev;
+
+		cb->finish(cb->net, NULL, skb);
+		dev_put(dev);
+	}
 	local_bh_enable();
 }
 
@@ -805,6 +810,8 @@ int xfrm_trans_queue_net(struct net *net, struct sk_buff *skb,
 
 	XFRM_TRANS_SKB_CB(skb)->finish = finish;
 	XFRM_TRANS_SKB_CB(skb)->net = net;
+	XFRM_TRANS_SKB_CB(skb)->dev = skb->dev;
+	dev_hold(XFRM_TRANS_SKB_CB(skb)->dev);
 	spin_lock_bh(&trans->queue_lock);
 	__skb_queue_tail(&trans->queue, skb);
 	spin_unlock_bh(&trans->queue_lock);
Signature · 1 line
2.43.0