All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ido Schimmel <idosch@nvidia.com>
To: Zhiling Zou <zhilinz@nebusec.ai>
Cc: netdev@vger.kernel.org, dsahern@kernel.org, davem@davemloft.net,
	edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
	horms@kernel.org, atenart@kernel.org, yuehaibing@huawei.com,
	kuniyu@google.com, kees@kernel.org, kylebot@openai.com,
	thorsten.blum@linux.dev, maoyixie.tju@gmail.com, vega@nebusec.ai
Subject: Re: [PATCH net v4 2/2] net: cap advertised IP tunnel headroom
Date: Mon, 17 Aug 2026 09:43:19 +0300	[thread overview]
Message-ID: <20260817064319.GB196908@shredder> (raw)
In-Reply-To: <ba04a1fd6bfae2377607fad5d8f80f7eb80fd4c4.1786542637.git.zhilinz@nebusec.ai>

On Thu, Aug 13, 2026 at 12:22:35AM +0800, Zhiling Zou wrote:
> IP tunnel devices derive their advertised needed_headroom from lower
> output devices. A stack of user-created devices can make the derived
> value larger than the 16-bit skb header offsets can represent. Once IP
> output reserves it, skb head expansion can wrap those offsets.
> 
> The runtime transmit path already caps a growing needed_headroom at 512.
> Apply the same cap when tunnel configuration publishes needed_headroom
> derived from a lower output device.
> 
> Capping the advertised value is safe: IP tunnel transmit still expands
> the skb when a packet needs more headroom. A nonsensical stacked
> configuration can therefore incur an extra reallocation, but it cannot
> publish an unbounded reservation to upper layers.
> 
> Fixes: 1a37e412a022 ("net: Use 16bits for *_headers fields of struct skbuff")
> Cc: stable@vger.kernel.org
> Reported-by: Vega <vega@nebusec.ai>
> Signed-off-by: Zhiling Zou <zhilinz@nebusec.ai>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

[...]

> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index 70c1710910203..200d0ba1a40e7 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -1137,8 +1137,11 @@ static void ip6gre_tnl_link_config_route(struct ip6_tnl *t, int set_mtu,
>  			return;
>  
>  		if (rt->dst.dev) {
> -			dev->needed_headroom = rt->dst.dev->hard_header_len +
> -					       t_hlen;
> +			unsigned int headroom;
> +
> +			headroom = rt->dst.dev->hard_header_len + t_hlen;
> +			headroom = ip_tunnel_limit_headroom(headroom);
> +			dev->needed_headroom = headroom;

"
Can this clamp in ip6gre_tnl_link_config_route() ever fire?

[...]

So rt->dst.dev->hard_header_len + t_hlen stays in the tens of bytes and
never reaches 512, which would make the new call here dead code.
"

In patch #1 we fixed a bug where hard_header_len was summed across
multiple devices. If this bug ever reappears, ip_tunnel_limit_headroom()
will cap the headroom and it costs us nothing to include it here.

"
This isn't a bug introduced by this patch, the missing needed_headroom term
in ip6gre_tnl_link_config_route() predates it, but the patch adds a clamp on
top of it and makes the four sites look uniform while they compute different
quantities. Would it be clearer to either leave the ip6gre site alone or
mention in the changelog that ip6gre derives a different value?
"

See above.

      reply	other threads:[~2026-08-17  6:43 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-12 16:22 [PATCH net v4 0/2] net: fix IP6GRE header length before capping tunnel headroom Zhiling Zou
2026-08-12 16:22 ` [PATCH net v4 1/2] ip6_gre: fix hardware header length for NBMA tunnels Zhiling Zou
2026-08-17  6:42   ` Ido Schimmel
2026-08-12 16:22 ` [PATCH net v4 2/2] net: cap advertised IP tunnel headroom Zhiling Zou
2026-08-17  6:43   ` Ido Schimmel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260817064319.GB196908@shredder \
    --to=idosch@nvidia.com \
    --cc=atenart@kernel.org \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kees@kernel.org \
    --cc=kuba@kernel.org \
    --cc=kuniyu@google.com \
    --cc=kylebot@openai.com \
    --cc=maoyixie.tju@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=thorsten.blum@linux.dev \
    --cc=vega@nebusec.ai \
    --cc=yuehaibing@huawei.com \
    --cc=zhilinz@nebusec.ai \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.