Re: [PATCH net] net-sysfs: release the TX queue tracker before allowing reuse

[Eric Dumazet]

On Tue, Sep 29, 2026 at 10:13 AM Antoine Tenart <atenart@xxxxxxxxxx> wrote:
>
> On Sun, Sep 27, 2026 at 01:33:15AM +0800, Chengfeng Ye wrote:
> > An interrupted sysfs_rtnl_lock() can drop the last kobject reference to a
> > removed TX queue without holding RTNL. netdev_queue_release() clears the
> > kobject before releasing queue->dev_tracker, allowing the queue to be
> > re-added while the old release still needs the shared tracker slot:
> >
> >   CPU 0                               CPU 1
> >   netdev_queue_release()
> >     memset(kobj, 0, sizeof(*kobj))
> >                                       netdev_queue_add_kobject()
> >                                         state_initialized is clear
> >                                         netdev_hold() installs new tracker
> >     netdev_put() releases the new tracker
> >
> > With CONFIG_NET_DEV_REFCNT_TRACKER enabled, the old tracker is leaked and
> > the new lifetime's tracker is released prematurely. A later queue release
> > then reports a double release. The numeric device references remain
> > balanced.
> >
> > The kernel reported:
> >
> >   ref_tracker: reference already released.
> >   ref_tracker: allocated in:
> >    netdev_queue_update_kobjects+0x23d/0x5c0
> >    netif_set_real_num_tx_queues+0x111/0x820
> >    veth_set_channels+0x327/0x930
> >    ethtool_set_channels+0x3ee/0x490
> >   ref_tracker: freed in:
> >    netdev_queue_release+0xbd/0x130
> >    kobject_put+0x1f9/0x280
> >    sysfs_rtnl_lock+0x18b/0x1f0
> >    xps_rxqs_show+0xad/0x250
> >   WARNING: lib/ref_tracker.c:322 at ref_tracker_free+0x49e/0x6d0
> >   Call Trace:
> >    netdev_queue_release+0xbd/0x130
> >    kobject_put+0x1f9/0x280
> >    netdev_queue_update_kobjects+0x3f9/0x5c0
> >    netif_set_real_num_tx_queues+0x111/0x820
> >    veth_set_channels+0x327/0x930
> >    ethtool_set_channels+0x3ee/0x490
> >
> > Release the tracker before clearing the kobject. Use a full memory barrier
> > to order the tracker access before clearing state_initialized, paired with
> > the control dependency from that check to the new tracker allocation.
> > Keep the device reference until after the reset so that the queue storage
> > remains alive throughout the callback's accesses.
> >
> > Fixes: b0b6fcfa6ad8 ("net-sysfs: remove rtnl_trylock from queue attributes")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Chengfeng Ye <nicoyip.dev@xxxxxxxxx>
> > ---
> >  net/core/net-sysfs.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> > index 352173df7578..b9aadc4c000c 100644
> > --- a/net/core/net-sysfs.c
> > +++ b/net/core/net-sysfs.c
> > @@ -1906,8 +1906,11 @@ static void netdev_queue_release(struct kobject *kobj)
> >  {
> >       struct netdev_queue *queue = to_netdev_queue(kobj);
> >
> > +     netdev_tracker_free(queue->dev, &queue->dev_tracker);
> > +     /* Finish using the tracker before allowing the queue to be re-added. */
> > +     smp_mb();
>
> Can't you use smp_wmb() instead as it's used to order two stores?

I do not think smp_wmb() would be enough: ref_tracker_free() only reads
queue->dev_tracker, it never writes to it. We need to order a load
before a store; smp_wmb() only orders stores.

But what does this smp_mb() pair with in netdev_queue_add_kobject()?

The changelog mentions a control dependency, but netdev_hold() is not
inside the if () clause, and state_initialized is a bitfield, so
READ_ONCE() is not possible. This works thanks to the early return,
but it is implicit and fragile.

Please add an explicit smp_mb() in netdev_queue_add_kobject() after
the state_initialized check, with a comment pointing to
netdev_queue_release(). This is not a fast path, and will help code
review/understanding.

if (unlikely(kobj->state_initialized)) {
        netdev_warn_once(dev, "Cannot re-add tx queues before their
removal completed");
        return -EAGAIN;
}
/* Pairs with smp_mb() in netdev_queue_release(): the previous
* lifetime must be done with queue->dev_tracker before we reuse it.
*/
smp_mb();

Also change ( /* Finish using the tracker before allowing the queue to
be re-added. */)
to the symmetric one (pairs with smp_mb() in  in netdev_queue_add_kobject()...)

pw-bot: cr