[PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls

Michael Bommarito posted 2 patches 2 weeks ago
drivers/net/amt.c | 87 ++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 63 insertions(+), 24 deletions(-)
[PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls
Posted by Michael Bommarito 2 weeks ago
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
Re: [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls
Posted by Taehee Yoo 3 days, 20 hours ago
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
Re: [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls
Posted by Michael Bommarito 3 days, 11 hours ago
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
Re: [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls
Posted by Jakub Kicinski 3 days, 21 hours ago
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?
Re: [PATCH net v5 0/2] amt: fix use-after-free of the skb head across pulls
Posted by Simon Horman 5 days, 11 hours ago
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.