Re: [PATCH] amt: don't read the IP source address from a reallocated skb header

[Taehee Yoo]

On Mon, Jun 15, 2026 at 12:56 AM Michael Bommarito
<michael.bommarito@xxxxxxxxx> 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@xxxxxxxxxxxxxxx # v5.16+
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>

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@xxxxxxxxx>

Thank you so much!
Taehee Yoo

> ---
> 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
>