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) Reported-by: Cen Zhang (Microsoft) Signed-off-by: Cen Zhang (Microsoft) --- 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