Re: [PATCH net v3] ip_tunnel: reserve FOU/GUE headroom before encapsulation

[Chengfeng Ye]

On Tue, Aug 25, 2026 at 3:41 PM Ido Schimmel <idosch@xxxxxxxxxx> wrote:
>
> On Mon, Aug 24, 2026 at 07:19:44PM +0800, Chengfeng Ye wrote:
> > ip_tunnel_encap() expects its callers to reserve headroom based on
> > ip_encap_hlen(). Unlike the IPv6 tunnel transmit paths, the IPv4
> > ip_tunnel_xmit() and ip_md_tunnel_xmit() push FOU and GUE headers
> > before they grow the skb headroom.
> >
> > That becomes visible when ipgre_changelink() publishes UDP
> > encapsulation before it updates the device headroom. The transmit path
> > does not serialize with RTNL, so it can interleave as follows:
> >
> >   CPU 0 (ipgre_changelink)        CPU 1 (ipgre_xmit)
> >   install GUE encapsulation
> >                                   reserve the old needed_headroom
> >   publish larger GRE flags
> >   update tunnel->tun_hlen
> >                                   push the larger GRE header
> >                                   push the GUE and UDP headers
> >   update dev->needed_headroom
> >
> > With REMCSUM, the new layout can push 16 bytes of GRE and 20 bytes of
> > GUE/UDP headers into an skb with only 32 bytes of actual headroom. The
> > final UDP push writes four bytes before skb->head.
> >
> > With the update window widened, the kernel reported:
> >
> >   skbuff: skb_under_panic: ... len:128 put:8 ... dev:gre0poc
> >   kernel BUG at net/core/skbuff.c:214!
> >   Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
> >   Call Trace:
> >    skb_push
> >    fou_build_udp
> >    gue_build_header
> >    ip_tunnel_xmit
> >    __gre_xmit
> >    ipgre_xmit
> >
> > Use ip_encap_hlen() up front, route and perform PMTU handling first,
> > then reserve the final headroom before ip_tunnel_encap() builds the
> > UDP tunnel headers. This matches the existing IPv6 pattern and keeps
> > ip_tunnel_encap() as a pure header builder.
>
> Mention in the commit message that the IPv6 fix [1] is still WIP.
> Otherwise, I'm pretty sure that Sashiko will complain about it.
>
> [1] https://lore.kernel.org/netdev/b58876297f7d45de008f2e94b6ecab8b2ed84d21.1786088695.git.petalzu987@xxxxxxxxx/
>
> >
> > Fixes: dd9d598c6657 ("ip_gre: add the support for i/o_flags update via netlink")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Chengfeng Ye <nicoyip.dev@xxxxxxxxx>
> > ---
> > Changes in v3:
> > - Move the headroom reservation into ip_tunnel_xmit() and
> >   ip_md_tunnel_xmit() instead of growing the skb inside the FOU/GUE
> >   builders.
> > - Use ip_encap_hlen() to reserve the final caller-side headroom before
> >   ip_tunnel_encap(), matching the existing IPv6 transmit pattern.
> > - Drop the IPv4 raw-pointer refreshes that were only needed when
> >   skb_cow_head() could run inside the encapsulation builders.
> >
> > Link: https://lore.kernel.org/netdev/20260808005956.3761487-1-nicoyip.dev@xxxxxxxxx/ [v2]
> > Link: https://lore.kernel.org/netdev/20260801060115.3538849-1-nicoyip.dev@xxxxxxxxx/ [v1]
> > ---
> >  net/ipv4/ip_tunnel.c | 28 +++++++++++++++++++++-------
> >  1 file changed, 21 insertions(+), 7 deletions(-)
> >
> > diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> > index 9d114bd575f9..5d5e7db11b3d 100644
> > --- a/net/ipv4/ip_tunnel.c
> > +++ b/net/ipv4/ip_tunnel.c
> > @@ -578,6 +578,7 @@ void ip_md_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
> >       const struct iphdr *inner_iph;
> >       struct rtable *rt = NULL;
> >       struct flowi4 fl4;
> > +     int encap_hlen;
> >       __be16 df = 0;
> >       u8 tos, ttl;
> >       bool use_cache;
> > @@ -601,11 +602,11 @@ void ip_md_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
> >                           tos & INET_DSCP_MASK, tunnel->net, 0, skb->mark,
> >                           skb_get_hash(skb), key->flow_flags);
> >
> > -     if (!tunnel_hlen)
> > -             tunnel_hlen = ip_encap_hlen(&tun_info->encap);
> > -
> > -     if (ip_tunnel_encap(skb, &tun_info->encap, &proto, &fl4) < 0)
> > +     encap_hlen = ip_encap_hlen(&tun_info->encap);
> > +     if (encap_hlen < 0)
> >               goto tx_error;
> > +     if (!tunnel_hlen)
> > +             tunnel_hlen = encap_hlen;
> >
> >       use_cache = ip_tunnel_dst_cache_usable(skb, tun_info);
> >       if (use_cache)
> > @@ -645,7 +646,8 @@ void ip_md_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
> >                       ttl = ip4_dst_hoplimit(&rt->dst);
> >       }
> >
> > -     headroom += LL_RESERVED_SPACE(rt->dst.dev) + rt->dst.header_len;
> > +     headroom += encap_hlen + LL_RESERVED_SPACE(rt->dst.dev) +
> > +                 rt->dst.header_len;
> >       if (skb_cow_head(skb, headroom)) {
> >               ip_rt_put(rt);
> >               goto tx_dropped;
> > @@ -653,6 +655,11 @@ void ip_md_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
> >
> >       ip_tunnel_adj_headroom(dev, headroom);
> >
> > +     if (ip_tunnel_encap(skb, &tun_info->encap, &proto, &fl4) < 0) {
> > +             ip_rt_put(rt);
> > +             goto tx_error;
> > +     }
> > +
> >       iptunnel_xmit(NULL, rt, skb, fl4.saddr, fl4.daddr, proto, tos, ttl,
> >                     df, !net_eq(tunnel->net, dev_net(dev)), 0);
> >       return;
>
> I don't think we need to change ip_md_tunnel_xmit(). It's not deriving
> the FOU/GUE encapsulation info from the device's configuration, so it's
> not exposed to the race described in the commit message. Instead, the
> encapsulation information is set by the bpf_skb_set_fou_encap() kfunc on
> the metadata dst attached to the skb.
>
> Mention this in the commit message, so that it's clear why only
> ip_tunnel_xmit() is patched.
>
> > @@ -677,6 +684,7 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
> >       __be16 payload_protocol;
> >       bool use_cache = false;
> >       struct flowi4 fl4;
> > +     int encap_hlen;
>
> Move this further down to maintain reverse xmas tree [2].
>
> [2] https://docs.kernel.org/next/process/maintainer-netdev.html#local-variable-ordering-reverse-xmas-tree-rcs
>
> >       bool md = false;
> >       bool connected;
> >       int err_count;
> > @@ -765,7 +773,8 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
> >                           tunnel->net, READ_ONCE(tunnel->parms.link),
> >                           tunnel->fwmark, skb_get_hash(skb), 0);
> >
> > -     if (ip_tunnel_encap(skb, &tunnel->encap, &protocol, &fl4) < 0)
> > +     encap_hlen = ip_encap_hlen(&tunnel->encap);
>
> This doesn't completely close the race. 'tunnel->encap' is still read
> twice. Here and in the call to ip_tunnel_encap() below. That's why the
> IPv6 fix takes a snapshot:
>
> ipencap = data_race(t->encap);
>
> And then continues to use 'ipencap' instead of 't->encap'.
>
> > +     if (encap_hlen < 0)
> >               goto tx_error;
> >
> >       if (connected && md) {
> > @@ -834,7 +843,7 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
> >       }
> >
> >       max_headroom = LL_RESERVED_SPACE(rt->dst.dev) + sizeof(struct iphdr)
> > -                     + rt->dst.header_len + ip_encap_hlen(&tunnel->encap);
> > +                     + rt->dst.header_len + encap_hlen;
> >
> >       if (skb_cow_head(skb, max_headroom)) {
> >               ip_rt_put(rt);
> > @@ -845,6 +854,11 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
> >
> >       ip_tunnel_adj_headroom(dev, max_headroom);
> >
> > +     if (ip_tunnel_encap(skb, &tunnel->encap, &protocol, &fl4) < 0) {
> > +             ip_rt_put(rt);
> > +             goto tx_error;
> > +     }
>
> tnl_update_pmtu() is calculating the size of the inner packet:
>
> pkt_size = skb->len - tunnel_hlen;
>
> And 'tunnel_hlen' includes the length of the encapsulation header which
> is no longer reflected in 'skb->len'. You need to pass the length of the
> encapsulation header as an argument to tnl_update_pmtu() and do:
>
> pkt_size = skb->len + encap_hlen - tunnel_hlen;
>
> Pass 0 from ip_md_tunnel_xmit() as there tnl_update_pmtu() is still
> called after ip_tunnel_encap().
>
> > +
> >       iptunnel_xmit(NULL, rt, skb, fl4.saddr, fl4.daddr, protocol, tos, ttl,
> >                     df, !net_eq(tunnel->net, dev_net(dev)), 0);
> >       return;
> > --
> > 2.43.0
> >

Sorry for the late response and thanks for the detailed guide on how
to fix, a v4 patch is just sent.

Best regards,
Chengfeng