[PATCH net] amt: do not store tunnel pointer in skb control block

[PATCH net] amt: do not store tunnel pointer in skb control block

[Cen Zhang (Microsoft)]

An skb queued in a qdisc can outlive the tunnel it references
through a raw pointer in skb->cb. For example, a netem delay of
180s exceeds the default tunnel lifetime of 135s (igmp_qrv=1);
when the tunnel expires and is freed, the subsequent dequeue
triggers a use-after-free in amt_dev_xmit().

  BUG: KASAN: slab-use-after-free in amt_dev_xmit+0x2763/0x2e20
  Call Trace:
   amt_dev_xmit+0x2763/0x2e20 [drivers/net/amt.c:1262]
   dev_hard_start_xmit+0x22f/0x620
   sch_direct_xmit+0x12e/0xac0
   netem_dequeue+0x333/0xc50
   net_tx_action+0x35c/0xa60

Store the tunnel identity (ip4 + source_port) in skb->cb instead
of a pointer, and re-lookup the tunnel under RCU in amt_dev_xmit().
If the tunnel is gone, the query is simply dropped.

Fixes: cbc21dc1cfe9 ("amt: add data plane of amt interface")
Reported-by: AutonomousCodeSecurity@microsoft.com
Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
Reported-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
---
 drivers/net/amt.c | 30 +++++++++++++++++++++++++-----
 include/net/amt.h |  4 +++-
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index 182a41d59a75..a85ba0dfe18a 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -789,6 +789,18 @@ static void amt_send_request(struct amt_dev *amt, bool v6)
 	rcu_read_unlock();
 }
 
+static struct amt_tunnel_list *amt_lookup_tunnel(struct amt_dev *amt,
+						 __be32 ip4, __be16 source_port)
+{
+	struct amt_tunnel_list *tunnel;
+
+	list_for_each_entry_rcu(tunnel, &amt->tunnel_list, list)
+		if (tunnel->ip4 == ip4 && tunnel->source_port == source_port)
+			return tunnel;
+
+	return NULL;
+}
+
 static void amt_send_igmp_gq(struct amt_dev *amt,
 			     struct amt_tunnel_list *tunnel)
 {
@@ -798,7 +810,8 @@ static void amt_send_igmp_gq(struct amt_dev *amt,
 	if (!skb)
 		return;
 
-	amt_skb_cb(skb)->tunnel = tunnel;
+	amt_skb_cb(skb)->tunnel_ip4 = tunnel->ip4;
+	amt_skb_cb(skb)->tunnel_port = tunnel->source_port;
 	dev_queue_xmit(skb);
 }
 
@@ -883,7 +896,8 @@ static void amt_send_mld_gq(struct amt_dev *amt, struct amt_tunnel_list *tunnel)
 	if (!skb)
 		return;
 
-	amt_skb_cb(skb)->tunnel = tunnel;
+	amt_skb_cb(skb)->tunnel_ip4 = tunnel->ip4;
+	amt_skb_cb(skb)->tunnel_port = tunnel->source_port;
 	dev_queue_xmit(skb);
 }
 #else
@@ -1259,15 +1273,21 @@ static netdev_tx_t amt_dev_xmit(struct sk_buff *skb, struct net_device *dev)
 		goto unlock;
 	} else if (amt->mode == AMT_MODE_RELAY) {
 		if (query) {
-			tunnel = amt_skb_cb(skb)->tunnel;
+			rcu_read_lock();
+			tunnel = amt_lookup_tunnel(amt,
+						   amt_skb_cb(skb)->tunnel_ip4,
+						   amt_skb_cb(skb)->tunnel_port);
 			if (!tunnel) {
-				WARN_ON(1);
+				rcu_read_unlock();
 				goto free;
 			}
 
 			/* Do not forward unexpected query */
-			if (amt_send_membership_query(amt, skb, tunnel, v6))
+			if (amt_send_membership_query(amt, skb, tunnel, v6)) {
+				rcu_read_unlock();
 				goto free;
+			}
+			rcu_read_unlock();
 			goto unlock;
 		}
 
diff --git a/include/net/amt.h b/include/net/amt.h
index a0255491f5b0..59c4bb88fb1e 100644
--- a/include/net/amt.h
+++ b/include/net/amt.h
@@ -231,8 +231,10 @@ struct amt_relay_headers {
 	};
 } __packed;
 
+/* Tunnel identity for re-lookup; do not store a pointer here. */
 struct amt_skb_cb {
-	struct amt_tunnel_list *tunnel;
+	__be32			tunnel_ip4;
+	__be16			tunnel_port;
 };
 
 struct amt_tunnel_list {
-- 
2.52.0