Re: [PATCH net v3 1/1] net: cap advertised IP tunnel headroom

[Ido Schimmel <idosch@nvidia.com>]

On Thu, Aug 06, 2026 at 09:21:52PM +0800, Zhiling Zou wrote:
> IP tunnel devices derive their advertised needed_headroom and, for IP6GRE
> devices with header_ops, hard_header_len 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 cap when configuration publishes headroom or header lengths
> derived from a lower output device.
> 
> Capping the advertised value is safe: IP tunnel transmit still expands the
> skb when the 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>
> ---
> changes in v3:
> - Split netkit handling into a separate follow-up.
> - Explain why capping advertised headroom is safe.
> - Use local variables for derived headroom.
> - v2 Link: https://lore.kernel.org/all/0ae4aa29223b89049727aec4d36f144bad41537e.1785476387.git.zhilinz@nebusec.ai/
> 
> changes in v2:
> - Move the fix from IP send paths to tunnel and netkit device control paths.
> - Cap advertised IP tunnel headroom at 512 and reject netkit headroom
>   values above that limit at device creation.
> - v1 Link: https://lore.kernel.org/all/0c6c64e9bbd71a0decc8504a384061e0e631be13.1785054561.git.zhilinz@nebusec.ai/
> 
>  include/net/ip_tunnels.h | 11 +++++++++--
>  net/ipv4/ip_tunnel.c     |  2 +-
>  net/ipv6/ip6_gre.c       | 10 ++++++----
>  net/ipv6/ip6_tunnel.c    |  7 +++++--
>  net/ipv6/sit.c           |  2 +-
>  5 files changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
> index d708b66e55cda..85e3455cea259 100644
> --- a/include/net/ip_tunnels.h
> +++ b/include/net/ip_tunnels.h
> @@ -629,8 +629,7 @@ struct metadata_dst *iptunnel_metadata_reply(struct metadata_dst *md,
>  int skb_tunnel_check_pmtu(struct sk_buff *skb, struct dst_entry *encap_dst,
>  			  int headroom, bool reply);
>  
> -static inline void ip_tunnel_adj_headroom(struct net_device *dev,
> -					  unsigned int headroom)
> +static inline unsigned int ip_tunnel_limit_headroom(unsigned int headroom)
>  {
>  	/* we must cap headroom to some upperlimit, else pskb_expand_head
>  	 * will overflow header offsets in skb_headers_offset_update().
> @@ -640,6 +639,14 @@ static inline void ip_tunnel_adj_headroom(struct net_device *dev,
>  	if (headroom > max_allowed)
>  		headroom = max_allowed;
>  
> +	return headroom;
> +}
> +
> +static inline void ip_tunnel_adj_headroom(struct net_device *dev,
> +					  unsigned int headroom)
> +{
> +	headroom = ip_tunnel_limit_headroom(headroom);
> +
>  	if (headroom > READ_ONCE(dev->needed_headroom))
>  		WRITE_ONCE(dev->needed_headroom, headroom);
>  }
> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> index 9d114bd575f92..5b1f180485d42 100644
> --- a/net/ipv4/ip_tunnel.c
> +++ b/net/ipv4/ip_tunnel.c
> @@ -317,7 +317,7 @@ static int ip_tunnel_bind_dev(struct net_device *dev)
>  		mtu = min(tdev->mtu, IP_MAX_MTU);
>  	}
>  
> -	dev->needed_headroom = t_hlen + hlen;
> +	dev->needed_headroom = ip_tunnel_limit_headroom(t_hlen + hlen);
>  	mtu -= t_hlen + (dev->type == ARPHRD_ETHER ? dev->hard_header_len : 0);
>  
>  	if (mtu < IPV4_MIN_MTU)
> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index b843116e9b703..cc757586be90a 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -1137,13 +1137,15 @@ static void ip6gre_tnl_link_config_route(struct ip6_tnl *t, int set_mtu,
>  			return;
>  
>  		if (rt->dst.dev) {
> -			unsigned short dst_len = 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);
>  
>  			if (t->dev->header_ops)
> -				dev->hard_header_len = dst_len;
> +				dev->hard_header_len = headroom;
>  			else
> -				dev->needed_headroom = dst_len;
> +				dev->needed_headroom = headroom;

There is a pre-existing issue here that results in the hardware header
length being accumulated over stacked devices and it should be fixed
before capping the headroom.

Commit 832ba596494b ("net: ip6_gre: set dev->hard_header_len when using
header_ops") is correct that when "header_ops->create is used,
dev->hard_header_len should reflect the length of the header created".

For ip6gretap and ip6erspan that's the Ethernet header (14 bytes). For
ip6gre tunnels without a remote (NBMA tunnels), that's what
ip6gre_header() is pushing: GRE header + possible FOU / GUE header +
outer IPv6 header.

So the problems are:

1. hard_header_len shouldn't include the hard_header_len of the lower
device. IOW, for NBMA tunnels it should be 't_hlen', not
'rt->dst.dev->hard_header_len + t_hlen'.

2. hard_header_len must not be changed for ip6gretap and ip6erspan which
have a fixed hardware header length (Ethernet header, 14 bytes).

Following diff should fix it. Please test with both locally generated
traffic and forwarded traffic out of an NBMA tunnel. This should be the
first patch in the series and it should blame commit 832ba596494b ("net:
ip6_gre: set dev->hard_header_len when using header_ops") and carry a
stable tag:

diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index b843116e9b70..70c171091020 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -1137,13 +1137,8 @@ static void ip6gre_tnl_link_config_route(struct ip6_tnl *t, int set_mtu,
 			return;
 
 		if (rt->dst.dev) {
-			unsigned short dst_len = rt->dst.dev->hard_header_len +
-						 t_hlen;
-
-			if (t->dev->header_ops)
-				dev->hard_header_len = dst_len;
-			else
-				dev->needed_headroom = dst_len;
+			dev->needed_headroom = rt->dst.dev->hard_header_len +
+					       t_hlen;
 
 			if (set_mtu) {
 				int mtu = rt->dst.dev->mtu - t_hlen;
@@ -1171,8 +1166,8 @@ static int ip6gre_calc_hlen(struct ip6_tnl *tunnel)
 
 	t_hlen = tunnel->hlen + sizeof(struct ipv6hdr);
 
-	if (tunnel->dev->header_ops)
-		tunnel->dev->hard_header_len = LL_MAX_HEADER + t_hlen;
+	if (tunnel->dev->header_ops && tunnel->dev->type == ARPHRD_IP6GRE)
+		tunnel->dev->hard_header_len = t_hlen;
 	else
 		tunnel->dev->needed_headroom = LL_MAX_HEADER + t_hlen;

There is a pre-existing bug / assumption in the code that NBMA tunnels
don't change to point-to-point tunnels (and vice-versa), so the fix
ignores it as well.

>  
>  			if (set_mtu) {
>  				int mtu = rt->dst.dev->mtu - t_hlen;
> diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
> index bf8e40af60b08..2c941acb081fa 100644
> --- a/net/ipv6/ip6_tunnel.c
> +++ b/net/ipv6/ip6_tunnel.c
> @@ -1522,8 +1522,11 @@ static void ip6_tnl_link_config(struct ip6_tnl *t)
>  			tdev = __dev_get_by_index(t->net, p->link);
>  
>  		if (tdev) {
> -			dev->needed_headroom = tdev->hard_header_len +
> -				tdev->needed_headroom + t_hlen;
> +			unsigned int headroom;
> +
> +			headroom = tdev->hard_header_len + tdev->needed_headroom;
> +			headroom += t_hlen;
> +			dev->needed_headroom = ip_tunnel_limit_headroom(headroom);
>  			mtu = min_t(unsigned int, tdev->mtu, IP6_MAX_MTU);
>  
>  			mtu = mtu - t_hlen;
> diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
> index a38b24fb83842..19b7fa8d1a2a0 100644
> --- a/net/ipv6/sit.c
> +++ b/net/ipv6/sit.c
> @@ -1131,7 +1131,7 @@ static void ipip6_tunnel_bind_dev(struct net_device *dev)
>  		WRITE_ONCE(dev->mtu, mtu);
>  		hlen = tdev->hard_header_len + tdev->needed_headroom;
>  	}
> -	dev->needed_headroom = t_hlen + hlen;
> +	dev->needed_headroom = ip_tunnel_limit_headroom(t_hlen + hlen);
>  }
>  
>  static void ipip6_tunnel_update(struct ip_tunnel *t,
> -- 
> 2.43.0