Re: [PATCH v2 net-next 8/8] vxlan: no longer rely on RTNL in vxlan_fill_info()

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

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

- [Low] vxlan_fill_info() (drivers/net/vxlan/vxlan_core.c:4719-4721)
  newly returns -ENODEV when rcu_dereference(vxlan->cfg) is NULL, but…

--

vxlan: no longer rely on RTNL in vxlan_fill_info()

This converts vxlan_fill_info() from rtnl_dereference() to
rcu_dereference() so link info dumping no longer needs RTNL, adding a
NULL check on vxlan->cfg that returns -ENODEV.

> diff --git a/drivers/net/vxlan/vxlan_core.c b/drivers/net/vxlan/vxlan_core.c
> index 2b7da90ae2da3..fd993a95501b0 100644
> --- a/drivers/net/vxlan/vxlan_core.c
> +++ b/drivers/net/vxlan/vxlan_core.c
> @@ -4712,8 +4712,14 @@ static int vxlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
>  	const struct vxlan_dev *vxlan = netdev_priv(dev);
>  	struct ifla_vxlan_port_range ports;
>  	const struct vxlan_config *cfg;
> +	int err = 0;
>  
> -	cfg = rtnl_dereference(vxlan->cfg);
> +	rcu_read_lock();
> +	cfg = rcu_dereference(vxlan->cfg);
> +	if (!cfg) {
> +		err = -ENODEV;
> +		goto out;
> +	}

[Severity: Low]

Can the rtnetlink core actually observe this -ENODEV?  The value returned
by rtnl_link_ops->fill_info() is propagated up through
rtnl_link_info_fill() -> rtnl_link_fill() into rtnl_fill_ifinfo(), which
only tests for failure:

net/core/rtnetlink.c:rtnl_fill_ifinfo() {
	...
	if (rtnl_link_fill(skb, dev) < 0)
		goto nla_put_failure;
	...
nla_put_failure:
	nlmsg_cancel(skb, nlh);
	return -EMSGSIZE;
}

so the -ENODEV is rewritten to -EMSGSIZE.  The callers treat -EMSGSIZE as
an internal kernel bug rather than as "device is going away":

net/core/rtnetlink.c:rtnl_getlink() {
	...
		if (err == -EMSGSIZE) {
			if (!need_rtnl) {
				/* Some altnames were added, retry with RTNL. */
				need_rtnl = true;
				goto retry;
			}
			/* -EMSGSIZE implies BUG in if_nlmsg_size */
			WARN_ON_ONCE(1);
		}
	...
}

net/core/rtnetlink.c:rtmsg_ifinfo_build_skb() {
	...
		/* -EMSGSIZE implies BUG in if_nlmsg_size() */
		WARN_ON(err == -EMSGSIZE);
	...
}

and in rtnl_dump_ifinfo() a negative return aborts the whole RTM_GETLINK
dump with "if (err < 0) break;" instead of skipping just this device.

Separately, is this branch reachable at all?  vxlan_config_apply()
publishes a non-NULL cfg before register_netdevice(), and the only writer
storing NULL is vxlan_free_dev():

drivers/net/vxlan/vxlan_core.c:vxlan_free_dev() {
	...
	RCU_INIT_POINTER(vxlan->cfg, NULL);
	kfree(cfg);
	...
}

which runs as dev->priv_destructor (after the device is unregistered and
no longer dumpable) or on the register_netdevice() failure path in
vxlan_dev_create(), where the device was never visible to rtnetlink.

If it is indeed unreachable, would dropping the check be clearer?  If it
is considered reachable, would returning 0 (emitting no vxlan attributes)
be preferable, so a device teardown race does not produce a WARN splat and
does not truncate "ip link show" output for unrelated devices?

Also, the sibling lockless reader of the same pointer has no NULL check:

drivers/net/vxlan/vxlan_vnifilter.c:vxlan_vnifilter_dump_dev() {
	...
	cfg = rcu_dereference(vxlan->cfg);
	if (!(cfg->flags & VXLAN_F_VNIFILTER)) {
	...
}

Should the two agree on whether a NULL cfg is possible?

Would it be worth mentioning this new -ENODEV path in the changelog, since
the commit message only describes the rtnl_dereference() to
rcu_dereference() conversion?

>  
>  	if (nla_put_u32(skb, IFLA_VXLAN_ID, be32_to_cpu(cfg->vni)))
>  		goto nla_put_failure;

[ ... ]

> @@ -4812,9 +4818,12 @@ static int vxlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
>  		    &cfg->reserved_bits))
>  		goto nla_put_failure;
>  
> -	return 0;
> +out:
> +	rcu_read_unlock();
> +	return err;
>  
>  nla_put_failure:
> +	rcu_read_unlock();
>  	return -EMSGSIZE;
>  }

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260906180111.1973188-1-edumazet%40google.com