Thread (32 messages) flat view 32 messages, 4 authors, 14h ago

Re: [PATCH v6 net-next 09/15] ipv6: Replace &nd_tbl with nd_table(net).

From: Kuniyuki Iwashima <kuniyu@google.com>
Date: 2026-09-04 16:36:28

On Thu, Sep 3, 2026 at 8:10 PM [off-list ref] wrote:
Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 1 · High: 0 · Medium: 0 · Low: 0

- [Critical] Calling `nd_table(net)` before `inet6_init()` executes
  results in a NULL pointer dereference during driver probe.

--
quoted
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/rep/neigh.c b/drivers/net/ethernet/mellanox/mlx5/core/en/rep/neigh.c
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/rep/neigh.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/rep/neigh.c
@@ -17,26 +17,27 @@
 #include "fs_core.h"
 #include "diag/en_rep_tracepoint.h"

-static unsigned long mlx5e_rep_ipv6_interval(void)
+static unsigned long mlx5e_rep_ipv6_interval(struct net *net)
 {
      if (IS_ENABLED(CONFIG_IPV6) && ipv6_mod_enabled())
-             return NEIGH_VAR(&nd_tbl.parms, DELAY_PROBE_TIME);
+             return NEIGH_VAR(&nd_table(net)->parms, DELAY_PROBE_TIME);

      return ~0UL;
 }

 static void mlx5e_rep_neigh_update_init_interval(struct mlx5e_rep_priv *rpriv)
 {
-     unsigned long ipv6_interval = mlx5e_rep_ipv6_interval();
      struct net_device *netdev = rpriv->netdev;
      struct net *net = dev_net(netdev);
      unsigned long ipv4_interval;
+     unsigned long ipv6_interval;
      struct neigh_table *tbl;
      struct mlx5e_priv *priv;

      priv = netdev_priv(netdev);
      tbl = arp_table(net);
      ipv4_interval = NEIGH_VAR(&tbl->parms, DELAY_PROBE_TIME);
+     ipv6_interval = mlx5e_rep_ipv6_interval(net);

      rpriv->neigh_update.min_interval = min_t(unsigned long, ipv6_interval, ipv4_interval);
      mlx5_fc_update_sampling_interval(priv->mdev, rpriv->neigh_update.min_interval);
[Severity: Critical]
Unlike &nd_tbl, which is a statically allocated object that is
valid from the very first instruction of the kernel, nd_table(net)
reads net->neigh_tables[NEIGH_ND_TABLE], which is only populated
once ndisc_init()/inet6_init() has run.

Both of the new dereferences here are on driver probe paths that can
run before that point: mlx5e_rep_ipv6_interval() (via
mlx5e_rep_neigh_update_init_interval()) and
mlxsw_sp_router_neighs_update_interval_init() take
&nd_table(net)->parms and feed it to NEIGH_VAR(), so a NULL table
turns into a dereference of offsetof(struct neigh_table, parms)
rather than a benign zero. For built-in drivers, drivers/ is linked
before net/, and both the driver module_init() and inet6_init() are
device_initcall(), so the driver can win that race and oops during
boot.
I'm a bit surprised that IPv6 uses device_initcall() instead of fs_,
but looking at 309b905deee5, this is for IPv4 dependency.

If such a race occurs, there should be more issues, and the cleaner
way would be to add a new init level between fs and device for IPv6.

Note that ipv6_mod_enabled() does not help here: it only
reflects the ipv6.disable boot parameter and returns true well
before the nd table exists.

The same concern applies to any other conversion in this series that
can be reached from driver probe or from a netdev notifier that runs
before inet6_init().

How do you want to handle this? Either nd_table(net) needs to be
safe to call before ndisc_init() (e.g. by allocating the per-netns
slots earlier, in a pernet subsys registered ahead of the drivers),
or these callers need an explicit NULL check on the returned table
before touching ->parms. Please spell out the ordering guarantee you
are relying on in the changelog, since it is no longer obvious once
&nd_tbl becomes an indirection through struct net.

--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260902203722.926528-1-kuniyu%40google.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help