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

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

[Michael Bommarito]

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")
Acked-by: Taehee Yoo <ap420073@gmail.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
v2: per Taehee Yoo's review
    (https://lore.kernel.org/all/CAMArcTWCg4x1bxrzr+XHc_FqbzJELCMu+tE=x8Jhewgr-_A3Rw@mail.gmail.com/):
    - retag the subject as [PATCH net] (this is a bug fix);
    - drop Cc: stable -- the Fixes tag is enough for the stable backport
      process to pick it up;
    - carry Taehee Yoo's Acked-by.
    No code change from v1.
    v1: https://lore.kernel.org/all/20260614155539.3106537-1-michael.bommarito@gmail.com/

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

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

[Jakub Kicinski]

On Wed, 17 Jun 2026 08:34:43 -0400 Michael Bommarito wrote:
> 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.

Sashikos point out a bunch more of these in AMT:
https://sashiko.dev/#/patchset/20260617123443.3586930-1-michael.bommarito@gmail.com
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260617123443.3586930-1-michael.bommarito@gmail.com

Let's fix them all with one patch?
-- 
pw-bot: cr

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

[Taehee Yoo]

On Mon, Jun 22, 2026 at 7:00 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 17 Jun 2026 08:34:43 -0400 Michael Bommarito wrote:
> > 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.
>
> Sashikos point out a bunch more of these in AMT:
> https://sashiko.dev/#/patchset/20260617123443.3586930-1-michael.bommarito@gmail.com
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260617123443.3586930-1-michael.bommarito@gmail.com
>
> Let's fix them all with one patch?

Agreed.
Michael, could you please fix the remaining ones Sashiko flagged?

Thanks a lot!
Taehee Yoo

> --
> pw-bot: cr

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

[Michael Bommarito]

On Mon, Jun 22, 2026 at 4:58 AM Taehee Yoo <ap420073@gmail.com> wrote:
> > Let's fix them all with one patch?
>
> Agreed.
> Michael, could you please fix the remaining ones Sashiko flagged?

Sure, will do

Thanks,
Mike

[PATCH net v3] amt: don't read the IP header from a reallocated skb head

[Michael Bommarito]

Several AMT receive paths cache a pointer into the skb header
(ip_hdr() / ipv6_hdr()) and then call a helper that can reallocate
the skb head before reading the cached pointer:

  amt_rcv()                 caches ip_hdr(skb), then amt_parse_type()
                            does pskb_may_pull() before reading
                            iph->saddr;
  amt_update_handler()      caches ip_hdr(skb), then pskb_may_pull() /
                            iptunnel_pull_header() before reading
                            iph->saddr;
  amt_igmpv3_report_handler() caches ip_hdr(skb), then ip_mc_may_pull()
                            in the record loop before reading
                            iph->saddr;
  amt_mldv2_report_handler() caches ipv6_hdr(skb), then
                            ipv6_mc_may_pull() in the record loop
                            before reading ip6h->saddr.

pskb_may_pull() and the *_mc_may_pull() helpers can reallocate the
skb head; when they do, the old head is freed and the cached pointer
dangles, so the later source-address read is a use-after-free of the
freed head.

The only field used after the pull in each case is the source
address, which is stable across the pull. Snapshot it before the
pull and use the snapshot.

The sibling handlers that re-derive ip_hdr() after the pull
(amt_multicast_data_handler(), amt_membership_query_handler()) and
the handlers that read the source address with no intervening pull
(amt_discovery_handler(), amt_request_handler(), the IGMPv2/MLDv1
report and leave handlers) are not affected.

Fixes: cbc21dc1cfe9 ("amt: add data plane of amt interface")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
v3: address Jakub Kicinski's and Taehee Yoo's review of v2: fix all of
    the stale-iph reads Sashiko flagged in amt.c in one patch, not just
    amt_update_handler(). v2 fixed only amt_update_handler(); this also
    covers amt_rcv(), amt_igmpv3_report_handler() and
    amt_mldv2_report_handler(), which have the same pre-pull cached
    header read. The amt_update_handler() change is functionally the
    same as v2 (snapshot the source address before the pull).
    v2: https://lore.kernel.org/all/20260617123443.3586930-1-michael.bommarito@gmail.com/

Built for x86_64 (CONFIG_AMT=m) with W=1, no new warnings; checkpatch
--strict clean.

 drivers/net/amt.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index 724a8163a5142..7094bdab0f463 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -2000,13 +2000,15 @@ static void amt_igmpv3_report_handler(struct amt_dev *amt, struct sk_buff *skb,
 	struct igmpv3_report *ihrv3 = igmpv3_report_hdr(skb);
 	int len = skb_transport_offset(skb) + sizeof(*ihrv3);
 	void *zero_grec = (void *)&igmpv3_zero_grec;
-	struct iphdr *iph = ip_hdr(skb);
 	struct amt_group_node *gnode;
 	union amt_addr group, host;
 	struct igmpv3_grec *grec;
+	__be32 saddr;
 	u16 nsrcs;
 	int i;
 
+	saddr = ip_hdr(skb)->saddr;
+
 	for (i = 0; i < ntohs(ihrv3->ngrec); i++) {
 		len += sizeof(*grec);
 		if (!ip_mc_may_pull(skb, len))
@@ -2022,7 +2024,7 @@ static void amt_igmpv3_report_handler(struct amt_dev *amt, struct sk_buff *skb,
 		memset(&group, 0, sizeof(union amt_addr));
 		group.ip4 = grec->grec_mca;
 		memset(&host, 0, sizeof(union amt_addr));
-		host.ip4 = iph->saddr;
+		host.ip4 = saddr;
 		gnode = amt_lookup_group(tunnel, &group, &host, false);
 		if (!gnode) {
 			gnode = amt_add_group(amt, tunnel, &group, &host,
@@ -2162,13 +2164,15 @@ static void amt_mldv2_report_handler(struct amt_dev *amt, struct sk_buff *skb,
 	struct mld2_report *mld2r = (struct mld2_report *)icmp6_hdr(skb);
 	int len = skb_transport_offset(skb) + sizeof(*mld2r);
 	void *zero_grec = (void *)&mldv2_zero_grec;
-	struct ipv6hdr *ip6h = ipv6_hdr(skb);
 	struct amt_group_node *gnode;
 	union amt_addr group, host;
 	struct mld2_grec *grec;
+	struct in6_addr saddr;
 	u16 nsrcs;
 	int i;
 
+	saddr = ipv6_hdr(skb)->saddr;
+
 	for (i = 0; i < ntohs(mld2r->mld2r_ngrec); i++) {
 		len += sizeof(*grec);
 		if (!ipv6_mc_may_pull(skb, len))
@@ -2184,7 +2188,7 @@ static void amt_mldv2_report_handler(struct amt_dev *amt, struct sk_buff *skb,
 		memset(&group, 0, sizeof(union amt_addr));
 		group.ip6 = grec->grec_mca;
 		memset(&host, 0, sizeof(union amt_addr));
-		host.ip6 = ip6h->saddr;
+		host.ip6 = saddr;
 		gnode = amt_lookup_group(tunnel, &group, &host, true);
 		if (!gnode) {
 			gnode = amt_add_group(amt, tunnel, &group, &host,
@@ -2455,8 +2459,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 +2478,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,
@@ -2772,7 +2778,7 @@ static void amt_gw_rcv(struct amt_dev *amt, struct sk_buff *skb)
 static int amt_rcv(struct sock *sk, struct sk_buff *skb)
 {
 	struct amt_dev *amt;
-	struct iphdr *iph;
+	__be32 saddr;
 	int type;
 	bool err;
 
@@ -2785,7 +2791,7 @@ static int amt_rcv(struct sock *sk, struct sk_buff *skb)
 	}
 
 	skb->dev = amt->dev;
-	iph = ip_hdr(skb);
+	saddr = ip_hdr(skb)->saddr;
 	type = amt_parse_type(skb);
 	if (type == -1) {
 		err = true;
@@ -2795,7 +2801,7 @@ static int amt_rcv(struct sock *sk, struct sk_buff *skb)
 	if (amt->mode == AMT_MODE_GATEWAY) {
 		switch (type) {
 		case AMT_MSG_ADVERTISEMENT:
-			if (iph->saddr != amt->discovery_ip) {
+			if (saddr != amt->discovery_ip) {
 				netdev_dbg(amt->dev, "Invalid Relay IP\n");
 				err = true;
 				goto drop;
@@ -2807,7 +2813,7 @@ static int amt_rcv(struct sock *sk, struct sk_buff *skb)
 			}
 			goto out;
 		case AMT_MSG_MULTICAST_DATA:
-			if (iph->saddr != amt->remote_ip) {
+			if (saddr != amt->remote_ip) {
 				netdev_dbg(amt->dev, "Invalid Relay IP\n");
 				err = true;
 				goto drop;
@@ -2818,7 +2824,7 @@ static int amt_rcv(struct sock *sk, struct sk_buff *skb)
 			else
 				goto out;
 		case AMT_MSG_MEMBERSHIP_QUERY:
-			if (iph->saddr != amt->remote_ip) {
+			if (saddr != amt->remote_ip) {
 				netdev_dbg(amt->dev, "Invalid Relay IP\n");
 				err = true;
 				goto drop;

base-commit: ab9de95c9cf952332ab79453b4b5d1bfca8e514f
-- 
2.53.0