Re: [PATCH net] seg6: keep room for the mac header when growing the headroom

[Justin Iurman <justin.iurman@gmail.com>]

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>