mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net] seg6: keep room for the mac header when growing the headroom
@ 2026-09-16 21:38 Yuya Kusakabe
  2026-09-17 10:12 ` Justin Iurman
  0 siblings, 1 reply; 5+ messages in thread
From: Yuya Kusakabe @ 2026-09-16 21:38 UTC (permalink / raw)
  To: Andrea Mayer, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Justin Iurman
  Cc: netdev, linux-kernel, Yuya Kusakabe

__seg6_do_srh_inline(), __seg6_do_srh_encap() and
seg6_do_srh_encap_red() all grow the headroom with skb_cow_head(), push
the new headers into it, and then rebuild the mac header below them with
skb_mac_header_rebuild().  The headroom left after the push has to be at
least skb->mac_len for that rebuild, but the three requests ask for the
pushed length plus dst_dev_overhead(), which leaves LL_RESERVED_SPACE()
of the egress device, 16 bytes for plain Ethernet.

Where the mac header is longer than that, as it is on ingress through a
VLAN device with reorder_hdr off, the rebuild runs out of room:
skb_set_mac_header(skb, -skb->mac_len) computes a negative offset,
stores it unchecked in the u16 skb->mac_header, and the memmove that
follows writes skb->mac_len bytes about 64 KB past skb->head.
Forwarding plain ping6 traffic through such a device reproduces it on
all five encapsulation modes; skb->mac_header comes back as 65534 on a
704-byte head.

Ask for whichever of the two is larger.  These requests carried
skb->mac_len until the egress overhead took its place rather than
joining it.

Fixes: 40475b63761a ("net: ipv6: seg6_iptunnel: mitigate 2-realloc issue")
Assisted-by: LLM
Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
---
 net/ipv6/seg6_iptunnel.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
index 61c6a27bf202..0e60bbca19ca 100644
--- a/net/ipv6/seg6_iptunnel.c
+++ b/net/ipv6/seg6_iptunnel.c
@@ -153,7 +153,8 @@ static int __seg6_do_srh_encap(struct sk_buff *skb, struct ipv6_sr_hdr *osrh,
 	hdrlen = (osrh->hdrlen + 1) << 3;
 	tot_len = hdrlen + sizeof(*hdr);
 
-	err = skb_cow_head(skb, tot_len + dst_dev_overhead(cache_dst, skb));
+	err = skb_cow_head(skb, tot_len + max(skb->mac_len,
+					      dst_dev_overhead(cache_dst, skb)));
 	if (unlikely(err))
 		return err;
 
@@ -255,7 +256,8 @@ static int seg6_do_srh_encap_red(struct sk_buff *skb,
 
 	tot_len = red_hdrlen + sizeof(struct ipv6hdr);
 
-	err = skb_cow_head(skb, tot_len + dst_dev_overhead(cache_dst, skb));
+	err = skb_cow_head(skb, tot_len + max(skb->mac_len,
+					      dst_dev_overhead(cache_dst, skb)));
 	if (unlikely(err))
 		return err;
 
@@ -351,7 +353,8 @@ static int __seg6_do_srh_inline(struct sk_buff *skb, struct ipv6_sr_hdr *osrh,
 
 	hdrlen = (osrh->hdrlen + 1) << 3;
 
-	err = skb_cow_head(skb, hdrlen + dst_dev_overhead(cache_dst, skb));
+	err = skb_cow_head(skb, hdrlen + max(skb->mac_len,
+					     dst_dev_overhead(cache_dst, skb)));
 	if (unlikely(err))
 		return err;
 

---
base-commit: 9a1599eeb8d18a2113e4cabcbd3bec5a8377dbed
change-id: 20260917-seg6-maclen-headroom-53a339c17dac

Best regards,
--  
Yuya Kusakabe <yuya.kusakabe@gmail.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] seg6: keep room for the mac header when growing the headroom
  2026-09-16 21:38 [PATCH net] seg6: keep room for the mac header when growing the headroom Yuya Kusakabe
@ 2026-09-17 10:12 ` Justin Iurman
  2026-09-17 16:28   ` Andrea Mayer
  0 siblings, 1 reply; 5+ messages in thread
From: Justin Iurman @ 2026-09-17 10:12 UTC (permalink / raw)
  To: Yuya Kusakabe, Andrea Mayer, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman
  Cc: netdev, linux-kernel

On 9/16/26 23:38, Yuya Kusakabe wrote:
> __seg6_do_srh_inline(), __seg6_do_srh_encap() and
> seg6_do_srh_encap_red() all grow the headroom with skb_cow_head(), push
> the new headers into it, and then rebuild the mac header below them with
> skb_mac_header_rebuild().  The headroom left after the push has to be at
> least skb->mac_len for that rebuild, but the three requests ask for the
> pushed length plus dst_dev_overhead(), which leaves LL_RESERVED_SPACE()
> of the egress device, 16 bytes for plain Ethernet.
> 
> Where the mac header is longer than that, as it is on ingress through a
> VLAN device with reorder_hdr off, the rebuild runs out of room:
> skb_set_mac_header(skb, -skb->mac_len) computes a negative offset,
> stores it unchecked in the u16 skb->mac_header, and the memmove that
> follows writes skb->mac_len bytes about 64 KB past skb->head.
> Forwarding plain ping6 traffic through such a device reproduces it on
> all five encapsulation modes; skb->mac_header comes back as 65534 on a
> 704-byte head.
> 
> Ask for whichever of the two is larger.  These requests carried
> skb->mac_len until the egress overhead took its place rather than
> joining it.
> 
> Fixes: 40475b63761a ("net: ipv6: seg6_iptunnel: mitigate 2-realloc issue")
> Assisted-by: LLM
> Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
> ---
>   net/ipv6/seg6_iptunnel.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
> index 61c6a27bf202..0e60bbca19ca 100644
> --- a/net/ipv6/seg6_iptunnel.c
> +++ b/net/ipv6/seg6_iptunnel.c
> @@ -153,7 +153,8 @@ static int __seg6_do_srh_encap(struct sk_buff *skb, struct ipv6_sr_hdr *osrh,
>   	hdrlen = (osrh->hdrlen + 1) << 3;
>   	tot_len = hdrlen + sizeof(*hdr);
>   
> -	err = skb_cow_head(skb, tot_len + dst_dev_overhead(cache_dst, skb));
> +	err = skb_cow_head(skb, tot_len + max(skb->mac_len,
> +					      dst_dev_overhead(cache_dst, skb)));
>   	if (unlikely(err))
>   		return err;
>   
> @@ -255,7 +256,8 @@ static int seg6_do_srh_encap_red(struct sk_buff *skb,
>   
>   	tot_len = red_hdrlen + sizeof(struct ipv6hdr);
>   
> -	err = skb_cow_head(skb, tot_len + dst_dev_overhead(cache_dst, skb));
> +	err = skb_cow_head(skb, tot_len + max(skb->mac_len,
> +					      dst_dev_overhead(cache_dst, skb)));
>   	if (unlikely(err))
>   		return err;
>   
> @@ -351,7 +353,8 @@ static int __seg6_do_srh_inline(struct sk_buff *skb, struct ipv6_sr_hdr *osrh,
>   
>   	hdrlen = (osrh->hdrlen + 1) << 3;
>   
> -	err = skb_cow_head(skb, hdrlen + dst_dev_overhead(cache_dst, skb));
> +	err = skb_cow_head(skb, hdrlen + max(skb->mac_len,
> +					     dst_dev_overhead(cache_dst, skb)));
>   	if (unlikely(err))
>   		return err;

Overall, LGTM, thanks. However, I think we'd need a v2 with the followings:

- use max_t(unsigned int, skb->mac_len, dst_dev_overhead(cache_dst, 
skb)) instead of max()
- apply the same changes to ioam6_iptunnel and rpl_iptunnel (all in one 
patch is fine)

Reviewed-by: Justin Iurman <justin.iurman@gmail.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] seg6: keep room for the mac header when growing the headroom
  2026-09-17 10:12 ` Justin Iurman
@ 2026-09-17 16:28   ` Andrea Mayer
  2026-09-17 20:00     ` Justin Iurman
  2026-09-18  3:00     ` Yuya Kusakabe
  0 siblings, 2 replies; 5+ messages in thread
From: Andrea Mayer @ 2026-09-17 16:28 UTC (permalink / raw)
  To: Justin Iurman
  Cc: Yuya Kusakabe, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, netdev, linux-kernel, stefano.salsano,
	Andrea Mayer

On Thu, 17 Sep 2026 12:12:11 +0200
Justin Iurman <justin.iurman@gmail.com> wrote:

> On 9/16/26 23:38, Yuya Kusakabe wrote:
>> [snip]
> 
> Overall, LGTM, thanks. However, I think we'd need a v2 with the followings:
> 
> - use max_t(unsigned int, skb->mac_len, dst_dev_overhead(cache_dst, 
> skb)) instead of max()
> - apply the same changes to ioam6_iptunnel and rpl_iptunnel (all in one 
> patch is fine)
> 
> Reviewed-by: Justin Iurman <justin.iurman@gmail.com>

Hi Justin,

Agreed, rpl and ioam6 inline do trigger. Single VLAN device per side,
reorder_hdr off on the receiving one, plain ping:

  BUG: KASAN: slab-out-of-bounds in rpl_do_srh_inline.isra.0+0x3d3/0x770
  Write of size 18 at addr ffff88810deeba7e by task ping/447

  CPU: 0 UID: 0 PID: 447 Comm: ping Not tainted 7.3.0-rc1 #364
  Call Trace:
   <IRQ>
   __asan_memmove+0x38/0x60
   rpl_do_srh_inline.isra.0+0x3d3/0x770
   rpl_input+0xd3/0x5e0
   lwtunnel_input+0x18d/0x420
   ipv6_rcv+0x452/0x460

  BUG: KASAN: slab-use-after-free in ioam6_do_inline+0x2d8/0x5e0
  Write of size 18 at addr ffff88811480fa7e by task ping/432

  CPU: 0 UID: 0 PID: 432 Comm: ping Not tainted 7.3.0-rc1 #364
  Call Trace:
   <IRQ>
   __asan_memmove+0x38/0x60
   ioam6_do_inline+0x2d8/0x5e0
   ioam6_output+0x335/0x970
   lwtunnel_output+0x1b0/0x440
   ip6_forward+0x16a7/0x16f0
   ipv6_rcv+0x452/0x460

ioam6_do_encap triggers too, with three VLAN tags via tc push:

  BUG: KASAN: use-after-free in ioam6_do_encap+0x202/0x5c0
  Write of size 26 at addr ffff88810de227fe by task ping/453

  CPU: 0 UID: 0 PID: 453 Comm: ping Not tainted 7.3.0-rc1 #364
  Call Trace:
   <IRQ>
   __asan_memmove+0x38/0x60
   ioam6_do_encap+0x202/0x5c0
   ioam6_output+0x3cc/0x970
   lwtunnel_output+0x1b0/0x440
   ip6_forward+0x16a7/0x16f0
   ipv6_rcv+0x452/0x460

I would fix dst_dev_overhead() itself rather than patching every
caller individually, that covers all callers at once and protects
any future user of the helper. dst_dev_overhead() already returns
skb->mac_len when dst is NULL, the fix would make the other branch
consistent:

--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -455,7 +455,8 @@ static inline unsigned int dst_dev_overhead(struct dst_entry *dst,
 					    struct sk_buff *skb)
 {
 	if (likely(dst))
-		return LL_RESERVED_SPACE(dst->dev);
+		return max_t(unsigned int, skb->mac_len,
+			     LL_RESERVED_SPACE(dst->dev));

 	return skb->mac_len;
 }

What do you think?

Thanks,
Andrea

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] seg6: keep room for the mac header when growing the headroom
  2026-09-17 16:28   ` Andrea Mayer
@ 2026-09-17 20:00     ` Justin Iurman
  2026-09-18  3:00     ` Yuya Kusakabe
  1 sibling, 0 replies; 5+ messages in thread
From: Justin Iurman @ 2026-09-17 20:00 UTC (permalink / raw)
  To: Andrea Mayer
  Cc: Yuya Kusakabe, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, netdev, linux-kernel, stefano.salsano

On 9/17/26 18:28, Andrea Mayer wrote:
> On Thu, 17 Sep 2026 12:12:11 +0200
> Justin Iurman <justin.iurman@gmail.com> wrote:
> 
>> On 9/16/26 23:38, Yuya Kusakabe wrote:
>>> [snip]
>>
>> Overall, LGTM, thanks. However, I think we'd need a v2 with the followings:
>>
>> - use max_t(unsigned int, skb->mac_len, dst_dev_overhead(cache_dst,
>> skb)) instead of max()
>> - apply the same changes to ioam6_iptunnel and rpl_iptunnel (all in one
>> patch is fine)
>>
>> Reviewed-by: Justin Iurman <justin.iurman@gmail.com>
> 
> Hi Justin,
> 
> Agreed, rpl and ioam6 inline do trigger. Single VLAN device per side,
> reorder_hdr off on the receiving one, plain ping:
> 
>    BUG: KASAN: slab-out-of-bounds in rpl_do_srh_inline.isra.0+0x3d3/0x770
>    Write of size 18 at addr ffff88810deeba7e by task ping/447
> 
>    CPU: 0 UID: 0 PID: 447 Comm: ping Not tainted 7.3.0-rc1 #364
>    Call Trace:
>     <IRQ>
>     __asan_memmove+0x38/0x60
>     rpl_do_srh_inline.isra.0+0x3d3/0x770
>     rpl_input+0xd3/0x5e0
>     lwtunnel_input+0x18d/0x420
>     ipv6_rcv+0x452/0x460
> 
>    BUG: KASAN: slab-use-after-free in ioam6_do_inline+0x2d8/0x5e0
>    Write of size 18 at addr ffff88811480fa7e by task ping/432
> 
>    CPU: 0 UID: 0 PID: 432 Comm: ping Not tainted 7.3.0-rc1 #364
>    Call Trace:
>     <IRQ>
>     __asan_memmove+0x38/0x60
>     ioam6_do_inline+0x2d8/0x5e0
>     ioam6_output+0x335/0x970
>     lwtunnel_output+0x1b0/0x440
>     ip6_forward+0x16a7/0x16f0
>     ipv6_rcv+0x452/0x460
> 
> ioam6_do_encap triggers too, with three VLAN tags via tc push:
> 
>    BUG: KASAN: use-after-free in ioam6_do_encap+0x202/0x5c0
>    Write of size 26 at addr ffff88810de227fe by task ping/453
> 
>    CPU: 0 UID: 0 PID: 453 Comm: ping Not tainted 7.3.0-rc1 #364
>    Call Trace:
>     <IRQ>
>     __asan_memmove+0x38/0x60
>     ioam6_do_encap+0x202/0x5c0
>     ioam6_output+0x3cc/0x970
>     lwtunnel_output+0x1b0/0x440
>     ip6_forward+0x16a7/0x16f0
>     ipv6_rcv+0x452/0x460
> 
> I would fix dst_dev_overhead() itself rather than patching every
> caller individually, that covers all callers at once and protects
> any future user of the helper. dst_dev_overhead() already returns
> skb->mac_len when dst is NULL, the fix would make the other branch
> consistent:
> 
> --- a/include/net/dst.h
> +++ b/include/net/dst.h
> @@ -455,7 +455,8 @@ static inline unsigned int dst_dev_overhead(struct dst_entry *dst,
>   					    struct sk_buff *skb)
>   {
>   	if (likely(dst))
> -		return LL_RESERVED_SPACE(dst->dev);
> +		return max_t(unsigned int, skb->mac_len,
> +			     LL_RESERVED_SPACE(dst->dev));
> 
>   	return skb->mac_len;
>   }

+1. That's even better, thanks!

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net] seg6: keep room for the mac header when growing the headroom
  2026-09-17 16:28   ` Andrea Mayer
  2026-09-17 20:00     ` Justin Iurman
@ 2026-09-18  3:00     ` Yuya Kusakabe
  1 sibling, 0 replies; 5+ messages in thread
From: Yuya Kusakabe @ 2026-09-18  3:00 UTC (permalink / raw)
  To: Andrea Mayer
  Cc: Yuya Kusakabe, Justin Iurman, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, netdev, linux-kernel,
	stefano.salsano

On Thu, 17 Sep 2026 18:28:26 +0200 Andrea Mayer <andrea.mayer@uniroma2.it> wrote:
> I would fix dst_dev_overhead() itself rather than patching every
> caller individually, that covers all callers at once and protects
> any future user of the helper.

Agreed, I will send a v3 with it and a Suggested-by.

v2 crossed with your mail and still patches the callers; please ignore
it.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-18  3:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-16 21:38 [PATCH net] seg6: keep room for the mac header when growing the headroom Yuya Kusakabe
2026-09-17 10:12 ` Justin Iurman
2026-09-17 16:28   ` Andrea Mayer
2026-09-17 20:00     ` Justin Iurman
2026-09-18  3:00     ` Yuya Kusakabe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®