* [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls
@ 2026-07-11 15:19 Michael Bommarito
2026-07-11 15:19 ` [PATCH net v5 1/2] amt: re-read skb header pointers after every pull Michael Bommarito
` (5 more replies)
0 siblings, 6 replies; 8+ messages in thread
From: Michael Bommarito @ 2026-07-11 15:19 UTC (permalink / raw)
To: Taehee Yoo, Andrew Lunn, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel
Several AMT receive and transmit paths cache a pointer into the skb head
and then call a helper that can reallocate that head before the cached
pointer is used again, so the later access reads or writes freed memory.
Patch 1 walks every AMT path and, for each pointer used after a
reallocating call, either snapshots the value before the first pull or
re-derives the pointer after the last one.
Patch 2 is a smaller, separable hardening change: the three handlers
that rewrite the ethernet header do so in place without making the head
private, which corrupts a cloned skb (for example one held by a packet
tap). It adds skb_cow_head() before the rewrite, split out so the
use-after-free fix is not held up by discussion of the clone case.
Both patches build cleanly (x86_64, CONFIG_AMT, W=1) and are
checkpatch --strict clean.
Changes since v4:
- amt_update_handler(): also snapshot amtmu->nonce and
amtmu->response_mac before iptunnel_pull_header(), which can
reallocate the head for a GSO cloned skb; the tunnel-match loop read
both fields through the stale amtmu. This is the same class as the
query handler's response_mac snapshot and was the one remaining site
the v4 fix missed.
- Remove the explanatory comments added in v4; the reason for each
snapshot/re-derive is described in the commit message instead.
- Order the local variable declarations longest-to-shortest in the
handlers that gained locals (amt_membership_query_handler and
amt_update_handler).
v4: https://lore.kernel.org/all/20260707193243.3448201-1-michael.bommarito@gmail.com/
v3: https://lore.kernel.org/all/20260626111917.802243-1-michael.bommarito@gmail.com/
v2: https://lore.kernel.org/all/20260617123443.3586930-1-michael.bommarito@gmail.com/
Michael Bommarito (2):
amt: re-read skb header pointers after every pull
amt: make the head writable before rewriting the L2 header
drivers/net/amt.c | 87 ++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 63 insertions(+), 24 deletions(-)
base-commit: 2c7c88a412aa6d09cd04b414211b4ef8553b5309
--
2.53.0
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net v5 1/2] amt: re-read skb header pointers after every pull 2026-07-11 15:19 [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls Michael Bommarito @ 2026-07-11 15:19 ` Michael Bommarito 2026-07-11 15:19 ` [PATCH net v5 2/2] amt: make the head writable before rewriting the L2 header Michael Bommarito ` (4 subsequent siblings) 5 siblings, 0 replies; 8+ messages in thread From: Michael Bommarito @ 2026-07-11 15:19 UTC (permalink / raw) To: Taehee Yoo, Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni Cc: netdev, linux-kernel Several AMT receive and transmit paths cache a pointer into the skb head (ip_hdr(), ipv6_hdr(), eth_hdr() or the AMT message header) and then call a helper that can reallocate that head before the cached pointer is used again. pskb_may_pull(), ip_mc_may_pull(), ipv6_mc_may_pull(), iptunnel_pull_header(), ip_mc_check_igmp() and ipv6_mc_check_mld() can all free the old head and move the data, so a pointer taken before the call dangles afterwards and the later access is a use-after-free of the freed head. The affected sites are: amt_rcv() caches ip_hdr() before amt_parse_type() pulls, then reads iph->saddr. amt_dev_xmit() caches ip_hdr()/ipv6_hdr() before ip_mc_check_igmp()/ ipv6_mc_check_mld() and pskb_may_pull(), then reads the group address. amt_multicast_data_handler() caches eth_hdr() before pskb_may_pull(), then writes the L2 header. amt_membership_query_handler() caches the AMT header, the outer and inner eth_hdr() and ip_hdr() before iptunnel_pull_header() and several pulls, then reads and writes them. amt_igmpv3_report_handler() and amt_mldv2_report_handler() cache ip_hdr()/ipv6_hdr() and the current group record and read the record count from the report header inside the record loop, across the *_mc_may_pull() calls. amt_update_handler() caches ip_hdr() and the AMT membership-update header before pskb_may_pull(), iptunnel_pull_header(), ip_mc_check_igmp() and the report handler, then reads iph->daddr and amtmu->nonce / amtmu->response_mac. Fix each site by either snapshotting the scalar that is used after the pull before the first pull runs, or re-deriving the header pointer from the skb after the last pull that can move the head. Values that are stable across the pull (source and group address, the response MAC and nonce, the record count, the outer source MAC) are snapshotted; pointers that are written through or read repeatedly are re-derived. 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> --- drivers/net/amt.c | 79 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 55 insertions(+), 24 deletions(-) diff --git a/drivers/net/amt.c b/drivers/net/amt.c index 951dd10e192b7..35e77af76bd9d 100644 --- a/drivers/net/amt.c +++ b/drivers/net/amt.c @@ -1211,7 +1211,7 @@ static netdev_tx_t amt_dev_xmit(struct sk_buff *skb, struct net_device *dev) data = true; } v6 = false; - group.ip4 = iph->daddr; + group.ip4 = ip_hdr(skb)->daddr; #if IS_ENABLED(CONFIG_IPV6) } else if (iph->version == 6) { ip6h = ipv6_hdr(skb); @@ -1235,7 +1235,7 @@ static netdev_tx_t amt_dev_xmit(struct sk_buff *skb, struct net_device *dev) data = true; } v6 = true; - group.ip6 = ip6h->daddr; + group.ip6 = ipv6_hdr(skb)->daddr; #endif } else { dev->stats.tx_errors++; @@ -1278,12 +1278,12 @@ static netdev_tx_t amt_dev_xmit(struct sk_buff *skb, struct net_device *dev) hlist_for_each_entry_rcu(gnode, &tunnel->groups[hash], node) { if (!v6) { - if (gnode->group_addr.ip4 == iph->daddr) + if (gnode->group_addr.ip4 == group.ip4) goto found; #if IS_ENABLED(CONFIG_IPV6) } else { if (ipv6_addr_equal(&gnode->group_addr.ip6, - &ip6h->daddr)) + &group.ip6)) goto found; #endif } @@ -2000,14 +2000,18 @@ 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; + u16 ngrec; int i; - for (i = 0; i < ntohs(ihrv3->ngrec); i++) { + saddr = ip_hdr(skb)->saddr; + ngrec = ntohs(ihrv3->ngrec); + + for (i = 0; i < ngrec; i++) { len += sizeof(*grec); if (!ip_mc_may_pull(skb, len)) break; @@ -2019,10 +2023,13 @@ static void amt_igmpv3_report_handler(struct amt_dev *amt, struct sk_buff *skb, if (!ip_mc_may_pull(skb, len)) break; + grec = (void *)(skb->data + len - sizeof(*grec) - + nsrcs * sizeof(__be32)); + 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,14 +2169,18 @@ 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; + u16 ngrec; int i; - for (i = 0; i < ntohs(mld2r->mld2r_ngrec); i++) { + saddr = ipv6_hdr(skb)->saddr; + ngrec = ntohs(mld2r->mld2r_ngrec); + + for (i = 0; i < ngrec; i++) { len += sizeof(*grec); if (!ipv6_mc_may_pull(skb, len)) break; @@ -2181,10 +2192,13 @@ static void amt_mldv2_report_handler(struct amt_dev *amt, struct sk_buff *skb, if (!ipv6_mc_may_pull(skb, len)) break; + grec = (void *)(skb->data + len - sizeof(*grec) - + nsrcs * sizeof(struct in6_addr)); + 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, @@ -2305,7 +2319,6 @@ static bool amt_multicast_data_handler(struct amt_dev *amt, struct sk_buff *skb) skb_push(skb, sizeof(*eth)); skb_reset_mac_header(skb); skb_pull(skb, sizeof(*eth)); - eth = eth_hdr(skb); if (!pskb_may_pull(skb, sizeof(*iph))) return true; @@ -2315,6 +2328,7 @@ static bool amt_multicast_data_handler(struct amt_dev *amt, struct sk_buff *skb) if (!ipv4_is_multicast(iph->daddr)) return true; skb->protocol = htons(ETH_P_IP); + eth = eth_hdr(skb); eth->h_proto = htons(ETH_P_IP); ip_eth_mc_map(iph->daddr, eth->h_dest); #if IS_ENABLED(CONFIG_IPV6) @@ -2328,6 +2342,7 @@ static bool amt_multicast_data_handler(struct amt_dev *amt, struct sk_buff *skb) if (!ipv6_addr_is_multicast(&ip6h->daddr)) return true; skb->protocol = htons(ETH_P_IPV6); + eth = eth_hdr(skb); eth->h_proto = htons(ETH_P_IPV6); ipv6_eth_mc_map(&ip6h->daddr, eth->h_dest); #endif @@ -2351,10 +2366,12 @@ static bool amt_membership_query_handler(struct amt_dev *amt, struct sk_buff *skb) { struct amt_header_membership_query *amtmq; - struct igmpv3_query *ihv3; struct ethhdr *eth, *oeth; + struct igmpv3_query *ihv3; + u8 h_source[ETH_ALEN]; struct iphdr *iph; int hdr_size, len; + u64 response_mac; hdr_size = sizeof(*amtmq) + sizeof(struct udphdr); if (!pskb_may_pull(skb, hdr_size)) @@ -2367,6 +2384,8 @@ static bool amt_membership_query_handler(struct amt_dev *amt, if (amtmq->nonce != amt->nonce) return true; + response_mac = amtmq->response_mac; + hdr_size -= sizeof(*eth); if (iptunnel_pull_header(skb, hdr_size, htons(ETH_P_TEB), false)) return true; @@ -2376,6 +2395,7 @@ static bool amt_membership_query_handler(struct amt_dev *amt, skb_pull(skb, sizeof(*eth)); skb_reset_network_header(skb); eth = eth_hdr(skb); + ether_addr_copy(h_source, oeth->h_source); if (!pskb_may_pull(skb, sizeof(*iph))) return true; @@ -2388,6 +2408,7 @@ static bool amt_membership_query_handler(struct amt_dev *amt, sizeof(*ihv3))) return true; + iph = ip_hdr(skb); if (!ipv4_is_multicast(iph->daddr)) return true; @@ -2395,10 +2416,11 @@ static bool amt_membership_query_handler(struct amt_dev *amt, skb_reset_transport_header(skb); skb_push(skb, sizeof(*iph) + AMT_IPHDR_OPTS); WRITE_ONCE(amt->ready4, true); - amt->mac = amtmq->response_mac; + amt->mac = response_mac; amt->req_cnt = 0; amt->qi = ihv3->qqic; skb->protocol = htons(ETH_P_IP); + eth = eth_hdr(skb); eth->h_proto = htons(ETH_P_IP); ip_eth_mc_map(iph->daddr, eth->h_dest); #if IS_ENABLED(CONFIG_IPV6) @@ -2421,10 +2443,11 @@ static bool amt_membership_query_handler(struct amt_dev *amt, skb_reset_transport_header(skb); skb_push(skb, sizeof(*ip6h) + AMT_IP6HDR_OPTS); WRITE_ONCE(amt->ready6, true); - amt->mac = amtmq->response_mac; + amt->mac = response_mac; amt->req_cnt = 0; amt->qi = mld2q->mld2q_qqic; skb->protocol = htons(ETH_P_IPV6); + eth = eth_hdr(skb); eth->h_proto = htons(ETH_P_IPV6); ipv6_eth_mc_map(&ip6h->daddr, eth->h_dest); #endif @@ -2432,7 +2455,7 @@ static bool amt_membership_query_handler(struct amt_dev *amt, return true; } - ether_addr_copy(eth->h_source, oeth->h_source); + ether_addr_copy(eth->h_source, h_source); skb->pkt_type = PACKET_MULTICAST; skb->ip_summed = CHECKSUM_NONE; len = skb->len; @@ -2455,8 +2478,11 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb) struct ethhdr *eth; struct iphdr *iph; int len, hdr_size; + u64 response_mac; + __be32 saddr; + __be32 nonce; - iph = ip_hdr(skb); + saddr = ip_hdr(skb)->saddr; hdr_size = sizeof(*amtmu) + sizeof(struct udphdr); if (!pskb_may_pull(skb, hdr_size)) @@ -2466,15 +2492,18 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb) if (amtmu->reserved || amtmu->version) return true; + nonce = amtmu->nonce; + response_mac = amtmu->response_mac; + if (iptunnel_pull_header(skb, hdr_size, skb->protocol, false)) return true; skb_reset_network_header(skb); list_for_each_entry_rcu(tunnel, &amt->tunnel_list, list) { - if (tunnel->ip4 == iph->saddr) { - if ((amtmu->nonce == tunnel->nonce && - amtmu->response_mac == tunnel->mac)) { + if (tunnel->ip4 == saddr) { + if ((nonce == tunnel->nonce && + response_mac == tunnel->mac)) { mod_delayed_work(amt_wq, &tunnel->gc_wq, msecs_to_jiffies(amt_gmi(amt)) * 3); @@ -2508,6 +2537,7 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb) eth = eth_hdr(skb); skb->protocol = htons(ETH_P_IP); eth->h_proto = htons(ETH_P_IP); + iph = ip_hdr(skb); ip_eth_mc_map(iph->daddr, eth->h_dest); #if IS_ENABLED(CONFIG_IPV6) } else if (iph->version == 6) { @@ -2527,6 +2557,7 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb) eth = eth_hdr(skb); skb->protocol = htons(ETH_P_IPV6); eth->h_proto = htons(ETH_P_IPV6); + ip6h = ipv6_hdr(skb); ipv6_eth_mc_map(&ip6h->daddr, eth->h_dest); #endif } else { @@ -2772,7 +2803,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 +2816,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 +2826,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 +2838,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 +2849,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: 2c7c88a412aa6d09cd04b414211b4ef8553b5309 -- 2.53.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH net v5 2/2] amt: make the head writable before rewriting the L2 header 2026-07-11 15:19 [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls Michael Bommarito 2026-07-11 15:19 ` [PATCH net v5 1/2] amt: re-read skb header pointers after every pull Michael Bommarito @ 2026-07-11 15:19 ` Michael Bommarito 2026-07-20 11:06 ` [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls Simon Horman ` (3 subsequent siblings) 5 siblings, 0 replies; 8+ messages in thread From: Michael Bommarito @ 2026-07-11 15:19 UTC (permalink / raw) To: Taehee Yoo, Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni Cc: netdev, linux-kernel amt_multicast_data_handler(), amt_membership_query_handler() and amt_update_handler() rewrite the ethernet header of the decapsulated skb in place (eth->h_proto, eth->h_dest and, for the query, also eth->h_source) before handing it up the stack. The skb head may be shared, for example when a packet tap has cloned it on the underlay interface, so writing through it corrupts the other reader's copy. Call skb_cow_head() before the rewrite so the head is private. It is placed before the pointers into the head are (re-)derived, so a reallocation caused by the copy is picked up by those derivations. 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> --- drivers/net/amt.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/net/amt.c b/drivers/net/amt.c index 35e77af76bd9d..b733309b866ff 100644 --- a/drivers/net/amt.c +++ b/drivers/net/amt.c @@ -2320,6 +2320,9 @@ static bool amt_multicast_data_handler(struct amt_dev *amt, struct sk_buff *skb) skb_reset_mac_header(skb); skb_pull(skb, sizeof(*eth)); + if (skb_cow_head(skb, 0)) + return true; + if (!pskb_may_pull(skb, sizeof(*iph))) return true; iph = ip_hdr(skb); @@ -2396,6 +2399,8 @@ static bool amt_membership_query_handler(struct amt_dev *amt, skb_reset_network_header(skb); eth = eth_hdr(skb); ether_addr_copy(h_source, oeth->h_source); + if (skb_cow_head(skb, 0)) + return true; if (!pskb_may_pull(skb, sizeof(*iph))) return true; @@ -2521,6 +2526,9 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb) if (!pskb_may_pull(skb, sizeof(*iph))) return true; + if (skb_cow_head(skb, 0)) + return true; + iph = ip_hdr(skb); if (iph->version == 4) { if (ip_mc_check_igmp(skb)) { -- 2.53.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls 2026-07-11 15:19 [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls Michael Bommarito 2026-07-11 15:19 ` [PATCH net v5 1/2] amt: re-read skb header pointers after every pull Michael Bommarito 2026-07-11 15:19 ` [PATCH net v5 2/2] amt: make the head writable before rewriting the L2 header Michael Bommarito @ 2026-07-20 11:06 ` Simon Horman 2026-07-22 0:43 ` Jakub Kicinski ` (2 subsequent siblings) 5 siblings, 0 replies; 8+ messages in thread From: Simon Horman @ 2026-07-20 11:06 UTC (permalink / raw) To: Michael Bommarito Cc: Taehee Yoo, Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel On Sat, Jul 11, 2026 at 11:19:32AM -0400, Michael Bommarito wrote: > Several AMT receive and transmit paths cache a pointer into the skb head > and then call a helper that can reallocate that head before the cached > pointer is used again, so the later access reads or writes freed memory. > > Patch 1 walks every AMT path and, for each pointer used after a > reallocating call, either snapshots the value before the first pull or > re-derives the pointer after the last one. > > Patch 2 is a smaller, separable hardening change: the three handlers > that rewrite the ethernet header do so in place without making the head > private, which corrupts a cloned skb (for example one held by a packet > tap). It adds skb_cow_head() before the rewrite, split out so the > use-after-free fix is not held up by discussion of the clone case. > > Both patches build cleanly (x86_64, CONFIG_AMT, W=1) and are > checkpatch --strict clean. > > Changes since v4: > - amt_update_handler(): also snapshot amtmu->nonce and > amtmu->response_mac before iptunnel_pull_header(), which can > reallocate the head for a GSO cloned skb; the tunnel-match loop read > both fields through the stale amtmu. This is the same class as the > query handler's response_mac snapshot and was the one remaining site > the v4 fix missed. > - Remove the explanatory comments added in v4; the reason for each > snapshot/re-derive is described in the commit message instead. > - Order the local variable declarations longest-to-shortest in the > handlers that gained locals (amt_membership_query_handler and > amt_update_handler). > > v4: https://lore.kernel.org/all/20260707193243.3448201-1-michael.bommarito@gmail.com/ > v3: https://lore.kernel.org/all/20260626111917.802243-1-michael.bommarito@gmail.com/ > v2: https://lore.kernel.org/all/20260617123443.3586930-1-michael.bommarito@gmail.com/ For the series: Reviewed-by: Simon Horman <horms@kernel.org> FTR, I believe the issues flagged in the AI-generated review of this patch should not block it's progress. Rather, you may wish to consider then in the context of possible follow-up. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls 2026-07-11 15:19 [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls Michael Bommarito ` (2 preceding siblings ...) 2026-07-20 11:06 ` [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls Simon Horman @ 2026-07-22 0:43 ` Jakub Kicinski 2026-07-22 1:41 ` Taehee Yoo 2026-07-22 15:00 ` patchwork-bot+netdevbpf 5 siblings, 0 replies; 8+ messages in thread From: Jakub Kicinski @ 2026-07-22 0:43 UTC (permalink / raw) To: Taehee Yoo Cc: Michael Bommarito, Andrew Lunn, David S . Miller, Eric Dumazet, Paolo Abeni, netdev, linux-kernel On Sat, 11 Jul 2026 11:19:32 -0400 Michael Bommarito wrote: > Several AMT receive and transmit paths cache a pointer into the skb head > and then call a helper that can reallocate that head before the cached > pointer is used again, so the later access reads or writes freed memory. Taehee, please review? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls 2026-07-11 15:19 [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls Michael Bommarito ` (3 preceding siblings ...) 2026-07-22 0:43 ` Jakub Kicinski @ 2026-07-22 1:41 ` Taehee Yoo 2026-07-22 10:44 ` Michael Bommarito 2026-07-22 15:00 ` patchwork-bot+netdevbpf 5 siblings, 1 reply; 8+ messages in thread From: Taehee Yoo @ 2026-07-22 1:41 UTC (permalink / raw) To: Michael Bommarito Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel On Sun, Jul 12, 2026 at 12:19 AM Michael Bommarito <michael.bommarito@gmail.com> wrote: > Hi Michael, Thanks a lot for your patience during the long review. > Several AMT receive and transmit paths cache a pointer into the skb head > and then call a helper that can reallocate that head before the cached > pointer is used again, so the later access reads or writes freed memory. > > Patch 1 walks every AMT path and, for each pointer used after a > reallocating call, either snapshots the value before the first pull or > re-derives the pointer after the last one. > > Patch 2 is a smaller, separable hardening change: the three handlers > that rewrite the ethernet header do so in place without making the head > private, which corrupts a cloned skb (for example one held by a packet > tap). It adds skb_cow_head() before the rewrite, split out so the > use-after-free fix is not held up by discussion of the clone case. > > Both patches build cleanly (x86_64, CONFIG_AMT, W=1) and are > checkpatch --strict clean. > > Changes since v4: > - amt_update_handler(): also snapshot amtmu->nonce and > amtmu->response_mac before iptunnel_pull_header(), which can > reallocate the head for a GSO cloned skb; the tunnel-match loop read > both fields through the stale amtmu. This is the same class as the > query handler's response_mac snapshot and was the one remaining site > the v4 fix missed. > - Remove the explanatory comments added in v4; the reason for each > snapshot/re-derive is described in the commit message instead. > - Order the local variable declarations longest-to-shortest in the > handlers that gained locals (amt_membership_query_handler and > amt_update_handler). > > v4: https://lore.kernel.org/all/20260707193243.3448201-1-michael.bommarito@gmail.com/ > v3: https://lore.kernel.org/all/20260626111917.802243-1-michael.bommarito@gmail.com/ > v2: https://lore.kernel.org/all/20260617123443.3586930-1-michael.bommarito@gmail.com/ > > Michael Bommarito (2): > amt: re-read skb header pointers after every pull > amt: make the head writable before rewriting the L2 header > LGTM for the series. Reviewed-by: Taehee Yoo <ap420073@gmail.com> Thanks a lot! Taehee Yoo > drivers/net/amt.c | 87 ++++++++++++++++++++++++++++++++++++++++--------------- > 1 file changed, 63 insertions(+), 24 deletions(-) > > > base-commit: 2c7c88a412aa6d09cd04b414211b4ef8553b5309 > -- > 2.53.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls 2026-07-22 1:41 ` Taehee Yoo @ 2026-07-22 10:44 ` Michael Bommarito 0 siblings, 0 replies; 8+ messages in thread From: Michael Bommarito @ 2026-07-22 10:44 UTC (permalink / raw) To: Taehee Yoo Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel On Tue, Jul 21, 2026 at 9:41 PM Taehee Yoo <ap420073@gmail.com> wrote: > Hi Michael, > Thanks a lot for your patience during the long review. No problem, it's summer :) Thank you for the careful review Thanks, Mike ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls 2026-07-11 15:19 [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls Michael Bommarito ` (4 preceding siblings ...) 2026-07-22 1:41 ` Taehee Yoo @ 2026-07-22 15:00 ` patchwork-bot+netdevbpf 5 siblings, 0 replies; 8+ messages in thread From: patchwork-bot+netdevbpf @ 2026-07-22 15:00 UTC (permalink / raw) To: Michael Bommarito Cc: ap420073, andrew+netdev, davem, edumazet, kuba, pabeni, netdev, linux-kernel Hello: This series was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Sat, 11 Jul 2026 11:19:32 -0400 you wrote: > Several AMT receive and transmit paths cache a pointer into the skb head > and then call a helper that can reallocate that head before the cached > pointer is used again, so the later access reads or writes freed memory. > > Patch 1 walks every AMT path and, for each pointer used after a > reallocating call, either snapshots the value before the first pull or > re-derives the pointer after the last one. > > [...] Here is the summary with links: - [net,v5,1/2] amt: re-read skb header pointers after every pull https://git.kernel.org/netdev/net/c/3656a79f94c4 - [net,v5,2/2] amt: make the head writable before rewriting the L2 header https://git.kernel.org/netdev/net/c/53969d704fa5 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-07-22 15:00 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-11 15:19 [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls Michael Bommarito 2026-07-11 15:19 ` [PATCH net v5 1/2] amt: re-read skb header pointers after every pull Michael Bommarito 2026-07-11 15:19 ` [PATCH net v5 2/2] amt: make the head writable before rewriting the L2 header Michael Bommarito 2026-07-20 11:06 ` [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls Simon Horman 2026-07-22 0:43 ` Jakub Kicinski 2026-07-22 1:41 ` Taehee Yoo 2026-07-22 10:44 ` Michael Bommarito 2026-07-22 15:00 ` patchwork-bot+netdevbpf
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.