Re: [PATCH] amt: don't read the IP source address from a reallocated skb header
From: Taehee Yoo <ap420073@gmail.com>
Date: 2026-06-15 15:13:43
Also in:
lkml
On Mon, Jun 15, 2026 at 12:56 AM Michael Bommarito [off-list ref] wrote:
Hi Michael, Thanks a lot for your work!
amt_update_handler() caches iph = ip_hdr(skb) and then calls
pskb_may_pull(). pskb_may_pull() can reallocate the skb head: the new
head is allocated and the old one is freed. The cached iph is not
refreshed, so the following tunnel lookup reads iph->saddr from the
freed head. On an AMT relay this lookup runs for every incoming
membership update, before the update's nonce and response MAC are
validated.
The sibling handlers amt_multicast_data_handler() and
amt_membership_query_handler() re-read ip_hdr() after the pull and are
not affected; only amt_update_handler() keeps the pre-pull pointer.
Snapshot the source address before the pulls and match against the
snapshot.
The stale read was confirmed by instrumentation rather than a sanitizer:
after the head is reallocated the comparison reads from the freed old
head. KASAN does not flag it because the skb head is released through
the page-fragment free path, which is not poisoned on free.
Fixes: cbc21dc1cfe9 ("amt: add data plane of amt interface")
Cc: stable@vger.kernel.org # v5.16+
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <redacted>Thanks for the fix. Please tag this as [PATCH net] in the subject, since it's a bug fix: [PATCH net] amt: don't read the IP source address from a reallocated skb header You can drop the Cc: stable. The Fixes: tag is enough for the stable backport process to pick it up. Please send a v2 with the above changes. You can carry my ack: Acked-by: Taehee Yoo <ap420073@gmail.com> Thank you so much! Taehee Yoo
quoted hunk ↗ jump to hunk
--- Confirmed on x86_64 by instrumenting the comparison: with the update packet built so the first pskb_may_pull() reallocates the head (it pulls bytes out of a page fragment with no tailroom), the read runs against the freed old head -- the head pointer moves and the old page's refcount is 0. Neither generic KASAN nor arm64 HW-tag KASAN reports it: page- fragment frees are not synchronously poisoned, and under MTE the freed page keeps a tag matching the stale pointer, so this class of stale- header read escapes the usual fuzzing oracles. On a live relay the freed head is also exposed to reuse by later skb allocations. amtdbg: cmp reads iph=...e000 (skb->head=...384380) stale_head=1 ref=0 A KUnit covering the re-read can follow separately. drivers/net/amt.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)diff --git a/drivers/net/amt.c b/drivers/net/amt.c index f2f3139..af6e28d 100644 --- a/drivers/net/amt.c +++ b/drivers/net/amt.c@@ -2455,8 +2455,10 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb) struct ethhdr *eth; struct iphdr *iph; int len, hdr_size; + __be32 saddr; iph = ip_hdr(skb); + saddr = iph->saddr; hdr_size = sizeof(*amtmu) + sizeof(struct udphdr); if (!pskb_may_pull(skb, hdr_size))@@ -2472,7 +2474,7 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb) skb_reset_network_header(skb); list_for_each_entry_rcu(tunnel, &amt->tunnel_list, list) { - if (tunnel->ip4 == iph->saddr) { + if (tunnel->ip4 == saddr) { if ((amtmu->nonce == tunnel->nonce && amtmu->response_mac == tunnel->mac)) { mod_delayed_work(amt_wq, &tunnel->gc_wq,base-commit: 5200f5f493f79f14bbdc349e402a40dfb32f23c8 -- 2.53.0