* [PATCH net v4 1/2] amt: send the relay's General Query directly from the receive path
2026-09-28 20:23 [PATCH net v4 0/2] amt: send the relay's General Query directly from the receive path Omar Ramadan
@ 2026-09-28 20:23 ` Omar Ramadan
2026-09-28 20:23 ` [PATCH net v4 2/2] selftests: net: amt: check that the relay's queries bypass the amt device Omar Ramadan
1 sibling, 0 replies; 3+ messages in thread
From: Omar Ramadan @ 2026-09-28 20:23 UTC (permalink / raw)
To: Taehee Yoo, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Shuah Khan, netdev, linux-kselftest, linux-kernel,
AutonomousCodeSecurity, Xiang Mei, tgopinath, kys, Cen Zhang
An skb queued in a qdisc can outlive the tunnel it references
through a raw pointer in skb->cb. For example, with igmp_qrv set
to 1 on the relay a tunnel lives for 135s, so a netem delay of
180s on the amt device outlives it; 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
amt_send_igmp_gq() and amt_send_mld_gq() are only called from
amt_request_handler(), inside the rcu_read_lock_bh() section of
amt_rcv(). amt_request_handler() already has the tunnel the query is
for: it found or created it inside that section. Queuing the query with
dev_queue_xmit() only leads back into amt_dev_xmit(), which strips the
Ethernet header and calls amt_send_membership_query() for that tunnel.
Make that call directly from the two senders instead, the same way
amt_send_advertisement() transmits from the receive path. The query
never waits in a qdisc, the tunnel is only dereferenced inside the
RCU section that found or created it, and nothing is stored in
skb->cb, so no lookup or refcount is needed. Remove the query branch
of amt_dev_xmit(), amt_skb_cb() and struct amt_skb_cb, which have no
users left.
Behaviour changes:
- The relay's own General Queries no longer pass through the amt
device's egress path: its qdisc, tc egress (clsact/tcx), the
netfilter egress hook and packet taps. They are still visible as
UDP on the underlay.
- A query that is sent successfully is no longer counted as
tx_dropped. The old query branch left through the unlock label,
which counted every sent query as dropped.
- A query that reaches amt_dev_xmit() on a relay from elsewhere,
such as a userspace querier, is now dropped at the IGMP/MLD type
switch. Before, it trusted whatever skb->cb held, and a NULL
tunnel hit the WARN_ON(1).
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 Security FORGE Labs) <cenzhang@linux.microsoft.com>
Signed-off-by: Omar Ramadan <omar@blockcast.net>
---
v4: no code change. Add a selftest (patch 2), as Taehee asked in the v2
thread:
https://lore.kernel.org/netdev/CAMArcTXsU+YbUzjF8BOLsVjL2L_Quasv54mdm8iiJN5QdoAPZA@mail.gmail.com/
v3: https://lore.kernel.org/netdev/20260928181601.85857-1-omar@blockcast.net/
v3 (Omar): send the GQ directly from amt_request_handler()'s RCU
section, instead of storing (ip4, source_port) in skb->cb and looking
the tunnel up again at dequeue. This also removes the per-query
tunnel_list walk that Taehee raised on v1, and the v2 window Sashiko
found in which a re-created tunnel could be matched before its nonce
and mac were written. Cen agreed in the v2 thread to go this way if
Taehee is fine with it.
v2: https://lore.kernel.org/netdev/20260922214150.13970-1-cenzhang@linux.microsoft.com/
v1: https://lore.kernel.org/netdev/20260818164825.63967-1-blbllhy@gmail.com/
Testing: Cen's KASAN reproducer, in which netem holds the General Query
for 160s past a 135s tunnel lifetime, reports the slab-use-after-free
in amt_dev_xmit() on net, and nothing with this patch or with v2
applied. tools/testing/selftests/net/amt.sh passes 5/5 with and without
this patch on a KASAN + lockdep kernel.
drivers/net/amt.c | 48 +++++++++++++++--------------------------------
include/net/amt.h | 4 ----
2 files changed, 15 insertions(+), 37 deletions(-)
diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index bddc24e18..b53f8ec55 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -80,15 +80,6 @@ static struct in6_addr mld2_all_node = MLD2_ALL_NODE_INIT;
static struct mld2_grec mldv2_zero_grec;
#endif
-static struct amt_skb_cb *amt_skb_cb(struct sk_buff *skb)
-{
- BUILD_BUG_ON(sizeof(struct amt_skb_cb) + sizeof(struct tc_skb_cb) >
- sizeof_field(struct sk_buff, cb));
-
- return (struct amt_skb_cb *)((void *)skb->cb +
- sizeof(struct tc_skb_cb));
-}
-
static void __amt_source_gc_work(void)
{
struct amt_source_node *snode;
@@ -791,6 +782,11 @@ static void amt_send_request(struct amt_dev *amt, bool v6)
rcu_read_unlock();
}
+static bool amt_send_membership_query(struct amt_dev *amt,
+ struct sk_buff *skb,
+ struct amt_tunnel_list *tunnel,
+ bool v6);
+
static void amt_send_igmp_gq(struct amt_dev *amt,
struct amt_tunnel_list *tunnel)
{
@@ -800,8 +796,11 @@ static void amt_send_igmp_gq(struct amt_dev *amt,
if (!skb)
return;
- amt_skb_cb(skb)->tunnel = tunnel;
- dev_queue_xmit(skb);
+ skb_pull(skb, sizeof(struct ethhdr));
+ if (amt_send_membership_query(amt, skb, tunnel, false)) {
+ amt->dev->stats.tx_dropped++;
+ kfree_skb(skb);
+ }
}
#if IS_ENABLED(CONFIG_IPV6)
@@ -885,8 +884,11 @@ static void amt_send_mld_gq(struct amt_dev *amt, struct amt_tunnel_list *tunnel)
if (!skb)
return;
- amt_skb_cb(skb)->tunnel = tunnel;
- dev_queue_xmit(skb);
+ skb_pull(skb, sizeof(struct ethhdr));
+ if (amt_send_membership_query(amt, skb, tunnel, true)) {
+ amt->dev->stats.tx_dropped++;
+ kfree_skb(skb);
+ }
}
#else
static void amt_send_mld_gq(struct amt_dev *amt, struct amt_tunnel_list *tunnel)
@@ -1186,7 +1188,6 @@ static netdev_tx_t amt_dev_xmit(struct sk_buff *skb, struct net_device *dev)
#endif
bool report = false;
struct igmphdr *ih;
- bool query = false;
struct iphdr *iph;
bool data = false;
bool v6 = false;
@@ -1204,9 +1205,6 @@ static netdev_tx_t amt_dev_xmit(struct sk_buff *skb, struct net_device *dev)
case IGMP_HOST_MEMBERSHIP_REPORT:
report = true;
break;
- case IGMP_HOST_MEMBERSHIP_QUERY:
- query = true;
- break;
default:
goto free;
}
@@ -1228,9 +1226,6 @@ static netdev_tx_t amt_dev_xmit(struct sk_buff *skb, struct net_device *dev)
case ICMPV6_MLD2_REPORT:
report = true;
break;
- case ICMPV6_MGM_QUERY:
- query = true;
- break;
default:
goto free;
}
@@ -1261,19 +1256,6 @@ static netdev_tx_t amt_dev_xmit(struct sk_buff *skb, struct net_device *dev)
goto free;
goto unlock;
} else if (amt->mode == AMT_MODE_RELAY) {
- if (query) {
- tunnel = amt_skb_cb(skb)->tunnel;
- if (!tunnel) {
- WARN_ON(1);
- goto free;
- }
-
- /* Do not forward unexpected query */
- if (amt_send_membership_query(amt, skb, tunnel, v6))
- goto free;
- goto unlock;
- }
-
if (!data)
goto free;
list_for_each_entry_rcu(tunnel, &amt->tunnel_list, list) {
diff --git a/include/net/amt.h b/include/net/amt.h
index a0255491f..2846dde0c 100644
--- a/include/net/amt.h
+++ b/include/net/amt.h
@@ -231,10 +231,6 @@ struct amt_relay_headers {
};
} __packed;
-struct amt_skb_cb {
- struct amt_tunnel_list *tunnel;
-};
-
struct amt_tunnel_list {
struct list_head list;
/* Protect All resources under an amt_tunne_list */
--
2.47.3
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH net v4 2/2] selftests: net: amt: check that the relay's queries bypass the amt device
2026-09-28 20:23 [PATCH net v4 0/2] amt: send the relay's General Query directly from the receive path Omar Ramadan
2026-09-28 20:23 ` [PATCH net v4 1/2] " Omar Ramadan
@ 2026-09-28 20:23 ` Omar Ramadan
1 sibling, 0 replies; 3+ messages in thread
From: Omar Ramadan @ 2026-09-28 20:23 UTC (permalink / raw)
To: Taehee Yoo, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, Shuah Khan, netdev, linux-kselftest, linux-kernel,
AutonomousCodeSecurity, Xiang Mei, tgopinath, kys, Cen Zhang
The relay used to hand its General Queries to dev_queue_xmit() on the
amt device, where a query could wait in a qdisc and outlive the tunnel
it pointed to. The previous patch sends them directly from the receive
path instead.
Count the IGMP and MLD queries that leave the relay through amtr with
tc flower filters on its egress, installed before the gateway comes
up, and check that there are none. The forwarding tests before it
already show that the gateway received its queries, since it cannot
join without one.
Without the previous patch the new test fails (one run counted 7 IGMP
and 6 MLD queries); with it, all of amt.sh passes.
Signed-off-by: Omar Ramadan <omar@blockcast.net>
---
tools/testing/selftests/net/amt.sh | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/tools/testing/selftests/net/amt.sh b/tools/testing/selftests/net/amt.sh
index 663744305..d13b20ccc 100755
--- a/tools/testing/selftests/net/amt.sh
+++ b/tools/testing/selftests/net/amt.sh
@@ -150,6 +150,13 @@ setup_interface()
ip netns exec "${RELAY}" ip a a 10.0.0.2/24 dev relay_gw
ip netns exec "${RELAY}" ip link add amtr type amt mode relay \
local 10.0.0.2 dev relay_gw relay_port 2268 max_tunnels 4
+ # Count the IGMP and MLD queries that leave the relay through its own
+ # amt device; test_query_egress expects none.
+ ip netns exec "${RELAY}" tc qdisc add dev amtr clsact
+ ip netns exec "${RELAY}" tc filter add dev amtr egress pref 1 \
+ protocol ip flower ip_proto 0x2 action pass
+ ip netns exec "${RELAY}" tc filter add dev amtr egress pref 2 \
+ protocol ipv6 flower ip_proto icmpv6 type 130 action pass
ip netns exec "${RELAY}" ip a a 172.17.0.1/24 dev relay_src
ip netns exec "${RELAY}" ip a a 2001:db8:3::1/64 dev relay_src
ip netns exec "${SOURCE}" ip a a 172.17.0.2/24 dev src_relay
@@ -246,6 +253,27 @@ test_ipv6_forward()
fi
}
+# The relay sends its General Queries straight from the receive path, in
+# the same context that found the tunnel. A query queued on the amt device
+# instead could outlive the tunnel it was built for. The forwarding tests
+# above show that the gateway got its queries.
+test_query_egress()
+{
+ local n4 n6
+
+ n4=$(ip netns exec "${RELAY}" tc -s -j filter show dev amtr egress \
+ pref 1 | jq '[.[].options.actions[0].stats.packets // empty] | add // 0')
+ n6=$(ip netns exec "${RELAY}" tc -s -j filter show dev amtr egress \
+ pref 2 | jq '[.[].options.actions[0].stats.packets // empty] | add // 0')
+ if [ "$n4" -eq 0 ] && [ "$n6" -eq 0 ]; then
+ printf "TEST: %-60s [ OK ]\n" "amt relay queries bypass the amt device"
+ else
+ printf "TEST: %-60s [FAIL]\n" "amt relay queries bypass the amt device"
+ echo "IGMP queries on amtr egress: $n4, MLD queries: $n6" >&2
+ ERR=1
+ fi
+}
+
send_mcast4()
{
sleep 5
@@ -287,6 +315,7 @@ wait $pid || err=$?
if [ $err -eq 1 ]; then
ERR=1
fi
+test_query_egress
printf "TEST: %-50s" "IPv4 amt traffic forwarding torture"
send_mcast_torture4
printf " [ OK ]\n"
--
2.47.3
^ permalink raw reply [flat|nested] 3+ messages in thread