Commit 309b905deee5 ("ipv6: convert CONFIG_IPV6 to built-in
only and clean up Kconfigs") started to call inet6_init() as
device_initcall().
It explains the reason as a race condition on IPv6 addrconf,
but this is not accurate.
The change initially used fs_initcall() but switched to
device_initcall() due to the null-ptr-deref stack trace in
the link below.
It happened because blackhole_netdev_init() uses device_initcall()
and blackhole_netdev was NULL when addrconf_init() called
ipv6_add_dev(blackhole_netdev).
Due to the order in Kbuild,
$ cat -n Kbuild | grep -E "(drivers|net)"
111 obj-y += drivers/
114 obj-$(CONFIG_NET) += net/
using device_initcall() for IPv6 allows built-in drivers to
be initialised earlier, which requires unnecessary NULL checks.
For the same ordering reason, IPv4 uses late_initcall() to
initialise blackhole_netdev.
Given loopback_net_ops is registered in net_dev_init() at
subsys_initcall(), blackhole_netdev_init() can be called
after that.
In addition, some places assume that IPv4 must be initialised
before IPv6. For example, mptcp_proto_v6_init() copies
mptcp_prot to mptcp_v6_prot, which would otherwise have NULL
mptcp_v6_prot.h.hashinfo.
Let's explicitly order blackhole_netdev_init() -> inet_init()
-> inet6_init() before device_initcall() with 3 different
initcall levels:
subsys_initcall : 4 : net_dev_init()
subsys_initcall_sync : 4s : blackhole_netdev_init()
fs_initcall : 5 : inet_init()
fs_initcall_sync : 5s : inet6_init()
device_initcall : 6 : built-in drivers
Note that both IPv4 and IPv6 can still use fs_initcall() thanks
to the order in net/Makefile, but explicit ordering would be
less error-prone.
$ cat -n net/Makefile | grep ipv
17 obj-$(CONFIG_INET) += ipv4/
22 obj-y += ipv6/
Link: https://lore.kernel.org/netdev/20260309074758.0ea95a18@kernel.org/
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
Cc: Fernando Fernandez Mancera <redacted>
---
drivers/net/loopback.c | 2 +-
net/ipv6/af_inet6.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
Commit 22600596b675 ("ipv4: give an IPv4 dev to blackhole_netdev")
started to allocate in_device for blackhole_netdev.
It used late_initcall() because blackhole_netdev was allocated
at device_initcall().
Now, it is allocated before inet_init(), and we no longer need
to use late_initcall().
Let's inline inet_blackhole_dev_init() to devinet_init().
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
net/ipv4/devinet.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
The next patch needs to use dev_isalive() in xfrm.
We should try to avoid using the dev_ prefix on APIs
we export via include/linux/.
Let's move it to include/linux/netdevice.h and rename
to netif_is_alive().
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
include/linux/netdevice.h | 6 ++++++
net/core/dev.h | 6 ------
net/core/net-sysfs.c | 16 ++++++++--------
net/core/netdev_work.c | 4 ++--
net/core/rtnetlink.c | 2 +-
5 files changed, 17 insertions(+), 17 deletions(-)
@@ -154,7 +154,7 @@ static void netdev_work_proc(struct work_struct *work)/* We took another ref above */netdev_put(dev,&dev->work_tracker);-if(!dev_isalive(dev))+if(!netif_is_alive(dev))core=events=0;}spin_unlock_bh(&netdev_work_lock);
struct dst_entry passed to xfrm_bundle_create() and
xfrm_create_dummy_bundle() could outlive synchronize_net() in
unregister_netdevice_many_notify().
For example, ip_route_output_flow() calls __ip_route_output_key()
to fetch a dst under RCU, but calls xfrm_lookup_route() outside of
that RCU critical section.
Thus, xfrm_fill_dst() could add a new uncached route after the
first NETDEV_UNREGISTER notification and rely on the rebroadcast
in netdev_wait_allrefs_any().
The following patches will move the uncached route flush for dying
netdev from the NETDEV_UNREGISTER handler to netdev_run_todo(),
where the rebroadcast is also skipped due to dev->dismantle,
making such late additions problematic.
Let's check netif_is_alive() after fetching dst_dev_rcu(dst)
under RCU.
This ensures that xfrm either finishes adding the uncached
route before synchronize_net() or fails with -ENODEV.
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
---
net/xfrm/xfrm_policy.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
IPv4 uncached routes are linked to the global per-cpu lists,
rt_uncached_list.
When unregistering a netdev, rt_flush_dev() iterates over the
potentially long lists to find uncached routes tied to the device
and swap it with blackhole_netdev.
Since it is called for every device in dying netns under RTNL,
it adds O(N_dev x (N_cpu + N_route)) costs to any batched device
unregistration.
Let's call it once per batched device unregistration without RTNL.
Note that rt_flush_dev() must be called after setting dev->reg_state
to NETREG_UNREGISTERED. Otherwise, because rt_flush_dev(NULL) runs
without RTNL, it could race with unregister_netdevice_many_notify()
and prematurely purge routes for NETREG_UNREGISTERING dev, for
which flush_all_backlogs() has not been called yet.
Reported-by: Chris J Arges <redacted>
Closes: https://lore.kernel.org/netdev/20260917-hash-bucket-route-lists-v3-0-30493a37b6eb@cloudflare.com/
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
include/net/route.h | 6 ++++++
net/core/dev.c | 4 ++++
net/ipv4/route.c | 11 +++++++++--
3 files changed, 19 insertions(+), 2 deletions(-)
IPv6 uncached routes are linked to the global per-cpu lists,
rt6_uncached_list.
When unregistering a netdev, rt6_uncached_list_flush_dev() iterates
over the potentially long lists to find uncached routes tied to
the device and swap it with blackhole_netdev.
Since it is called for every device in dying netns under RTNL,
it adds O(N_dev x (N_cpu + N_route)) costs to any batched device
unregistration.
Let's call it once per batched device unregistration without RTNL.
Because it now runs without RTNL, rt->dst.dev and rt->rt6i_idev->dev
may transition to NETREG_UNREGISTERED at different times. Thus, we
must check that both devices are blackhole_netdev before unlinking
the route; otherwise, the remaining device reference would not be
flushed when the other device is unregistered.
In addition, since netdev_run_todo() runs even when IPv6 is disabled,
rt6_uncached_list_flush_dev() must check ipv6_mod_enabled() before
touching rt6_uncached_list.
Note also that netdev_run_todo() cannot be called before
rt6_uncached_list is initialised because:
1. unregister_netdevice_many_notify() panics when
dev_boot_phase == 1 (net_dev_init() is subsys_initcall()).
2. the only device registered before fs_initcall_sync() is
loopback and loopback_net_init() panics in case of failure.
Reported-by: Chris J Arges <redacted>
Closes: https://lore.kernel.org/netdev/20260917-hash-bucket-route-lists-v3-0-30493a37b6eb@cloudflare.com/
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
v3:
* Unlink only when both devices are blackhole_netdev
* Clarify netdev_run_todo() does not run before
rt6_uncached_list is initialised
* Fix CONFIG_IPV6=n build
---
include/net/ip6_route.h | 7 +++++++
net/core/dev.c | 5 ++++-
net/ipv6/route.c | 24 ++++++++++++++++--------
3 files changed, 27 insertions(+), 9 deletions(-)
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] In net/ipv4/devinet.c, devinet_init() now panics when
inetdev_init(blackhole_netdev) returns an ERR_PTR.
Pre-existing issues:
- [Low] If alloc_netdev() fails in blackhole_netdev_init()
(drivers/net/loopback.c, subsys_initcall_sync), the function returns…
--
ipv4: Inline inet_blackhole_dev_init() to devinet_init().
This commit removes the late_initcall inet_blackhole_dev_init(). The
in_device for blackhole_netdev is now allocated directly from
devinet_init(), because blackhole_netdev is allocated before inet_init().
@@ -2974,4 +2962,9 @@ void __init devinet_init(void) panic("Unable to register inet_af_ops\n"); rtnl_register_many(devinet_rtnl_msg_handlers);++ rtnl_lock();+ if (IS_ERR(inetdev_init(blackhole_netdev)))+ panic("IP: failed to allocate in_device for blackhole_netdev\n");
[Severity: Low]
This isn't a bug, but should the commit message say that this failure is
now fatal?
The removed inet_blackhole_dev_init() returned PTR_ERR_OR_ZERO(in_dev).
On an inetdev_init() failure, the initcall core only logged the error,
and boot continued with blackhole_netdev->ip_ptr still NULL. After this
change, the same failure panics.
Panicking here matches how devinet_init() already handles a
rtnl_af_register() failure a few lines above. The only ways this call can
fail are the kzalloc_obj() and neigh_parms_alloc() allocations in
inetdev_init(). Even so, the commit message only says:
Let's inline inet_blackhole_dev_init() to devinet_init().
Could a sentence be added noting that an allocation failure here now
panics instead of being ignored?
[Severity: Low]
This is a pre-existing issue and was not introduced by this patch, but
what happens if blackhole_netdev is NULL at this point?
drivers/net/loopback.c:blackhole_netdev_init() {
blackhole_netdev = alloc_netdev(0, "blackhole_dev", NET_NAME_UNKNOWN,
blackhole_netdev_setup);
if (!blackhole_netdev)
return -ENOMEM;
...
}
The initcall core ignores that -ENOMEM, so blackhole_netdev stays NULL.
inetdev_init() then dereferences it through dev_net(dev) on its first
line. That would oops before the new panic message is printed.
The removed late_initcall had no NULL check either, and addrconf_init()
also calls ipv6_add_dev(blackhole_netdev) without one. This patch just
moves an existing early-boot crash from late_initcall time to fs_initcall
time.
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 0 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] Missing RCU grace period between the device-pointer swap in
rt_flush_dev(NULL) and freeing the net_device.
--
ipv4: Batch rt_flush_dev() in netdev_run_todo().
This patch takes the IPv4 uncached route flush out of the per-device
unregister path. netdev_run_todo() now calls rt_flush_dev(NULL) once per
batch, without RTNL. With a NULL dev, rt_flush_dev() moves every uncached
route whose device has reached NETREG_UNREGISTERED over to
blackhole_netdev.
Closes: https://lore.kernel.org/netdev/20260917-hash-bucket-route-lists-v3-0-30493a37b6eb@cloudflare.com/
[Severity: High]
Is there still an RCU grace period between the rt->dst.dev swap and
freeing the old net_device?
Before this patch, fib_netdev_event()->rt_flush_dev(dev) did the swap
during NETDEV_UNREGISTER. That was before the second synchronize_net() in
unregister_netdevice_many_notify(), and before the rcu_barrier() at the
top of netdev_run_todo(). So any RCU reader that had already loaded the
old rt->dst.dev was done before the device could be freed.
With this patch, the swap and the reference drop both happen after those
calls:
net/ipv4/route.c:rt_flush_dev() {
...
rcu_assign_pointer(rt->dst.dev_rcu, blackhole_netdev);
netdev_ref_replace(rt_dev, blackhole_netdev,
&rt->dst.dev_tracker, GFP_ATOMIC);
...
}
If that drops the last extra reference, netdev_wait_allrefs_any() returns
from its first check, before any rcu_barrier() or msleep():
list_for_each_entry(dev, list, todo_list)
if (netdev_refcnt_read(dev) == 1)
return dev;
netdev_run_todo() then goes straight on to priv_destructor, free_netdev()
and kobject_put(). Some drivers instead call free_netdev() right after
unregister_netdev() returns. Either way, netdev_release() frees the
device without waiting:
/* no need to wait for rcu grace period:
* device is dead and about to be freed.
*/
kfree(rcu_access_pointer(dev->ifalias));
kvfree(dev);
The only synchronize_net() left on this path is in netdev_napi_exit(). It
runs only when dev->napi_list is not empty, so most virtual devices skip
it.
Meanwhile, a sender whose socket has an uncached IPv4 route to this device
in sk_dst_cache can be in sk_dst_check()->ipv4_dst_check()->rt_is_expired():
rcu_read_lock();
res = rth->rt_genid != rt_genid_ipv4(dev_net_rcu(rth->dst.dev));
rcu_read_unlock();
rt_flush_dev() does not mark the dst obsolete, so this check still runs.
If the reader loads the old rth->dst.dev and is then preempted (under
PREEMPT_RCU) or delayed while unregistration finishes, can it read
dev->nd_net from freed memory? Other readers that use dst_dev_rcu() rely
on RCU to keep the device alive in the same way.
Deleting the device needs only CAP_NET_ADMIN in a user namespace's netns
(for example "ip link del"), so an unprivileged user with userns can
reach this path.
The later patch "ipv6: Batch rt6_uncached_list_flush_dev() in
netdev_run_todo()" seems to follow the same pattern for IPv6. At the end
of the series, netdev_run_todo() still calls rt_flush_dev(NULL) and
rt6_uncached_list_flush_dev(NULL) and goes directly into the free loop.
Would a synchronize_net() after the flush calls in netdev_run_todo() be
needed here, or is there something else that guarantees a grace period
before the device is freed?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001204752.2572265-1-kuniyu%40google.com
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 1 · High: 0 · Medium: 1 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Critical] Use-after-free of the net_device because no RCU grace period
separates the uncached-route swap from the device free.
- [Medium] Uninitialised per-CPU list access in
rt6_uncached_list_flush_dev() when IPv6 init failed at boot.
--
ipv6: Batch rt6_uncached_list_flush_dev() in netdev_run_todo().
This commit stops flushing IPv6 uncached routes from the per-device
NETDEV_UNREGISTER path. It now does the flush once per batched
unregistration from netdev_run_todo(), without RTNL. Routes are matched by
NETREG_UNREGISTERED and are unlinked only after both dst.dev and
rt6i_idev->dev point at blackhole_netdev. An ipv6_mod_enabled() check is
added because netdev_run_todo() also runs when IPv6 is disabled.
Closes: https://lore.kernel.org/netdev/20260917-hash-bucket-route-lists-v3-0-30493a37b6eb@cloudflare.com/
@@ -11872,8 +11873,10 @@ void netdev_run_todo(void) linkwatch_sync_dev(dev); }- if (!list_empty(&list))+ if (!list_empty(&list)) { rt_flush_dev(NULL);+ rt6_uncached_list_flush_dev(NULL);+ } cnt = 0; while (!list_empty(&list)) {
[Severity: Critical]
Does this flush need an RCU grace period before the device is freed?
Before this patch, rt6_disable_ip() called rt6_uncached_list_flush_dev(dev)
during NETDEV_UNREGISTER processing. The synchronize_net() in
unregister_netdevice_many_notify() ran after that call. So any RCU reader
that had loaded the old dst.dev was done before the device could be freed.
Now the swap happens after the only rcu_barrier() in netdev_run_todo(), and
the old device reference is dropped right away:
rt6_uncached_list_flush_dev(NULL)
rcu_assign_pointer(rt->dst.dev_rcu, blackhole_netdev);
netdev_ref_replace(rt_dev, blackhole_netdev, ...)
__dev_put(rt_dev)
The in6_dev_put(rt_idev) call can also reach in6_dev_finish_destroy(),
which drops a device reference synchronously.
netdev_wait_allrefs_any() then checks the refcount before any
rcu_barrier() or msleep():
list_for_each_entry(dev, list, todo_list)
if (netdev_refcnt_read(dev) == 1)
return dev;
After that, netdev_run_todo() runs netdev_do_free_pcpu_stats(),
priv_destructor and free_netdev(). The final put reaches netdev_release(),
which calls kvfree(dev) under the comment "no need to wait for rcu grace
period".
Readers such as ip6_finish_output2() use the device under RCU without
taking a reference:
struct net_device *dev = dst_dev_rcu(dst);
struct inet6_dev *idev = ip6_dst_idev(dst);
unsigned int hh_len = LL_RESERVED_SPACE(dev);
Take a sender holding an uncached rt6 on device X. It could come from an
IPV6_HDRINCL raw socket, an xfrm6 bundle, or icmp6_dst_alloc():
CPU1 CPU0
ip6_finish_output2()
dev = dst_dev_rcu(dst) /* X */
netdev_run_todo()
rt6_uncached_list_flush_dev(NULL)
swap to blackhole, put X
netdev_wait_allrefs_any()
refcnt == 1, return X
...
kvfree(X)
LL_RESERVED_SPACE(dev),
dev_queue_xmit()
Can CPU1 then dereference the freed net_device and its tx queues? This
could happen while CPU1 is preempted inside its RCU section under
PREEMPT_RCU.
The previous patch in this series, "ipv4: Batch rt_flush_dev() in
netdev_run_todo().", seems to do the same for IPv4 uncached routes. Would
a synchronize_net() after the rt_flush_dev(NULL) and
rt6_uncached_list_flush_dev(NULL) calls, before netdev_wait_allrefs_any(),
close this window?
[Severity: Medium]
Is ipv6_mod_enabled() enough to show that rt6_uncached_list was
initialised?
ipv6_mod_enabled() only reflects disable_ipv6_mod. The per-cpu list heads
and locks are set up at the very end of ip6_route_init(), after every step
that can fail:
net/ipv6/route.c:ip6_route_init() {
...
ret = register_netdevice_notifier(&ip6_route_dev_notifier);
if (ret)
goto out_register_late_subsys;
...
for_each_possible_cpu(cpu) {
struct uncached_list *ul = per_cpu_ptr(&rt6_uncached_list, cpu);
INIT_LIST_HEAD(&ul->head);
spin_lock_init(&ul->lock);
}
...
}
ip6_route_init() can fail, or inet6_init() can fail before it calls
ip6_route_init(). In either case the inet6_init() error paths leave
disable_ipv6_mod unset. Because CONFIG_IPV6 is bool, boot carries on.
The next device unregistration would then run:
rtnl_unlock()
netdev_run_todo()
rt6_uncached_list_flush_dev(NULL)
list_empty(&ul->head) /* head->next == NULL, seen as non-empty */
spin_lock_bh(&ul->lock) /* lock never initialised */
list_for_each_entry_safe() /* loads through NULL head->next */
Could this oops with BH disabled and the per-cpu lock held?
The commit message says:
Note also that netdev_run_todo() cannot be called before
rt6_uncached_list is initialised because:
That covers ordering, but not the case where initialisation never
finishes. Before this patch, the function was only reachable from
rt6_disable_ip() through the ip6_route/addrconf notifiers. Those are
registered only after ip6_route_init() has succeeded.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001204752.2572265-1-kuniyu%40google.com
On Sun, Oct 4, 2026 at 4:02 PM [off-list ref] wrote:
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.
Critical: 1 · High: 0 · Medium: 1 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Critical] Use-after-free of the net_device because no RCU grace period
separates the uncached-route swap from the device free.
- [Medium] Uninitialised per-CPU list access in
rt6_uncached_list_flush_dev() when IPv6 init failed at boot.
--
ipv6: Batch rt6_uncached_list_flush_dev() in netdev_run_todo().
This commit stops flushing IPv6 uncached routes from the per-device
NETDEV_UNREGISTER path. It now does the flush once per batched
unregistration from netdev_run_todo(), without RTNL. Routes are matched by
NETREG_UNREGISTERED and are unlinked only after both dst.dev and
rt6i_idev->dev point at blackhole_netdev. An ipv6_mod_enabled() check is
added because netdev_run_todo() also runs when IPv6 is disabled.
Closes: https://lore.kernel.org/netdev/20260917-hash-bucket-route-lists-v3-0-30493a37b6eb@cloudflare.com/
@@ -11872,8 +11873,10 @@ void netdev_run_todo(void) linkwatch_sync_dev(dev); }- if (!list_empty(&list))+ if (!list_empty(&list)) { rt_flush_dev(NULL);+ rt6_uncached_list_flush_dev(NULL);+ } cnt = 0; while (!list_empty(&list)) {
[Severity: Critical]
Does this flush need an RCU grace period before the device is freed?
No.
ip6_finish_output2() mentioned below should not happen at this
point, and also nexthop does not wait RCU grace period
after dst_dev_put():
After calling fib6_info_release() at NETDEV_UNREGISTER,
synchronize_net() in unregister_netdevice_many_notify()
makes fib6_info_destroy_rcu() schedulable, and dst_dev_put()
is called there, and rcu_barrier() in netdev_run_todo() waits the
callback to finish, but there is not more synchornize_rcu() for
dst_dev_put() before free_netdev().
fib6_info_destroy_rcu() -> nexthop_free_rcu() -> fib6_nh_release()
-> fib6_nh_release_dsts() -> dst_dev_put()
Before this patch, rt6_disable_ip() called rt6_uncached_list_flush_dev(dev)
during NETDEV_UNREGISTER processing. The synchronize_net() in
unregister_netdevice_many_notify() ran after that call. So any RCU reader
that had loaded the old dst.dev was done before the device could be freed.
Now the swap happens after the only rcu_barrier() in netdev_run_todo(), and
the old device reference is dropped right away:
rt6_uncached_list_flush_dev(NULL)
rcu_assign_pointer(rt->dst.dev_rcu, blackhole_netdev);
netdev_ref_replace(rt_dev, blackhole_netdev, ...)
__dev_put(rt_dev)
The in6_dev_put(rt_idev) call can also reach in6_dev_finish_destroy(),
which drops a device reference synchronously.
netdev_wait_allrefs_any() then checks the refcount before any
rcu_barrier() or msleep():
list_for_each_entry(dev, list, todo_list)
if (netdev_refcnt_read(dev) == 1)
return dev;
After that, netdev_run_todo() runs netdev_do_free_pcpu_stats(),
priv_destructor and free_netdev(). The final put reaches netdev_release(),
which calls kvfree(dev) under the comment "no need to wait for rcu grace
period".
Readers such as ip6_finish_output2() use the device under RCU without
taking a reference:
struct net_device *dev = dst_dev_rcu(dst);
struct inet6_dev *idev = ip6_dst_idev(dst);
unsigned int hh_len = LL_RESERVED_SPACE(dev);
Take a sender holding an uncached rt6 on device X. It could come from an
IPV6_HDRINCL raw socket, an xfrm6 bundle, or icmp6_dst_alloc():
CPU1 CPU0
ip6_finish_output2()
dev = dst_dev_rcu(dst) /* X */
netdev_run_todo()
rt6_uncached_list_flush_dev(NULL)
swap to blackhole, put X
netdev_wait_allrefs_any()
refcnt == 1, return X
...
kvfree(X)
LL_RESERVED_SPACE(dev),
dev_queue_xmit()
Can CPU1 then dereference the freed net_device and its tx queues? This
could happen while CPU1 is preempted inside its RCU section under
PREEMPT_RCU.
The previous patch in this series, "ipv4: Batch rt_flush_dev() in
netdev_run_todo().", seems to do the same for IPv4 uncached routes. Would
a synchronize_net() after the rt_flush_dev(NULL) and
rt6_uncached_list_flush_dev(NULL) calls, before netdev_wait_allrefs_any(),
close this window?
[Severity: Medium]
Is ipv6_mod_enabled() enough to show that rt6_uncached_list was
initialised?
Yes.
Now inet6_init() runs at fs_initcall_sync() during early boot, where
small GFP_KERNEL allocations do not fail.
The error handling is a leftover from when =m was possible.
Also, other ipv6_mod_enabled() users assume the same, if it
returns true, inet6_init() finished successfully.
ipv6_mod_enabled() only reflects disable_ipv6_mod. The per-cpu list heads
and locks are set up at the very end of ip6_route_init(), after every step
that can fail:
net/ipv6/route.c:ip6_route_init() {
...
ret = register_netdevice_notifier(&ip6_route_dev_notifier);
if (ret)
goto out_register_late_subsys;
...
for_each_possible_cpu(cpu) {
struct uncached_list *ul = per_cpu_ptr(&rt6_uncached_list, cpu);
INIT_LIST_HEAD(&ul->head);
spin_lock_init(&ul->lock);
}
...
}
ip6_route_init() can fail, or inet6_init() can fail before it calls
ip6_route_init(). In either case the inet6_init() error paths leave
disable_ipv6_mod unset. Because CONFIG_IPV6 is bool, boot carries on.
The next device unregistration would then run:
rtnl_unlock()
netdev_run_todo()
rt6_uncached_list_flush_dev(NULL)
list_empty(&ul->head) /* head->next == NULL, seen as non-empty */
spin_lock_bh(&ul->lock) /* lock never initialised */
list_for_each_entry_safe() /* loads through NULL head->next */
Could this oops with BH disabled and the per-cpu lock held?
The commit message says:
Note also that netdev_run_todo() cannot be called before
rt6_uncached_list is initialised because:
That covers ordering, but not the case where initialisation never
finishes. Before this patch, the function was only reachable from
rt6_disable_ip() through the ip6_route/addrconf notifiers. Those are
registered only after ip6_route_init() has succeeded.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001204752.2572265-1-kuniyu%40google.com
On Sun, Oct 4, 2026 at 4:02 PM [off-list ref] wrote:
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 0 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] Missing RCU grace period between the device-pointer swap in
rt_flush_dev(NULL) and freeing the net_device.
Same with IPv6 comment.
--
ipv4: Batch rt_flush_dev() in netdev_run_todo().
This patch takes the IPv4 uncached route flush out of the per-device
unregister path. netdev_run_todo() now calls rt_flush_dev(NULL) once per
batch, without RTNL. With a NULL dev, rt_flush_dev() moves every uncached
route whose device has reached NETREG_UNREGISTERED over to
blackhole_netdev.
Closes: https://lore.kernel.org/netdev/20260917-hash-bucket-route-lists-v3-0-30493a37b6eb@cloudflare.com/
[Severity: High]
Is there still an RCU grace period between the rt->dst.dev swap and
freeing the old net_device?
Before this patch, fib_netdev_event()->rt_flush_dev(dev) did the swap
during NETDEV_UNREGISTER. That was before the second synchronize_net() in
unregister_netdevice_many_notify(), and before the rcu_barrier() at the
top of netdev_run_todo(). So any RCU reader that had already loaded the
old rt->dst.dev was done before the device could be freed.
With this patch, the swap and the reference drop both happen after those
calls:
net/ipv4/route.c:rt_flush_dev() {
...
rcu_assign_pointer(rt->dst.dev_rcu, blackhole_netdev);
netdev_ref_replace(rt_dev, blackhole_netdev,
&rt->dst.dev_tracker, GFP_ATOMIC);
...
}
If that drops the last extra reference, netdev_wait_allrefs_any() returns
from its first check, before any rcu_barrier() or msleep():
list_for_each_entry(dev, list, todo_list)
if (netdev_refcnt_read(dev) == 1)
return dev;
netdev_run_todo() then goes straight on to priv_destructor, free_netdev()
and kobject_put(). Some drivers instead call free_netdev() right after
unregister_netdev() returns. Either way, netdev_release() frees the
device without waiting:
/* no need to wait for rcu grace period:
* device is dead and about to be freed.
*/
kfree(rcu_access_pointer(dev->ifalias));
kvfree(dev);
The only synchronize_net() left on this path is in netdev_napi_exit(). It
runs only when dev->napi_list is not empty, so most virtual devices skip
it.
Meanwhile, a sender whose socket has an uncached IPv4 route to this device
in sk_dst_cache can be in sk_dst_check()->ipv4_dst_check()->rt_is_expired():
rcu_read_lock();
res = rth->rt_genid != rt_genid_ipv4(dev_net_rcu(rth->dst.dev));
rcu_read_unlock();
rt_flush_dev() does not mark the dst obsolete, so this check still runs.
If the reader loads the old rth->dst.dev and is then preempted (under
PREEMPT_RCU) or delayed while unregistration finishes, can it read
dev->nd_net from freed memory? Other readers that use dst_dev_rcu() rely
on RCU to keep the device alive in the same way.
Deleting the device needs only CAP_NET_ADMIN in a user namespace's netns
(for example "ip link del"), so an unprivileged user with userns can
reach this path.
The later patch "ipv6: Batch rt6_uncached_list_flush_dev() in
netdev_run_todo()" seems to follow the same pattern for IPv6. At the end
of the series, netdev_run_todo() still calls rt_flush_dev(NULL) and
rt6_uncached_list_flush_dev(NULL) and goes directly into the free loop.
Would a synchronize_net() after the flush calls in netdev_run_todo() be
needed here, or is there something else that guarantees a grace period
before the device is freed?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261001204752.2572265-1-kuniyu%40google.com
From: Fernando Fernandez Mancera <hidden> Date: 2026-10-05 12:19:41
On 10/1/26 10:47 PM, Kuniyuki Iwashima wrote:
Commit 309b905deee5 ("ipv6: convert CONFIG_IPV6 to built-in
only and clean up Kconfigs") started to call inet6_init() as
device_initcall().
It explains the reason as a race condition on IPv6 addrconf,
but this is not accurate.
The change initially used fs_initcall() but switched to
device_initcall() due to the null-ptr-deref stack trace in
the link below.
It happened because blackhole_netdev_init() uses device_initcall()
and blackhole_netdev was NULL when addrconf_init() called
ipv6_add_dev(blackhole_netdev).
Due to the order in Kbuild,
$ cat -n Kbuild | grep -E "(drivers|net)"
111 obj-y += drivers/
114 obj-$(CONFIG_NET) += net/
using device_initcall() for IPv6 allows built-in drivers to
be initialised earlier, which requires unnecessary NULL checks.
For the same ordering reason, IPv4 uses late_initcall() to
initialise blackhole_netdev.
Given loopback_net_ops is registered in net_dev_init() at
subsys_initcall(), blackhole_netdev_init() can be called
after that.
In addition, some places assume that IPv4 must be initialised
before IPv6. For example, mptcp_proto_v6_init() copies
mptcp_prot to mptcp_v6_prot, which would otherwise have NULL
mptcp_v6_prot.h.hashinfo.
Let's explicitly order blackhole_netdev_init() -> inet_init()
-> inet6_init() before device_initcall() with 3 different
initcall levels:
subsys_initcall : 4 : net_dev_init()
subsys_initcall_sync : 4s : blackhole_netdev_init()
fs_initcall : 5 : inet_init()
fs_initcall_sync : 5s : inet6_init()
device_initcall : 6 : built-in drivers
Note that both IPv4 and IPv6 can still use fs_initcall() thanks
to the order in net/Makefile, but explicit ordering would be
less error-prone.
$ cat -n net/Makefile | grep ipv
17 obj-$(CONFIG_INET) += ipv4/
22 obj-y += ipv6/
Link: https://lore.kernel.org/netdev/20260309074758.0ea95a18@kernel.org/
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
Reviewed-by: Fernando Fernandez Mancera <redacted>
I have tested this in multiple environments with multiple Kconfigs and command line options. Thanks for looking into this Kuniyuki.
On Thu, Oct 01, 2026 at 08:47:12PM +0000, Kuniyuki Iwashima wrote:
Chris J Arges reported high RTNL contention during cleanup_net()
caused by rt_flush_dev() and rt6_uncached_list_flush_dev() iterating
over the global per-cpu uncached route lists for every netdev in
dying netns [0].
This series resolves the issue by batching the uncached route
cleanup after __rtnl_unlock() in netdev_run_todo()
[0]: https://lore.kernel.org/netdev/20260917-hash-bucket-route-lists-v3-0-30493a37b6eb@cloudflare.com/
Changelog:
v3:
* Add patch 1 ~ 4
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
For next time:
The cover letter should explain how the patches fit together. Otherwise
it's up to the reviewer to figure it out. The dependency between patches
1 and 6 is not immediately clear. My understanding:
Patch 1 is needed so that the IPv6 uncached lists are initialized before
they can be traversed for devices that are unregistered due to failed
initialization.
Patch 2 is a cleanup that is made possible by patch 1.
Patches 3-4 are preparations for patches 5-6 that are going to walk the
uncached lists once per batch of unregistered devices. If a dst entry
that points to an unregistered device is put on the list after the walk,
nothing will clean it up after patches 5-6.
Patches 5-6 avoid walking the uncached lists for every device being
unregistered. Instead, walk the lists after one or more devices were
unregistered and delete from the lists entries that point to such
devices.
On Tue, Oct 6, 2026 at 1:12 AM Ido Schimmel [off-list ref] wrote:
On Thu, Oct 01, 2026 at 08:47:12PM +0000, Kuniyuki Iwashima wrote:
quoted
Chris J Arges reported high RTNL contention during cleanup_net()
caused by rt_flush_dev() and rt6_uncached_list_flush_dev() iterating
over the global per-cpu uncached route lists for every netdev in
dying netns [0].
This series resolves the issue by batching the uncached route
cleanup after __rtnl_unlock() in netdev_run_todo()
[0]: https://lore.kernel.org/netdev/20260917-hash-bucket-route-lists-v3-0-30493a37b6eb@cloudflare.com/
Changelog:
v3:
* Add patch 1 ~ 4
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
For next time:
The cover letter should explain how the patches fit together. Otherwise
it's up to the reviewer to figure it out.
Sorry, I should have added it.
The dependency between patches
1 and 6 is not immediately clear. My understanding:
Yes, correct.
Thanks !
Patch 1 is needed so that the IPv6 uncached lists are initialized before
they can be traversed for devices that are unregistered due to failed
initialization.
Patch 2 is a cleanup that is made possible by patch 1.
Patches 3-4 are preparations for patches 5-6 that are going to walk the
uncached lists once per batch of unregistered devices. If a dst entry
that points to an unregistered device is put on the list after the walk,
nothing will clean it up after patches 5-6.
Patches 5-6 avoid walking the uncached lists for every device being
unregistered. Instead, walk the lists after one or more devices were
unregistered and delete from the lists entries that point to such
devices.
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski [off-list ref]:
On Thu, 1 Oct 2026 20:47:12 +0000 you wrote:
Chris J Arges reported high RTNL contention during cleanup_net()
caused by rt_flush_dev() and rt6_uncached_list_flush_dev() iterating
over the global per-cpu uncached route lists for every netdev in
dying netns [0].
This series resolves the issue by batching the uncached route
cleanup after __rtnl_unlock() in netdev_run_todo()
[...]