[PATCH net v2] fou: reserve headroom before pushing the encap header

[PATCH net v2] fou: reserve headroom before pushing the encap header

[Theodor Arsenij Larionov Trichkine]

ip_tunnel_xmit() and ip_md_tunnel_xmit() call ip_tunnel_encap() before
reserving headroom. A packet redirected into the tunnel (tc mirred, nft
fwd) has only the headroom of its original device, so fou_build_udp()
hits skb_under_panic(); with a mirred mirror the push overwrites the
Ethernet header of the original packet.

Reserve the headroom in fou_build_header() and gue_build_header(). GUE
sizes and builds the header from one copy of the encap config, which
can change concurrently. As the skb head may move, ip_tunnel_xmit() and
ip_md_tunnel_xmit() reload inner_iph after ip_tunnel_encap(), and
ip_tunnel_xmit() reads tnl_params before it: for GRE with header_ops,
tnl_params points into the head.

Fixes: 56328486539d ("net: Changes to ip_tunnel to support foo-over-udp encapsulation")
Fixes: ac931d4cdec3 ("ipip,ip_tunnel,sit: Add FOU support for externally controlled ipip devices")
Cc: stable@vger.kernel.org
Signed-off-by: Theodor Arsenij Larionov Trichkine <theodorlarionov@gmail.com>
---
v2: move the headroom reservation into {fou,gue}_build_header(), reload
inner_iph after ip_tunnel_encap() and read tnl_params before it, which
the Sashiko review of v1 pointed out.
v1: https://lore.kernel.org/netdev/20261010223601.1193655-1-theodorlarionov@gmail.com/

Chengfeng Ye posted an earlier series for this bug; this takes the
builder approach Paolo suggested.
Link: https://lore.kernel.org/netdev/20260904165544.1362052-1-nicoyip.dev@gmail.com/

 net/ipv4/fou_core.c  | 15 +++++++++++----
 net/ipv4/ip_tunnel.c |  6 ++++--
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/fou_core.c b/net/ipv4/fou_core.c
index 076fdca44f51..896e63adf698 100644
--- a/net/ipv4/fou_core.c
+++ b/net/ipv4/fou_core.c
@@ -1059,6 +1059,9 @@ static int fou_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
 	__be16 sport;
 	int err;
 
+	if (skb_cow_head(skb, fou_encap_hlen(e)))
+		return -ENOMEM;
+
 	err = __fou_build_header(skb, e, protocol, &sport, type);
 	if (err)
 		return err;
@@ -1071,16 +1074,20 @@ static int fou_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
 static int gue_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
 			    u8 *protocol, struct flowi4 *fl4)
 {
-	int type = e->flags & TUNNEL_ENCAP_FLAG_CSUM ? SKB_GSO_UDP_TUNNEL_CSUM :
-						       SKB_GSO_UDP_TUNNEL;
+	struct ip_tunnel_encap encap = READ_ONCE(*e);
+	int type = encap.flags & TUNNEL_ENCAP_FLAG_CSUM ?
+		   SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
 	__be16 sport;
 	int err;
 
-	err = __gue_build_header(skb, e, protocol, &sport, type);
+	if (skb_cow_head(skb, gue_encap_hlen(&encap)))
+		return -ENOMEM;
+
+	err = __gue_build_header(skb, &encap, protocol, &sport, type);
 	if (err)
 		return err;
 
-	fou_build_udp(skb, e, fl4, protocol, sport);
+	fou_build_udp(skb, &encap, fl4, protocol, sport);
 
 	return 0;
 }
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index e6bcf01411d0..742c2b309608 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -606,6 +606,7 @@ void ip_md_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 
 	if (ip_tunnel_encap(skb, &tun_info->encap, &proto, &fl4) < 0)
 		goto tx_error;
+	inner_iph = (const struct iphdr *)skb_inner_network_header(skb);
 
 	use_cache = ip_tunnel_dst_cache_usable(skb, tun_info);
 	if (use_cache)
@@ -749,6 +750,8 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 	}
 
 	tos = tnl_params->tos;
+	df = tnl_params->frag_off;
+	ttl = tnl_params->ttl;
 	if (tos & 0x1) {
 		tos &= ~0x1;
 		if (payload_protocol == htons(ETH_P_IP)) {
@@ -767,6 +770,7 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 
 	if (ip_tunnel_encap(skb, &tunnel->encap, &protocol, &fl4) < 0)
 		goto tx_error;
+	inner_iph = (const struct iphdr *)skb_inner_network_header(skb);
 
 	if (connected && md) {
 		use_cache = ip_tunnel_dst_cache_usable(skb, tun_info);
@@ -799,7 +803,6 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 		goto tx_error;
 	}
 
-	df = tnl_params->frag_off;
 	if (payload_protocol == htons(ETH_P_IP) && !tunnel->ignore_df)
 		df |= (inner_iph->frag_off & htons(IP_DF));
 
@@ -821,7 +824,6 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 	}
 
 	tos = ip_tunnel_ecn_encap(tos, inner_iph, skb);
-	ttl = tnl_params->ttl;
 	if (ttl == 0) {
 		if (payload_protocol == htons(ETH_P_IP))
 			ttl = inner_iph->ttl;
-- 
2.34.1

Re: [PATCH net v2] fou: reserve headroom before pushing the encap header

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v2] fou: reserve headroom before pushing the encap header

[Chengfeng Ye]

On Wed, Oct 21, 2026 at 2:55 PM Theodor Arsenij Larionov Trichkine
<theodorlarionov@gmail.com> wrote:
>
> ip_tunnel_xmit() and ip_md_tunnel_xmit() call ip_tunnel_encap() before
> reserving headroom. A packet redirected into the tunnel (tc mirred, nft
> fwd) has only the headroom of its original device, so fou_build_udp()
> hits skb_under_panic(); with a mirred mirror the push overwrites the
> Ethernet header of the original packet.
>
> Reserve the headroom in fou_build_header() and gue_build_header(). GUE
> sizes and builds the header from one copy of the encap config, which
> can change concurrently. As the skb head may move, ip_tunnel_xmit() and
> ip_md_tunnel_xmit() reload inner_iph after ip_tunnel_encap(), and
> ip_tunnel_xmit() reads tnl_params before it: for GRE with header_ops,
> tnl_params points into the head.
>
> Fixes: 56328486539d ("net: Changes to ip_tunnel to support foo-over-udp encapsulation")
> Fixes: ac931d4cdec3 ("ipip,ip_tunnel,sit: Add FOU support for externally controlled ipip devices")
> Cc: stable@vger.kernel.org
> Signed-off-by: Theodor Arsenij Larionov Trichkine <theodorlarionov@gmail.com>
> ---
> v2: move the headroom reservation into {fou,gue}_build_header(), reload
> inner_iph after ip_tunnel_encap() and read tnl_params before it, which
> the Sashiko review of v1 pointed out.
> v1: https://lore.kernel.org/netdev/20261010223601.1193655-1-theodorlarionov@gmail.com/
>
> Chengfeng Ye posted an earlier series for this bug; this takes the
> builder approach Paolo suggested.
> Link: https://lore.kernel.org/netdev/20260904165544.1362052-1-nicoyip.dev@gmail.com/

My v1/v2 [1][2] reserved the headroom in gue_build_header(), close to
what this patch does. Ido objected to that [3]: the caller of
ip_tunnel_encap() is expected to reserve encap_hlen() bytes, and an
skb_cow_head() in build_header() forces every caller to refresh
pointers into skb->head. I moved the fix into ip_tunnel_xmit() for
v3/v4 as Ido suggested, and wait for the IPv6 part [4] to be accepted first.
That patch is changes-requested, and Paolo said the IPv6 side should become
an RCU conversion targeting net-next [5].

I'm fine with either approach, and will drop my v4 if this patch is taken.

If this version is applied, I'd appreciate a Reported-by, as I first
reported the bug in [1]:

Reported-by: Chengfeng Ye <nicoyip.dev@gmail.com>
Closes: https://lore.kernel.org/netdev/20260801060115.3538849-1-nicoyip.dev@gmail.com/

[1] https://lore.kernel.org/netdev/20260801060115.3538849-1-nicoyip.dev@gmail.com/
[2] https://lore.kernel.org/netdev/20260821145908.2829132-1-nicoyip.dev@gmail.com/
[3] https://lore.kernel.org/netdev/20260824075011.GA858321@shredder/
[4] https://lore.kernel.org/netdev/2ce8f3e4bbed6060afb0aee33dc4e1d9ae021280.1788262122.git.petalzu987@gmail.com/
[5] https://lore.kernel.org/netdev/e651fe9e-2d84-4320-99a1-6fddcfc00889@redhat.com/

>  net/ipv4/fou_core.c  | 15 +++++++++++----
>  net/ipv4/ip_tunnel.c |  6 ++++--
>  2 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/net/ipv4/fou_core.c b/net/ipv4/fou_core.c
> index 076fdca44f51..896e63adf698 100644
> --- a/net/ipv4/fou_core.c
> +++ b/net/ipv4/fou_core.c
> @@ -1059,6 +1059,9 @@ static int fou_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
>         __be16 sport;
>         int err;
>
> +       if (skb_cow_head(skb, fou_encap_hlen(e)))
> +               return -ENOMEM;
> +
>         err = __fou_build_header(skb, e, protocol, &sport, type);
>         if (err)
>                 return err;
> @@ -1071,16 +1074,20 @@ static int fou_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
>  static int gue_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
>                             u8 *protocol, struct flowi4 *fl4)
>  {
> -       int type = e->flags & TUNNEL_ENCAP_FLAG_CSUM ? SKB_GSO_UDP_TUNNEL_CSUM :
> -                                                      SKB_GSO_UDP_TUNNEL;
> +       struct ip_tunnel_encap encap = READ_ONCE(*e);
> +       int type = encap.flags & TUNNEL_ENCAP_FLAG_CSUM ?
> +                  SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
>         __be16 sport;
>         int err;
>
> -       err = __gue_build_header(skb, e, protocol, &sport, type);
> +       if (skb_cow_head(skb, gue_encap_hlen(&encap)))
> +               return -ENOMEM;
> +
> +       err = __gue_build_header(skb, &encap, protocol, &sport, type);
>         if (err)
>                 return err;
>
> -       fou_build_udp(skb, e, fl4, protocol, sport);
> +       fou_build_udp(skb, &encap, fl4, protocol, sport);
>
>         return 0;
>  }
> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> index e6bcf01411d0..742c2b309608 100644
> --- a/net/ipv4/ip_tunnel.c
> +++ b/net/ipv4/ip_tunnel.c
> @@ -606,6 +606,7 @@ void ip_md_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
>
>         if (ip_tunnel_encap(skb, &tun_info->encap, &proto, &fl4) < 0)
>                 goto tx_error;
> +       inner_iph = (const struct iphdr *)skb_inner_network_header(skb);
>
>         use_cache = ip_tunnel_dst_cache_usable(skb, tun_info);
>         if (use_cache)
> @@ -749,6 +750,8 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
>         }
>
>         tos = tnl_params->tos;
> +       df = tnl_params->frag_off;
> +       ttl = tnl_params->ttl;
>         if (tos & 0x1) {
>                 tos &= ~0x1;
>                 if (payload_protocol == htons(ETH_P_IP)) {
> @@ -767,6 +770,7 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
>
>         if (ip_tunnel_encap(skb, &tunnel->encap, &protocol, &fl4) < 0)
>                 goto tx_error;
> +       inner_iph = (const struct iphdr *)skb_inner_network_header(skb);
>
>         if (connected && md) {
>                 use_cache = ip_tunnel_dst_cache_usable(skb, tun_info);
> @@ -799,7 +803,6 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
>                 goto tx_error;
>         }
>
> -       df = tnl_params->frag_off;
>         if (payload_protocol == htons(ETH_P_IP) && !tunnel->ignore_df)
>                 df |= (inner_iph->frag_off & htons(IP_DF));
>
> @@ -821,7 +824,6 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
>         }
>
>         tos = ip_tunnel_ecn_encap(tos, inner_iph, skb);
> -       ttl = tnl_params->ttl;
>         if (ttl == 0) {
>                 if (payload_protocol == htons(ETH_P_IP))
>                         ttl = inner_iph->ttl;
> --
> 2.34.1
>

Thanks,
Chengfeng

Re: [PATCH net v2] fou: reserve headroom before pushing the encap header

[Paolo Abeni]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
> diff --git a/net/ipv4/fou_core.c b/net/ipv4/fou_core.c
> --- a/net/ipv4/fou_core.c
> +++ b/net/ipv4/fou_core.c
> @@ -1071,16 +1074,20 @@ static int fou_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
>  static int gue_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,
>  			    u8 *protocol, struct flowi4 *fl4)
>  {
> -	int type = e->flags & TUNNEL_ENCAP_FLAG_CSUM ? SKB_GSO_UDP_TUNNEL_CSUM :
> -						       SKB_GSO_UDP_TUNNEL;
> +	struct ip_tunnel_encap encap = READ_ONCE(*e);
> +	int type = encap.flags & TUNNEL_ENCAP_FLAG_CSUM ?
> +		   SKB_GSO_UDP_TUNNEL_CSUM : SKB_GSO_UDP_TUNNEL;
>  	__be16 sport;
>  	int err;
>  
> -	err = __gue_build_header(skb, e, protocol, &sport, type);
> +	if (skb_cow_head(skb, gue_encap_hlen(&encap)))
> +		return -ENOMEM;
> +
> +	err = __gue_build_header(skb, &encap, protocol, &sport, type);
>  	if (err)
>  		return err;
>  
> -	fou_build_udp(skb, e, fl4, protocol, sport);
> +	fou_build_udp(skb, &encap, fl4, protocol, sport);
>  
>  	return 0;
>  }

[Severity: High]
[Finding: sashiko-gemini-db46d5c5]
[Sources: sashiko-gemini]
struct ip_tunnel_encap is four 16-bit fields. It is 8 bytes in size
but only 2-byte aligned. Inside struct ip_tunnel the encap member
follows parms and three ints (mlink, encap_hlen, hlen). That places
&tunnel->encap on a 4-byte boundary, not an 8-byte one, on 64-bit.

READ_ONCE(*e) on an 8-byte object passes
compiletime_assert_rwonce_type() and is done as a single 64-bit
access. On arm64 with CONFIG_LTO, the 8-byte case of __READ_ONCE()
emits ldar/ldapr, which requires natural alignment. Every GUE
transmit that reaches gue_build_header() with &tunnel->encap, from
ip_tunnel_xmit() or ipip6_tunnel_xmit(), can then take an alignment
fault on the hot path. CONFIG_UBSAN_ALIGNMENT will also flag the
misaligned 64-bit access elsewhere.

The whole-struct READ_ONCE() also does not give the atomicity it
suggests. ip_tunnel_encap_setup() fills tunnel->encap with separate
stores, so the copy can still be torn between fields.

What must stay consistent here is flags, between gue_encap_hlen() and
__gue_build_header(). Could the local copy be built field by field,
with READ_ONCE() on flags, sport and dport, instead of one READ_ONCE()
of the whole struct? Or is there a reason you need a single 8-byte
load?
-- 
This is an AI-generated review.