From: Eric Dumazet <edumazet@google.com> Date: 2026-09-07 07:58:50
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 addresses PRL issues, modernizes SIT parameter
management to use RCU protection, 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 adds GFP_KERNEL_ACCOUNT to struct ip_tunnel_prl_entry
allocations in ipip6_tunnel_add_prl().
- Patch 3 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 4 annotates data races on tunnel->fwmark with READ_ONCE() and
WRITE_ONCE().
- Patch 5 converts 6RD configuration (tunnel->ip6rd) to an RCU-protected
pointer, preventing torn reads on the 128-bit IPv6 prefix.
- Patch 6 implements a dedicated ipip6_get_iflink() callback to decouple
SIT parameter handling from generic ip_tunnel.
- Patch 7 dynamically allocates struct ip_tunnel_parm_kern (sit_parms)
as a preparatory step.
- Patch 8 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 9 wraps attribute serialization in ipip6_fill_info() under
rcu_read_lock(), eliminating the reliance on the RTNL lock.
Eric Dumazet (9):
sit: fix UAF in ipip6_tunnel_del_prl()
sit: charge ip_tunnel_prl_entry allocations to memcg
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 | 477 +++++++++++++++++++++++++++------------
3 files changed, 339 insertions(+), 157 deletions(-)
--
2.55.0.979.g7e5102b832-goog
From: Eric Dumazet <edumazet@google.com> Date: 2026-09-07 07:58:51
When flushing all potential router list (PRL) entries,
ipip6_tunnel_del_prl() called call_rcu(&x->rcu_head, prl_list_destroy_rcu)
before clearing t->prl.
A concurrent reader in isatap_chksrc() could enter an RCU read-side
critical section after call_rcu() but before t->prl is set to NULL,
allowing prl_list_destroy_rcu() to free nodes while the reader traverses
them.
Clear t->prl using RCU_INIT_POINTER() before invoking call_rcu().
Also use rcu_assign_pointer() when unlinking individual entries.
Fixes: ef9a9d1183b3 ("ipv6 sit: RCU conversion phase I")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/sit.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Eric Dumazet <edumazet@google.com> Date: 2026-09-07 07:58:52
ipip6_tunnel_add_prl() allocates struct ip_tunnel_prl_entry for each
potential router entry added via SIOCADDPRL.
An unprivileged user with CAP_NET_ADMIN in a user namespace can add
an unbounded number of entries, consuming uncharged kernel memory.
Commit 1b51d8271973 ("memcg: ipv6/sit: account and don't WARN on
ip_tunnel_prl structs allocation") added accounting to the temporary
buffer in ipip6_tunnel_get_prl(), but missed the entry allocations
in ipip6_tunnel_add_prl().
Use GFP_KERNEL_ACCOUNT when allocating struct ip_tunnel_prl_entry.
Fixes: 1b51d8271973 ("memcg: ipv6/sit: account and don't WARN on ip_tunnel_prl structs allocation")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/sit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Eric Dumazet <edumazet@google.com> Date: 2026-09-07 07:58:53
Update ip_tunnel_encap_setup() to use WRITE_ONCE() when writing
to encap fields (type, sport, dport, flags) and hlen fields.
This ensures that concurrent lockless readers (like fill_info)
do not see torn writes.
Also remove the unsafe memset() on t->encap which could cause
concurrent readers to transiently see zeroed fields.
Removing it also fixes a bug where t->encap was left cleared
even if ip_encap_hlen() failed, resulting in partial configuration.
Fixes: 56328486539d ("net: Changes to ip_tunnel to support foo-over-udp encapsulation")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/ip_tunnel.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
From: Eric Dumazet <edumazet@google.com> Date: 2026-09-07 07:58:55
ipip6_tunnel_xmit() runs locklessly (dev->lltx == true) and reads
tunnel->fwmark.
In preparation for converting ipip6_fill_info() to run without RTNL,
add READ_ONCE() and WRITE_ONCE() annotations around tunnel->fwmark.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/sit.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
From: Eric Dumazet <edumazet@google.com> Date: 2026-09-07 07:58:56
In order to allow lockless readers in future patches, convert
'tunnel->ip6rd' to an RCU protected pointer.
Updating 6RD configuration via ipip6_tunnel_update_6rd() or
ipip6_tunnel_clone_6rd() now allocates a struct ip_tunnel_6rd_parm and
uses rcu_assign_pointer() to publish it, freeing the previous
parameters with kfree_rcu().
Readers in check_6rd() and only_dnatted() use rcu_dereference() under
existing RCU read lock, preventing torn reads on the 128-bit IPv6
prefix.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/ip_tunnels.h | 3 +-
net/ipv6/sit.c | 160 ++++++++++++++++++++++++++++-----------
2 files changed, 117 insertions(+), 46 deletions(-)
@@ -185,7 +186,7 @@ struct ip_tunnel {/* for SIT */#ifdef CONFIG_IPV6_SIT_6RD-structip_tunnel_6rd_parmip6rd;+structip_tunnel_6rd_parm__rcu*ip6rd;#endifstructip_tunnel_prl_entry__rcu*prl;/* potential router list */unsignedintprl_count;/* # of entries in PRL */
@@ -1863,17 +1929,21 @@ static int __net_init sit_init_net(struct net *net)t=netdev_priv(sitn->fb_tunnel_dev);t->net=net;+err=ipip6_tunnel_clone_6rd(sitn->fb_tunnel_dev,sitn);+if(err<0)+gotoerr_reg_dev;+err=register_netdev(sitn->fb_tunnel_dev);if(err)gotoerr_reg_dev;-ipip6_tunnel_clone_6rd(sitn->fb_tunnel_dev,sitn);ipip6_fb_tunnel_init(sitn->fb_tunnel_dev);strscpy(t->parms.name,sitn->fb_tunnel_dev->name);return0;err_reg_dev:+ipip6_dev_free(sitn->fb_tunnel_dev);free_netdev(sitn->fb_tunnel_dev);err_alloc_dev:returnerr;
From: Eric Dumazet <edumazet@google.com> Date: 2026-09-07 07:58:58
SIT currently uses ip_tunnel_get_iflink() which reads tunnel->parms.link.
In order to decouple SIT parameter handling from ip_tunnel, implement
its own ndo_get_iflink callback.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/sit.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
@@ -190,6 +190,7 @@ struct ip_tunnel {#endifstructip_tunnel_prl_entry__rcu*prl;/* potential router list */unsignedintprl_count;/* # of entries in PRL */+structip_tunnel_parm_kern*sit_parms;unsignedintip_tnl_net_id;structgro_cellsgro_cells;__u32fwmark;
@@ -938,9 +942,9 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,structnet_device*dev){structip_tunnel*tunnel=netdev_priv(dev);-conststructiphdr*tiph=&tunnel->parms.iph;+conststructiphdr*tiph=&tunnel->sit_parms->iph;conststructipv6hdr*iph6=ipv6_hdr(skb);-u8tos=tunnel->parms.iph.tos;+u8tos=tunnel->sit_parms->iph.tos;__be16df=tiph->frag_off;structrtable*rt;/* Route to the other host */structnet_device*tdev;/* Device to other host */
@@ -1935,6 +1956,17 @@ static int __net_init sit_init_net(struct net *net)t=netdev_priv(sitn->fb_tunnel_dev);t->net=net;+t->sit_parms=kzalloc_obj(*t->sit_parms);+if(!t->sit_parms){+err=-ENOMEM;+gotoerr_reg_dev;+}+t->sit_parms->iph.version=4;+t->sit_parms->iph.protocol=IPPROTO_IPV6;+t->sit_parms->iph.ihl=5;+t->sit_parms->iph.ttl=64;+strscpy(t->sit_parms->name,sitn->fb_tunnel_dev->name);+t->parms=*t->sit_parms;err=ipip6_tunnel_clone_6rd(sitn->fb_tunnel_dev,sitn);if(err<0)
@@ -1945,8 +1977,6 @@ static int __net_init sit_init_net(struct net *net)gotoerr_reg_dev;ipip6_fb_tunnel_init(sitn->fb_tunnel_dev);--strscpy(t->parms.name,sitn->fb_tunnel_dev->name);return0;err_reg_dev:
From: Eric Dumazet <edumazet@google.com> Date: 2026-09-07 07:59:01
Now that SIT parameters are dynamically allocated, convert
tunnel->sit_parms to an RCU-protected pointer.
Updates in ipip6_tunnel_update() allocate a new parameter block,
publish it using rcu_assign_pointer(), and free the old one
via kfree_rcu().
We only need to unlink and re-link the tunnel in the hash table
if either saddr or daddr changed. When neither address changes,
the unhash/re-hash and synchronize_net() can be completely skipped.
Readers in ipip6_tunnel_lookup(), ipip6_tunnel_xmit(), ipip6_err(),
and ipip6_rcv() now safely dereference tunnel->sit_parms under RCU.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/ip_tunnels.h | 3 +-
net/ipv6/sit.c | 245 +++++++++++++++++++++++++--------------
2 files changed, 157 insertions(+), 91 deletions(-)
@@ -190,7 +191,7 @@ struct ip_tunnel {#endifstructip_tunnel_prl_entry__rcu*prl;/* potential router list */unsignedintprl_count;/* # of entries in PRL */-structip_tunnel_parm_kern*sit_parms;+structip_tunnel_parm_kern__rcu*sit_parms;unsignedintip_tnl_net_id;structgro_cellsgro_cells;__u32fwmark;
@@ -942,20 +967,27 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,structnet_device*dev){structip_tunnel*tunnel=netdev_priv(dev);-conststructiphdr*tiph=&tunnel->sit_parms->iph;+conststructip_tunnel_parm_kern*parms;+conststructiphdr*tiph;conststructipv6hdr*iph6=ipv6_hdr(skb);-u8tos=tunnel->sit_parms->iph.tos;-__be16df=tiph->frag_off;+u8tos;+__be16df;structrtable*rt;/* Route to the other host */structnet_device*tdev;/* Device to other host */unsignedintmax_headroom;/* The extra header space needed */-__be32dst=tiph->daddr;+__be32dst;interr_count,mtu;structflowi4fl4;u8ttl;u8protocol=IPPROTO_IPV6;intt_hlen=tunnel->hlen+sizeof(structiphdr);+parms=rcu_dereference(tunnel->sit_parms);+tiph=&parms->iph;+tos=parms->iph.tos;+df=tiph->frag_off;+dst=tiph->daddr;+if(tos==1)tos=ipv6_get_dsfield(iph6);
@@ -1956,17 +2020,18 @@ static int __net_init sit_init_net(struct net *net)t=netdev_priv(sitn->fb_tunnel_dev);t->net=net;-t->sit_parms=kzalloc_obj(*t->sit_parms);-if(!t->sit_parms){+nt_parms=kzalloc_obj(*nt_parms);+if(!nt_parms){err=-ENOMEM;gotoerr_reg_dev;}-t->sit_parms->iph.version=4;-t->sit_parms->iph.protocol=IPPROTO_IPV6;-t->sit_parms->iph.ihl=5;-t->sit_parms->iph.ttl=64;-strscpy(t->sit_parms->name,sitn->fb_tunnel_dev->name);-t->parms=*t->sit_parms;+nt_parms->iph.version=4;+nt_parms->iph.protocol=IPPROTO_IPV6;+nt_parms->iph.ihl=5;+nt_parms->iph.ttl=64;+strscpy(nt_parms->name,sitn->fb_tunnel_dev->name);+t->parms=*nt_parms;+rcu_assign_pointer(t->sit_parms,nt_parms);err=ipip6_tunnel_clone_6rd(sitn->fb_tunnel_dev,sitn);if(err<0)
From: Eric Dumazet <edumazet@google.com> Date: 2026-09-07 07:59:03
Now that SIT parameters and 6RD parameters are RCU-protected,
and fwmark/encap are annotated with READ_ONCE()/WRITE_ONCE(),
ipip6_fill_info() no longer requires RTNL to be held.
Wrap the attribute serialization in rcu_read_lock() and rcu_read_unlock().
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv6/sit.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
From: Lorenzo Bianconi <hidden> Date: 2026-09-07 12:28:24
When flushing all potential router list (PRL) entries,
ipip6_tunnel_del_prl() called call_rcu(&x->rcu_head, prl_list_destroy_rcu)
before clearing t->prl.
A concurrent reader in isatap_chksrc() could enter an RCU read-side
critical section after call_rcu() but before t->prl is set to NULL,
allowing prl_list_destroy_rcu() to free nodes while the reader traverses
them.
Clear t->prl using RCU_INIT_POINTER() before invoking call_rcu().
Also use rcu_assign_pointer() when unlinking individual entries.
Fixes: ef9a9d1183b3 ("ipv6 sit: RCU conversion phase I")
Signed-off-by: Eric Dumazet <edumazet@google.com>
From: Lorenzo Bianconi <hidden> Date: 2026-09-07 12:35:35
ipip6_tunnel_add_prl() allocates struct ip_tunnel_prl_entry for each
potential router entry added via SIOCADDPRL.
An unprivileged user with CAP_NET_ADMIN in a user namespace can add
an unbounded number of entries, consuming uncharged kernel memory.
Commit 1b51d8271973 ("memcg: ipv6/sit: account and don't WARN on
ip_tunnel_prl structs allocation") added accounting to the temporary
buffer in ipip6_tunnel_get_prl(), but missed the entry allocations
in ipip6_tunnel_add_prl().
Use GFP_KERNEL_ACCOUNT when allocating struct ip_tunnel_prl_entry.
Fixes: 1b51d8271973 ("memcg: ipv6/sit: account and don't WARN on ip_tunnel_prl structs allocation")
Signed-off-by: Eric Dumazet <edumazet@google.com>
From: Lorenzo Bianconi <hidden> Date: 2026-09-07 13:00:28
In order to allow lockless readers in future patches, convert
'tunnel->ip6rd' to an RCU protected pointer.
Updating 6RD configuration via ipip6_tunnel_update_6rd() or
ipip6_tunnel_clone_6rd() now allocates a struct ip_tunnel_6rd_parm and
uses rcu_assign_pointer() to publish it, freeing the previous
parameters with kfree_rcu().
Readers in check_6rd() and only_dnatted() use rcu_dereference() under
existing RCU read lock, preventing torn reads on the 128-bit IPv6
prefix.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Hi Eric,
just few nits inline.
Acked-by: Lorenzo Bianconi <redacted>
Regards,
Lorenzo
@@ -185,7 +186,7 @@ struct ip_tunnel {/* for SIT */#ifdef CONFIG_IPV6_SIT_6RD-structip_tunnel_6rd_parmip6rd;+structip_tunnel_6rd_parm__rcu*ip6rd;#endifstructip_tunnel_prl_entry__rcu*prl;/* potential router list */unsignedintprl_count;/* # of entries in PRL */
From: Lorenzo Bianconi <hidden> Date: 2026-09-07 13:01:53
SIT currently uses ip_tunnel_get_iflink() which reads tunnel->parms.link.
In order to decouple SIT parameter handling from ip_tunnel, implement
its own ndo_get_iflink callback.
Signed-off-by: Eric Dumazet <edumazet@google.com>
From: Lorenzo Bianconi <hidden> Date: 2026-09-07 13:16:56
In preparation for converting SIT configuration parameters to RCU,
dynamically allocate struct ip_tunnel_parm_kern as sit_parms in
struct ip_tunnel.
Signed-off-by: Eric Dumazet <edumazet@google.com>
@@ -190,6 +190,7 @@ struct ip_tunnel {#endifstructip_tunnel_prl_entry__rcu*prl;/* potential router list */unsignedintprl_count;/* # of entries in PRL */+structip_tunnel_parm_kern*sit_parms;unsignedintip_tnl_net_id;structgro_cellsgro_cells;__u32fwmark;
@@ -938,9 +942,9 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,structnet_device*dev){structip_tunnel*tunnel=netdev_priv(dev);-conststructiphdr*tiph=&tunnel->parms.iph;+conststructiphdr*tiph=&tunnel->sit_parms->iph;conststructipv6hdr*iph6=ipv6_hdr(skb);-u8tos=tunnel->parms.iph.tos;+u8tos=tunnel->sit_parms->iph.tos;__be16df=tiph->frag_off;structrtable*rt;/* Route to the other host */structnet_device*tdev;/* Device to other host */
@@ -1935,6 +1956,17 @@ static int __net_init sit_init_net(struct net *net)t=netdev_priv(sitn->fb_tunnel_dev);t->net=net;+t->sit_parms=kzalloc_obj(*t->sit_parms);+if(!t->sit_parms){+err=-ENOMEM;+gotoerr_reg_dev;+}+t->sit_parms->iph.version=4;+t->sit_parms->iph.protocol=IPPROTO_IPV6;+t->sit_parms->iph.ihl=5;+t->sit_parms->iph.ttl=64;+strscpy(t->sit_parms->name,sitn->fb_tunnel_dev->name);+t->parms=*t->sit_parms;err=ipip6_tunnel_clone_6rd(sitn->fb_tunnel_dev,sitn);if(err<0)
@@ -1945,8 +1977,6 @@ static int __net_init sit_init_net(struct net *net)gotoerr_reg_dev;ipip6_fb_tunnel_init(sitn->fb_tunnel_dev);--strscpy(t->parms.name,sitn->fb_tunnel_dev->name);return0;err_reg_dev:
At this point in the series old_p is freed with no grace period: the
synchronize_net() runs before the pointer switch and only covers the
hash readers. ipip6_tunnel_xmit() doesn't go through the hash and
keeps tiph = &tunnel->sit_parms->iph across the route lookup, and
ipip6_get_iflink() is reached from iflink_show() with no lock at all.
8/9 turns this into kfree_rcu(), so the end result is fine, but this
commit on its own is a UAF and breaks bisection. Moving the rcu_head
and kfree_rcu() here (or squashing 7 and 8) would keep each step safe.
Smaller thing of the same kind in 5/9: check_6rd() gets
rcu_dereference() but the rcu_read_lock() around the xmit path only
arrives in 8/9.
Artem
At this point in the series old_p is freed with no grace period: the
synchronize_net() runs before the pointer switch and only covers the
hash readers. ipip6_tunnel_xmit() doesn't go through the hash and
keeps tiph = &tunnel->sit_parms->iph across the route lookup, and
ipip6_get_iflink() is reached from iflink_show() with no lock at all.
8/9 turns this into kfree_rcu(), so the end result is fine, but this
commit on its own is a UAF and breaks bisection. Moving the rcu_head
and kfree_rcu() here (or squashing 7 and 8) would keep each step safe.
Smaller thing of the same kind in 5/9: check_6rd() gets
rcu_dereference() but the rcu_read_lock() around the xmit path only
arrives in 8/9.
Yeah, I had to split my original patch into three parts because our
sashiko 5,000,000 token limit was hit.
From: Lorenzo Bianconi <hidden> Date: 2026-09-07 14:33:00
Now that SIT parameters are dynamically allocated, convert
tunnel->sit_parms to an RCU-protected pointer.
Updates in ipip6_tunnel_update() allocate a new parameter block,
publish it using rcu_assign_pointer(), and free the old one
via kfree_rcu().
We only need to unlink and re-link the tunnel in the hash table
if either saddr or daddr changed. When neither address changes,
the unhash/re-hash and synchronize_net() can be completely skipped.
Readers in ipip6_tunnel_lookup(), ipip6_tunnel_xmit(), ipip6_err(),
and ipip6_rcv() now safely dereference tunnel->sit_parms under RCU.
I think this patch is fine, I am just wondering if we can use more generic name
with respect to 'sit_parms' since I guess we have the same issue for IPIP and
IP6IP6 tunnels. Do you prefer to have dedicated pointers for them?
Acked-by: Lorenzo Bianconi <redacted>
Regards,
Lorenzo
@@ -190,7 +191,7 @@ struct ip_tunnel {#endifstructip_tunnel_prl_entry__rcu*prl;/* potential router list */unsignedintprl_count;/* # of entries in PRL */-structip_tunnel_parm_kern*sit_parms;+structip_tunnel_parm_kern__rcu*sit_parms;unsignedintip_tnl_net_id;structgro_cellsgro_cells;__u32fwmark;
@@ -942,20 +967,27 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,structnet_device*dev){structip_tunnel*tunnel=netdev_priv(dev);-conststructiphdr*tiph=&tunnel->sit_parms->iph;+conststructip_tunnel_parm_kern*parms;+conststructiphdr*tiph;conststructipv6hdr*iph6=ipv6_hdr(skb);-u8tos=tunnel->sit_parms->iph.tos;-__be16df=tiph->frag_off;+u8tos;+__be16df;structrtable*rt;/* Route to the other host */structnet_device*tdev;/* Device to other host */unsignedintmax_headroom;/* The extra header space needed */-__be32dst=tiph->daddr;+__be32dst;interr_count,mtu;structflowi4fl4;u8ttl;u8protocol=IPPROTO_IPV6;intt_hlen=tunnel->hlen+sizeof(structiphdr);+parms=rcu_dereference(tunnel->sit_parms);+tiph=&parms->iph;+tos=parms->iph.tos;+df=tiph->frag_off;+dst=tiph->daddr;+if(tos==1)tos=ipv6_get_dsfield(iph6);
@@ -1956,17 +2020,18 @@ static int __net_init sit_init_net(struct net *net)t=netdev_priv(sitn->fb_tunnel_dev);t->net=net;-t->sit_parms=kzalloc_obj(*t->sit_parms);-if(!t->sit_parms){+nt_parms=kzalloc_obj(*nt_parms);+if(!nt_parms){err=-ENOMEM;gotoerr_reg_dev;}-t->sit_parms->iph.version=4;-t->sit_parms->iph.protocol=IPPROTO_IPV6;-t->sit_parms->iph.ihl=5;-t->sit_parms->iph.ttl=64;-strscpy(t->sit_parms->name,sitn->fb_tunnel_dev->name);-t->parms=*t->sit_parms;+nt_parms->iph.version=4;+nt_parms->iph.protocol=IPPROTO_IPV6;+nt_parms->iph.ihl=5;+nt_parms->iph.ttl=64;+strscpy(nt_parms->name,sitn->fb_tunnel_dev->name);+t->parms=*nt_parms;+rcu_assign_pointer(t->sit_parms,nt_parms);err=ipip6_tunnel_clone_6rd(sitn->fb_tunnel_dev,sitn);if(err<0)
From: Eric Dumazet <edumazet@google.com> Date: 2026-09-07 14:42:24
On Mon, Sep 7, 2026 at 4:33 PM Lorenzo Bianconi
[off-list ref] wrote:
quoted
Now that SIT parameters are dynamically allocated, convert
tunnel->sit_parms to an RCU-protected pointer.
Updates in ipip6_tunnel_update() allocate a new parameter block,
publish it using rcu_assign_pointer(), and free the old one
via kfree_rcu().
We only need to unlink and re-link the tunnel in the hash table
if either saddr or daddr changed. When neither address changes,
the unhash/re-hash and synchronize_net() can be completely skipped.
Readers in ipip6_tunnel_lookup(), ipip6_tunnel_xmit(), ipip6_err(),
and ipip6_rcv() now safely dereference tunnel->sit_parms under RCU.
I think this patch is fine, I am just wondering if we can use more generic name
with respect to 'sit_parms' since I guess we have the same issue for IPIP and
IP6IP6 tunnels. Do you prefer to have dedicated pointers for them?
Name could be more generic indeed.
I was thinking of using a union for future tunnel conversions.
Acked-by: Lorenzo Bianconi <redacted>
Regards,
Lorenzo
From: Lorenzo Bianconi <hidden> Date: 2026-09-07 15:04:32
Now that SIT parameters and 6RD parameters are RCU-protected,
and fwmark/encap are annotated with READ_ONCE()/WRITE_ONCE(),
ipip6_fill_info() no longer requires RTNL to be held.
Wrap the attribute serialization in rcu_read_lock() and rcu_read_unlock().
From: Lorenzo Bianconi <hidden> Date: 2026-09-07 15:12:23
Update ip_tunnel_encap_setup() to use WRITE_ONCE() when writing
to encap fields (type, sport, dport, flags) and hlen fields.
This ensures that concurrent lockless readers (like fill_info)
do not see torn writes.
Also remove the unsafe memset() on t->encap which could cause
concurrent readers to transiently see zeroed fields.
Removing it also fixes a bug where t->encap was left cleared
even if ip_encap_hlen() failed, resulting in partial configuration.
Fixes: 56328486539d ("net: Changes to ip_tunnel to support foo-over-udp encapsulation")
Signed-off-by: Eric Dumazet <edumazet@google.com>
From: Lorenzo Bianconi <hidden> Date: 2026-09-07 15:13:04
ipip6_tunnel_xmit() runs locklessly (dev->lltx == true) and reads
tunnel->fwmark.
In preparation for converting ipip6_fill_info() to run without RTNL,
add READ_ONCE() and WRITE_ONCE() annotations around tunnel->fwmark.
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 1 (2) · Low: 0
- [Medium] The mirror helper ip6_tnl_encap_setup()
(net/ipv6/ip6_tunnel.c:1816) is left unconverted, so the exact bug…
Pre-existing issues:
- [Medium] ip_tunnel_encap_setup() recomputes t->hlen as 'hlen +
t->tun_hlen' (net/ipv4/ip_tunnel.c:504), which disagrees with the…
- [Medium] The six encap/hlen fields are published as independent
WRITE_ONCE() stores with no seqcount, RCU pointer swap or release…
--
ip_tunnel: use WRITE_ONCE in ip_tunnel_encap_setup
This converts the encap field and hlen stores in ip_tunnel_encap_setup()
to WRITE_ONCE() so lockless readers such as fill_info cannot observe torn
values. It also drops the memset() of t->encap, which both removed a
transient all-zero window for readers and stopped leaving t->encap cleared
when ip_encap_hlen() fails.
No Link: tags present in the commit header.
@@ -491,19 +491,17 @@ int ip_tunnel_encap_setup(struct ip_tunnel *t,{inthlen;-memset(&t->encap,0,sizeof(t->encap));-hlen=ip_encap_hlen(ipencap);if(hlen<0)returnhlen;
[Severity: Medium]
Should the IPv6 twin get the same treatment? ip6_tnl_encap_setup() in
net/ipv6/ip6_tunnel.c still has the original shape at this commit:
net/ipv6/ip6_tunnel.c:ip6_tnl_encap_setup() {
memset(&t->encap, 0, sizeof(t->encap));
hlen = ip6_encap_hlen(ipencap);
if (hlen < 0)
return hlen;
t->encap.type = ipencap->type;
t->encap.sport = ipencap->sport;
t->encap.dport = ipencap->dport;
t->encap.flags = ipencap->flags;
t->encap_hlen = hlen;
t->hlen = t->encap_hlen + t->tun_hlen;
}
So the "t->encap was left cleared even if ip_encap_hlen() failed" case
described in the changelog is still reachable for ip6tnl and ip6gre, on
already configured tunnels:
ip6_tnl_changelink()
ip6_tnl_encap_setup(t, &ipencap) /* t is live */
memset(&t->encap, 0, ...)
ip6_encap_hlen() returns -EINVAL /* e.g. fou6 ops not registered,
or type >= MAX_IPTUN_ENCAP_OPS */
return hlen
ip6gre_changelink_common() reaches the same helper the same way. The
netlink command fails, but the live tunnel's encapsulation configuration
has already been wiped.
The plain stores after the memset in ip6_tnl_encap_setup() also leave the
transient all-zero encap visible to ip6_tnl_fill_info()/ip6gre_fill_info(),
which run without RTNL since RTM_GETLINK is registered with
RTNL_FLAG_DOIT_UNLOCKED | RTNL_FLAG_DUMP_UNLOCKED in
net/core/rtnetlink.c, and to the ip6 xmit path.
[Severity: Medium]
This is a pre-existing issue, not something this patch introduces, but is
it worth noting in the changelog that WRITE_ONCE() only removes torn reads
and does not give readers a coherent snapshot?
encap.type, the field that gates encapsulation, is still published first,
with no seqcount or RCU pointer swap, so a reader can pair the new type
with the stale sport/dport/flags or with the stale (smaller) encap_hlen and
hlen. The same ordering existed before the patch (with zeroes instead of
stale values), so nothing gets worse here.
On the transmit side the two reads are separated:
net/ipv6/sit.c:ipip6_tunnel_xmit() {
int t_hlen = tunnel->hlen + sizeof(struct iphdr);
...
max_headroom = LL_RESERVED_SPACE(tdev) + t_hlen;
if (skb_headroom(skb) < max_headroom || ...)
new_skb = skb_realloc_headroom(skb, max_headroom);
...
if (ip_tunnel_encap(skb, &tunnel->encap, &protocol, &fl4) < 0) {
}
The headroom comes from the earlier hlen snapshot while ip_tunnel_encap()
reads the live tunnel->encap, and fou_build_udp()/__gue_build_header() use
skb_push(). I could not show that the reserved headroom is actually
exceeded, since skb_realloc_headroom()/__skb_cow() round expansions up, so
this part is speculative.
[Severity: Medium]
This isn't a bug introduced by this patch, since the formula is unchanged
(t->encap_hlen + t->tun_hlen before, hlen + t->tun_hlen after), but does
this hlen formula disagree with the ERSPAN one?
net/ipv4/ip_gre.c:erspan_tunnel_init() {
tunnel->hlen = tunnel->tun_hlen + tunnel->encap_hlen +
erspan_hdr_len(tunnel->erspan_ver);
}
erspan_changelink() reaches this helper on an already initialised device:
erspan_changelink()
ipgre_newlink_encap_setup()
ip_tunnel_encap_setup() /* last writer of t->hlen */
ndo_init does not run again on that path, so an
"ip link set <erspan> type erspan encap ..." command appears to drop
erspan_hdr_len() (12 bytes for v1, 16 for v2) from t->hlen.
ip_tunnel_bind_dev() then derives both the MTU and
dev->needed_headroom = ip_tunnel_limit_headroom(t_hlen + hlen);
from the smaller value, while erspan_xmit() only does
skb_cow_head(skb, dev->needed_headroom) before pushing the erspan header,
the GRE header, the encap header and the outer IPv4 header.
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 0 · Low: 1
- [Low] Incomplete fwmark annotation for sit devices:
ipip6_tunnel_update() now publishes tunnel->fwmark with WRITE_ONCE()…
--
sit: annotate data-races around tunnel->fwmark
This adds READ_ONCE()/WRITE_ONCE() annotations around tunnel->fwmark in
the sit driver, since ipip6_tunnel_xmit() runs locklessly with
dev->lltx == true. It is preparation for running ipip6_fill_info()
without RTNL.
[Severity: Low]
Should the other lockless transmit path of the same sit device also be
annotated?
ipip6_tunnel_xmit() is only one of the two xmit paths. For IPv4 and MPLS
payloads, sit_tunnel_xmit() dispatches elsewhere:
net/ipv6/sit.c:sit_tunnel_xmit() {
switch (skb->protocol) {
case htons(ETH_P_IP):
sit_tunnel_xmit__(skb, dev, IPPROTO_IPIP);
break;
case htons(ETH_P_IPV6):
ipip6_tunnel_xmit(skb, dev);
break;
#if IS_ENABLED(CONFIG_MPLS)
case htons(ETH_P_MPLS_UC):
sit_tunnel_xmit__(skb, dev, IPPROTO_MPLS);
break;
#endif
...
}
and sit_tunnel_xmit__() -> ip_tunnel_xmit() reads the same field with a
plain load, right next to an already annotated read:
net/ipv4/ip_tunnel.c:ip_tunnel_xmit() {
...
ip_tunnel_init_flow(&fl4, protocol, dst, tnl_params->saddr,
tunnel->parms.o_key, tos & INET_DSCP_MASK,
tunnel->net, READ_ONCE(tunnel->parms.link),
tunnel->fwmark, skb_get_hash(skb), 0);
...
}
So for a sit tunnel carrying IPv4 or MPLS frames, an RTNL writer running
ipip6_changelink()/ioctl -> ipip6_tunnel_update() -> WRITE_ONCE(t->fwmark,
fwmark) still races with an unmarked read, and KCSAN can still report it.
The field is a naturally aligned u32, so the read cannot tear; the visible
effects are the KCSAN report plus one route lookup using the pre-update
mark.
Note that the generic writer in net/ipv4/ip_tunnel.c:ip_tunnel_update() is
also still a plain store to t->fwmark, so closing this would mean touching
the generic ip_tunnel code as well, which may be outside the intended scope
of this series. It was checked at the end of the series and the plain read
in ip_tunnel_xmit() is still there.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907075846.2913645-1-edumazet%40google.com
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 1 · Low: 1
- [Medium] check_6rd() now performs `ip6rd =
rcu_dereference(tunnel->ip6rd)` (net/ipv6/sit.c:849), but check_6rd()…
- [Low] The patch changes SIOCDEL6RD behaviour beyond the advertised RCU
conversion: ipip6_tunnel_clone_6rd() now, when an old 6RD…
--
sit: convert 6RD configuration to RCU protection
This converts tunnel->ip6rd from an embedded struct into an RCU protected
pointer, so updates via ipip6_tunnel_update_6rd() and
ipip6_tunnel_clone_6rd() allocate a new struct ip_tunnel_6rd_parm, publish
it with rcu_assign_pointer() and free the old one with kfree_rcu().
Readers in check_6rd(), only_dnatted(), ipip6_tunnel_get6rd() and
ipip6_fill_info() are updated accordingly.
[Severity: Low]
Is the new dst_cache_reset() plus netdev_state_change() in
ipip6_tunnel_clone_6rd() intended as part of this conversion?
Before this patch, the SIOCDEL6RD path did neither:
ipip6_tunnel_6rdctl()
ipip6_tunnel_clone_6rd() /* just overwrote t->ip6rd fields */
Only ipip6_tunnel_update_6rd() invalidated the destination cache and
notified userspace. Since ipip6_tunnel_xmit() derives the IPv4
destination from the 6RD configuration via try_6rd() and then consults
dst_cache_get_ip4(&tunnel->dst_cache, ...), the added reset changes where
packets go after a 6RD deletion, and netdev_state_change() ->
netif_state_change() emits an extra NETDEV_CHANGE plus RTM_NEWLINK that
link watchers will see.
Would it make sense to split that out into its own patch, with a Fixes:
tag if it is fixing stale routing after SIOCDEL6RD? The changelog
currently only describes the allocation and publish conversion.
[ ... ]
[Severity: Medium]
Is plain rcu_dereference() the right accessor here? The changelog says:
"Readers in check_6rd() and only_dnatted() use rcu_dereference() under
existing RCU read lock"
That holds for the receive path, but check_6rd() is also reached from
transmit:
netdev_start_xmit()
sit_tunnel_xmit()
ipip6_tunnel_xmit()
try_6rd()
check_6rd()
rcu_dereference(tunnel->ip6rd)
and the xmit callers do not hold rcu_read_lock(). __dev_queue_xmit()
takes rcu_read_lock_bh(), which acquires rcu_bh_lock_map, and the
AF_PACKET qdisc bypass path __dev_direct_xmit() only does
local_bh_disable():
net/core/dev.c:__dev_direct_xmit() {
...
local_bh_disable();
dev_xmit_recursion_inc();
HARD_TX_LOCK(dev, txq, smp_processor_id());
...
}
rcu_dereference() expands to rcu_dereference_check(p, 0), whose condition
is rcu_read_lock_held():
kernel/rcu/update.c:rcu_read_lock_held() {
...
return lock_is_held(&rcu_lock_map);
}
so on a CONFIG_PROVE_RCU kernel, would sending a packet on a sit device
from these contexts produce a "suspicious rcu_dereference_check() usage"
splat? Object lifetime looks fine, since BH-disabled regions are RCU
read-side sections, but the annotation seems off.
Would rcu_dereference_bh() (or rcu_dereference_check() with
rcu_read_lock_bh_held()) be more accurate, or alternatively an explicit
rcu_read_lock() in ipip6_tunnel_xmit(), like vxlan_xmit() does before its
RCU lookups? The same question applies to the new rcu_dereference() in
only_dnatted().
[ ... ]
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 1 · Low: 1
- [Medium] Intermediate-state use-after-free in net/ipv6/sit.c:
ipip6_tunnel_update() publishes the freshly allocated parameter block…
- [Low] Missing publication barrier / unannotated lockless pointer: in
this commit `sit_parms` is a plain (non-__rcu) pointer in struct…
--
sit: dynamically allocate struct ip_tunnel_parm_kern
This moves the SIT configuration parameters out of the embedded
ip_tunnel.parms into a separately allocated struct ip_tunnel_parm_kern
reachable through the new ip_tunnel.sit_parms pointer, in preparation for
converting them to RCU. All readers and writers in net/ipv6/sit.c are
switched over to the new pointer.
A couple of questions about the intermediate state this commit leaves the
tree in.
@@ -190,6 +190,7 @@ struct ip_tunnel {#endifstructip_tunnel_prl_entry__rcu*prl;/* potential router list */unsignedintprl_count;/* # of entries in PRL */+structip_tunnel_parm_kern*sit_parms;
^^^^
[Severity: Low]
Should this member already carry the __rcu annotation in this commit? It
is read from softirq packet paths (ipip6_tunnel_lookup(), ipip6_rcv(),
sit_tunnel_rcv(), ipip6_tunnel_xmit(), sit_tunnel_xmit__()) without RTNL,
while ipip6_tunnel_update() replaces it with a plain store. As a plain
pointer, sparse cannot flag the unprotected dereferences, the publish has
no release semantics, and the loads are plain data races for KCSAN.
The following patch in the series, "sit: convert configuration to RCU
protection", changes this to struct ip_tunnel_parm_kern __rcu *sit_parms
and converts publication to rcu_assign_pointer(), so the window is limited
to this single commit. Would it be worth folding the annotation in here?
quoted hunk
unsigned int ip_tnl_net_id;
struct gro_cells gro_cells;
__u32 fwmark;
@@ -108,24 +108,24 @@ static struct ip_tunnel *ipip6_tunnel_lookup(struct net *net,intifindex=dev?dev->ifindex:0;for_each_ip_tunnel_rcu(t,sitn->tunnels_r_l[h0^h1]){-if(local==t->parms.iph.saddr&&-remote==t->parms.iph.daddr&&-(!dev||!t->parms.link||ifindex==t->parms.link||-sifindex==t->parms.link)&&+if(local==t->sit_parms->iph.saddr&&+remote==t->sit_parms->iph.daddr&&+(!dev||!t->sit_parms->link||ifindex==t->sit_parms->link||+sifindex==t->sit_parms->link)&&
^^^^
[Severity: Low]
Can this mix fields from two different parameter blocks? t->sit_parms is
reloaded up to four times inside one match decision here, and
ipip6_tunnel_update() swaps the pointer concurrently, so saddr/daddr could
come from the old block and link from the new one.
The next patch caches a single rcu_dereference(t->sit_parms) in a local
parms variable per candidate, which removes this; could that be done here
instead?
^^^^
[Severity: Medium]
Can this read freed memory? ipip6_tunnel_xmit() reaches the tunnel via
netdev_priv(dev), never through the sitn->tunnels[] buckets, and it keeps
the interior pointer tiph across the route lookup, the PMTU handling, the
headroom reallocation and iptunnel_xmit().
Meanwhile ipip6_tunnel_update() frees the block the pointer points into:
net/ipv6/sit.c:ipip6_tunnel_update() {
...
ipip6_tunnel_unlink(sitn, t);
synchronize_net();
t->sit_parms = new_p;
...
kfree(old_p);
return 0;
}
The synchronize_net() runs before the new pointer is published and only
drains readers that came in through the hash buckets, so it does not cover
this path. dev->lltx is set for sit, so nothing else serializes a
SIOCCHGTUNNEL or RTM_NEWLINK change against traffic on the same device.
The values read out of the freed block (tiph->saddr, tos, ttl, frag_off)
end up in the outer IPv4 header.
The immediately following patch, "sit: convert configuration to RCU
protection", wraps the transmit path in rcu_read_lock() and uses
kfree_rcu(old_p, rcu), so this hazard exists only between the two commits
(a concern for bisection and partial backports).
__be16 df = tiph->frag_off;
struct rtable *rt; /* Route to the other host */
struct net_device *tdev; /* Device to other host */
^^^^
[Severity: Medium]
Same question here: this hands an interior pointer of the freeable
parameter block to ip_tunnel_xmit() with no reference and no RCU section
held, so a concurrent ipip6_tunnel_update() can kfree() it underneath.
if (iptunnel_handle_offloads(skb, SKB_GSO_IPXIP4))
goto tx_error;
^^^^
[Severity: Medium]
Is the ordering here intentional? The grace period happens before the new
pointer is published, so nothing separates the store of new_p from the
kfree(old_p) below for readers that already loaded the old pointer.
^^^^
[Severity: Medium]
Would kfree_rcu() be needed here, given that ipip6_tunnel_xmit(),
sit_tunnel_xmit__() and ipip6_get_iflink() all reach sit_parms through
netdev_priv(dev) rather than through the hash buckets?
The next patch in the series adds a struct rcu_head to
ip_tunnel_parm_kern and turns this into kfree_rcu(old_p, rcu).
^^^^
[Severity: Medium]
Here the READ_ONCE() now covers only the link field, not the pointer load
itself. Since this accessor can run without RTNL, can it dereference the
block that ipip6_tunnel_update() is about to kfree()? The following patch
adds an rcu_read_lock() section and an rcu_dereference() here.
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 1 · Low: 0
- [Medium] SIT tunnel parameters are now read from two objects that live
in different synchronization domains, and ipip6_tunnel_update()…
--
sit: convert configuration to RCU protection
This converts tunnel->sit_parms into an RCU-protected pointer, so that
ipip6_tunnel_update() publishes a freshly allocated parameter block with
rcu_assign_pointer() and frees the old one with kfree_rcu(). The unlink /
re-link plus synchronize_net() are now skipped when neither saddr nor daddr
changed.
[Severity: Medium]
After this change, are the SIT transmit parameters read from two objects
that are no longer updated together?
ipip6_tunnel_update() now performs two separate stores here:
t->parms.iph = new_p->iph; /* plain struct copy */
WRITE_ONCE(t->parms.link, new_p->link);
rcu_assign_pointer(t->sit_parms, new_p); /* published */
On the transmit side, sit_tunnel_xmit__() takes the header out of the
RCU-published block:
parms = rcu_dereference(tunnel->sit_parms);
tiph = &parms->iph;
ip_tunnel_xmit(skb, dev, tiph, ipproto);
but ip_tunnel_xmit() in net/ipv4/ip_tunnel.c mixes that with the shadow
copy that is not RCU protected:
connected = (tunnel->parms.iph.daddr != 0);
...
dst = tnl_params->daddr;
...
rt = connected ? dst_cache_get_ip4(&tunnel->dst_cache,
&fl4.saddr) : NULL;
...
else if (!md && connected)
dst_cache_set_ip4(&tunnel->dst_cache, &rt->dst,
fl4.saddr);
Since sit_tunnel_xmit() runs locklessly, can it observe the two objects
from different generations, e.g. sit_parms with daddr == 0 (so dst is
derived per packet from the skb/neighbour) while tunnel->parms.iph.daddr
already holds the new address, making connected true? In that case
ip_tunnel_xmit() would consult and populate tunnel->dst_cache for a
destination-specific route on a tunnel that the header says is NBMA, so a
burst of packets could be routed via the cached entry. The reverse pairing
(old shadow daddr, new sit_parms daddr) looks possible too where the plain
tunnel->parms.iph.daddr load can be reordered against the rcu_dereference().
Independently of the ordering question, is the plain struct assignment
t->parms.iph = new_p->iph a data race? It writes roughly twenty bytes
without any annotation while the xmit path is reading ttl, tos, frag_off
and daddr from tunnel->parms, which KCSAN should flag and which can hand
the transmit path a mixture of old and new field values.
The synchronize_net() in the move case only runs after publication, so it
bounds but does not close the window for a reader that entered right at the
grace period boundary.
Would it make sense for ip_tunnel_xmit() to derive connected and link from
the same parameter block that tiph came from, so that only one object is
consulted per packet?
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski [off-list ref]:
On Mon, 7 Sep 2026 07:58:37 +0000 you wrote:
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.
[...]