Re: [PATCH net-next 2/9] vxlan: vnifilter: free vxlan_vni_group via RCU in vxlan_vnigroup_uninit()

On Mon, Sep 7, 2026 at 8:11 AM [off-list ref] wrote:

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

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

- [Medium] Changelog premise is not supported by the code: commit "vxlan:
  vnifilter: free vxlan_vni_group via RCU in vxlan_vnigroup_uninit()"…

--

vxlan: vnifilter: free vxlan_vni_group via RCU in vxlan_vnigroup_uninit()

This clears vxlan->vnigrp with rcu_assign_pointer() in
vxlan_vnigroup_uninit() and defers the freeing of struct vxlan_vni_group
to kfree_rcu(), adding an rcu_head to the structure.  It is described as
fixing a use-after-free between device deletion and an RTM_GETTUNNEL dump.

quoted

    vxlan->vnigrp is an RCU-protected pointer accessed locklessly under
    rcu_read_lock() in vxlan_vnifilter_dump_dev().

    Currently, vxlan_vnigroup_uninit() frees struct vxlan_vni_group
    synchronously via kfree(vg). If a VXLAN device is deleted concurrently
    with an RTM_GETTUNNEL dump, vxlan_vnifilter_dump_dev() can suffer a
    use-after-free when reading vg->num_vnis or walking vg->vni_list.

    Fixes: f9c4bb0b245c ("vxlan: vni filtering support on collect metadata device")

[Severity: Medium]
Is the described race reachable in this tree?  The RTM_GETTUNNEL dump is
registered without RTNL_FLAG_DUMP_UNLOCKED:

drivers/net/vxlan/vxlan_vnifilter.c:
static const struct rtnl_msg_handler vxlan_vnifilter_rtnl_msg_handlers[] = {
        {THIS_MODULE, PF_BRIDGE, RTM_GETTUNNEL, NULL, vxlan_vnifilter_dump, 0},

With flags == 0, rtnl_dumpit() wraps the whole callback in RTNL:

net/core/rtnetlink.c:rtnl_dumpit() {
        const bool needs_lock = !(cb->flags & RTNL_FLAG_DUMP_UNLOCKED);
        ...
        if (needs_lock)
                rtnl_lock();
        err = dumpit(skb, cb);
        if (needs_lock)
                rtnl_unlock();
}

so vxlan_vnifilter_dump() and vxlan_vnifilter_dump_dev() run with RTNL held,
not "locklessly" as the changelog states.

On the freeing side, vxlan_vnigroup_uninit() is reached only from the
vxlan_init() error path and from vxlan_uninit() (ndo_uninit), both of which
run under RTNL via register_netdevice() and
unregister_netdevice_many_notify().  Doesn't that make the dump and the free
mutually exclusive today?

The dump also does not cache a device or vnigrp pointer across paginated
recvmsg iterations: it stores only an ifindex in cb->args[0] and re-resolves
through dev_get_by_index_rcu()/for_each_netdev_rcu(), so a stale vnigrp does
not appear to be observable between iterations either.

The code change itself looks harmless, since the remaining readers already
tolerate a NULL vnigrp:

drivers/net/vxlan/vxlan_vnifilter.c:vxlan_vnifilter_dump_dev() {
        /* RCU needed because of the vni locking rules (rcu || rtnl) */
        vg = rcu_dereference(vxlan->vnigrp);
        if (!vg || !vg->num_vnis)
                return 0;

and vxlan_vnifilter_lookup() in drivers/net/vxlan/vxlan_private.h does the
same NULL check.

Would it be more accurate to describe this as preparation for an
RTNL-unlocked RTM_GETTUNNEL dump, and to drop the Fixes: tag (or justify it)
so this is not backported to stable as a use-after-free fix?

The next patch in the series, "vxlan: vnifilter: use
list_for_each_entry_rcu() in vxlan_vnifilter_dump_dev()", repeats the same
premise ("runs under rcu_read_lock() without RTNL"), and the RTM_GETTUNNEL
handler is still registered with flags == 0 at the end of the series, so the
same question applies there.

Not that Kuniyuki pointed out that the patch was not needed, it was
dropped from V2.