Re: [PATCH net v4 1/1] ip6_tunnel: snapshot encap in xmit

[Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>]

[-- Attachment #1: Type: text/plain, Size: 4881 bytes --]

> On Sun, Sep 6, 2026 at 7:36 PM Lorenzo Bianconi
> <lorenzo.bianconi@oss.qualcomm.com> wrote:
> >
> > > From: Zixuan Chai <petalzu987@gmail.com>
> > >
> > > ip6_tnl_changelink() can update encapsulation parameters while the
> > > netdevice is transmitting packets. ip6_tnl_xmit() can calculate packet
> > > headroom with t->encap_hlen and later build an encapsulation header from
> > > the live t->encap. A concurrent update can change the encapsulation
> > > header between these accesses and make skb_push() underflow the skb head.
> > >
> > > Take a local snapshot of t->encap before calculating the encapsulation
> > > header length. Use that same snapshot for headroom accounting, metadata
> > > validation, and build_header(). This keeps all encapsulation decisions
> > > for an skb consistent even if changelink updates the live configuration.
> > >
> > > Fixes: b3a27b519b22 ("ip6_tunnel: Add support for fou/gue encapsulation")
> > > Cc: stable@vger.kernel.org
> > > Reported-by: Vega <vega@nebusec.ai>
> > > Assisted-by: LLM
> > > Signed-off-by: Zixuan Chai <petalzu987@gmail.com>
> > > Signed-off-by: Ren Wei <weir@nebusec.ai>
> >
> > Hi Ren and Zixuan,
> >
> > I agree this is a real issue, but I guess this patch is fixing just a
> > small part of more extended problem. In particular, there are multiple
> > parameters that are updated in ip6_tnl_update()/ip6_tnl_change() that are
> > accessed concurrently in ip6_tnl_xmit() or in ip6_tnl_fill_forward_path().
> > I guess we should try to find a general fix for the extended issue.
> > What do you think? We have probably the same issue in the IPv4 counterpart.
> 
> The general answer is : convert tunnels to RCU based configuration.
> 
> In my quest for RTNL-less ip link dumps, I converted SIT tunnels to
> RCU configuration.
> I was holding the series because the net-next queue is huge, my vxlan
> series was not merged yet.
> 
> <cover letter>
> 
> SIT (IPv6-in-IPv4) tunnel configuration and status reporting have
> historically relied on the RTNL lock for synchronization. Consequently,
> netlink dumps via ipip6_fill_info() had to run with RTNL held, adding
> contention during network device dumps.
> 
> At the same time, the transmit path (dev->lltx == true), tunnel lookups,
> and error handling run locklessly and can race with configuration
> updates. This can result in torn reads of multi-word fields (such as the
> 128-bit 6RD IPv6 prefix) or transiently zeroed encapsulation parameters.
> 
> Furthermore, ipip6_tunnel_update() currently unhashes, re-hashes, and
> calls synchronize_net() unconditionally, even when the tunnel endpoint
> addresses (saddr and daddr) have not changed.
> 
> This patch series modernizes SIT parameter management to use RCU
> protection, fixes existing race conditions, optimizes tunnel updates,
> and removes the RTNL requirement from ipip6_fill_info():
> 
> - Patch 1 fixes a pre-existing UAF in PRL (Potential Router List)
>   deletion where call_rcu() was invoked before unlinking t->prl.
> - Patch 2 removes the unsafe in-place memset() in ip_tunnel_encap_setup()
>   and uses WRITE_ONCE() to prevent lockless readers from observing
>   transiently zeroed or torn fields.
> - Patch 3 annotates data races on tunnel->fwmark with READ_ONCE() and
>   WRITE_ONCE().
> - Patch 4 converts 6RD configuration (tunnel->ip6rd) to an RCU-protected
>   pointer, preventing torn reads on the 128-bit IPv6 prefix.
> - Patch 5 implements a dedicated ipip6_get_iflink() callback to decouple
>   SIT parameter handling from generic ip_tunnel.
> - Patch 6 dynamically allocates struct ip_tunnel_parm_kern (sit_parms)
>   as a preparatory step.
> - Patch 7 converts tunnel->sit_parms to full RCU protection. Updates
>   publish new parameters via rcu_assign_pointer() and free the old ones
>   via kfree_rcu(). When saddr and daddr do not change, unhashing,
>   re-hashing, and synchronize_net() are completely bypassed.
> - Patch 8 wraps attribute serialization in ipip6_fill_info() under
>   rcu_read_lock(), eliminating the reliance on the RTNL lock.

ack, nice. This is exactly I meant :)

Regards,
Lorenzo

> 
> Eric Dumazet (8):
>       sit: fix UAF in ipip6_tunnel_del_prl()
>       ip_tunnel: use WRITE_ONCE in ip_tunnel_encap_setup
>       sit: annotate data-races around tunnel->fwmark
>       sit: convert 6RD configuration to RCU protection
>       sit: implement ipip6_get_iflink()
>       sit: dynamically allocate struct ip_tunnel_parm_kern
>       sit: convert configuration to RCU protection
>       sit: no longer rely on RTNL in ipip6_fill_info()
> 
>  include/net/ip_tunnels.h |   5 +-
>  net/ipv4/ip_tunnel.c     |  14 +-
>  net/ipv6/sit.c           | 475 ++++++++++++++++++++++++++++++++---------------
>  3 files changed, 338 insertions(+), 156 deletions(-)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]