Re: [PATCH net-next v2 2/6] net: devmem: support TX over NETMEM_TX_NO_DMA devices

On Tue, May 05, 2026 at 06:18:56PM -0700, Bobby Eshleman wrote:
> On Wed, May 06, 2026 at 12:29:05AM +0000, [email protected] wrote:
> > Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> > - [High] Double lock lockdep splat and potential deadlock in netdev_nl_bind_tx_doit()
> > - [High] Use-After-Free of dma_dev pointer in netdev_nl_bind_tx_doit()
> > - [High] ABA vulnerability and pointer aliasing due to untracked vdev lifecycle
> > --
> > 
> > commit fea3266a76d31b8278bf86dac679860b442a5bea
> > Author: Bobby Eshleman <[email protected]>
> > 
> > net: devmem: support TX over NETMEM_TX_NO_DMA devices
> > 
> > This patch enables devmem TX bindings on netkit virtual devices by routing
> > the binding to the underlying DMA-capable physical device. It extends the
> > skb validation and binding lookup to support the netkit architecture.
> > 

[...]
> 
> > > diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> > > index b8f6076d8007..0e296c3bb677 100644
> > > --- a/net/core/netdev-genl.c
> > > +++ b/net/core/netdev-genl.c
> > [ ... ]
> > > @@ -1164,16 +1197,30 @@ int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
> > [ ... ]
> > > +	bind_dev = netdev_find_netmem_tx_dev(netdev);
> > > +	if (!bind_dev) {
> > > +		err = -EOPNOTSUPP;
> > > +		NL_SET_ERR_MSG(info->extack,
> > > +			       "No DMA-capable device found for netmem TX");
> > > +		goto err_unlock_netdev;
> > > +	}
> > > +
> > > +	if (bind_dev != netdev)
> > > +		netdev_lock(bind_dev);
> > 
> > Will acquiring netdev_lock() on bind_dev while already holding it on netdev
> > trigger a lockdep warning? Since both are struct net_device instances, their
> > locks belong to the exact same lockdep class. Without a mutex_lock_nested()
> > annotation, acquiring them sequentially might trigger a possible circular
> > locking dependency splat, and introduce a potential deadlock if locked in
> > reverse order elsewhere.
> 
> The tests passed with CONFIG_LOCKDEP=y and CONFIG_PROVE_LOCKING=y. Since
> they take the bind_dev != netdev path, I'm not not sure if this is
> needed. Though I admit I don't fully understand mutex subclasses...

Thinking about this more... I think the cleanest approach will be to
just release netdev after looking up bind_dev and then
netdev_lock(bind_dev) immediately after and hold until returning, which
covers both cases bind_dev == netdev and bind_dev != netdev.