Re: [RFC PATCH net-next 1/2] devlink: Hold a reference on parent device

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 21 Jun 2023 09:31:43 +0300 Ido Schimmel wrote:
> Thanks for taking a look.
> 
> Moving the release to devlink_free() [1] was the first thing I tried and
> it indeed solves the problem I mentioned earlier, but creates a new one.
> After devlink_free() returns the devlink instance can still be accessed
> by user space in devlink_get_from_attrs_lock(). If I reload in a loop
> while concurrently removing and adding the device [2], we can hit a UAF
> when trying to acquire the device lock [3].

Ugh, I didn't look at the second patch, it's taking the device lock
before validating that the devlink instance is registered. 
So we need to extend the list of fields which must always be valid :(

Let's try to fix it at the netdevsim level then? AFAIU we only need the
bus to remain loaded for nsim_bus_dev_release to exist? What if we split
netdevsim into two modules, put the bus stuff in a new module called
netdevsim_bus, and leave the rest (driver) in just netdevsim. That way
we can take a ref on netdevsim_bus until all devices are gone, and still
load / unload netdevsim. With unload resulting in all devices getting
auto-deleted.

I haven't looked in detail so maybe you'll immediately tell me it won't
work, but I'm guessing this is how "real" buses work avoid the problem?