Re: kernel BUG at net/core/net-sysfs.c:LINE!

[Andy Shevchenko]

On Mon, Mar 25, 2019 at 11:20:01PM +0800, wanghai (M) wrote:
> thanks , Can it be fixed like this?

I dunno. I think no, it can't.

As far as I can see the issue happened due to freeing entire network device at
the point of putting reference count to the device (struct device is embedded
into struct net_device).

When it happens the access to _any_ field of struct net_device will crash the
system.

Basically it means that put_device() should be carefully placed case-by-case,
because on real hardware the actual device is parent and usually no-one does
access to the child without need. On the contrary the tunX devices are
artificial and are controlled by the network stack.

So, it means we need to do something like

ret = register_netdev(...);
if (ret) {
	put_device(&ndev->dev);
	...
}

But as I mentioned, it would be tricky to not break something else.

P.S. It might be I have missed something, I'm not an expert in network stack.

> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> index 4ff661f..e609c8d 100644
> --- a/net/core/net-sysfs.c
> +++ b/net/core/net-sysfs.c
> @@ -1745,16 +1745,21 @@ int netdev_register_kobject(struct net_device *ndev)
> 
> ÂÂÂÂÂÂÂ error = device_add(dev);
> ÂÂÂÂÂÂÂ if (error)
> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ return error;
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ goto error_put_device;
> 
> ÂÂÂÂÂÂÂ error = register_queue_kobjects(ndev);
> -ÂÂÂÂÂÂ if (error) {
> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ device_del(dev);
> -ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ return error;
> -ÂÂÂÂÂÂ }
> +ÂÂÂÂÂÂ if (error)
> +ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ goto error_device_del;
> 
> ÂÂÂÂÂÂÂ pm_runtime_set_memalloc_noio(dev, true);
> 
> +ÂÂÂÂÂÂ return 0;
> +
> +error_device_del:
> +ÂÂÂÂÂÂ device_del(dev);
> +error_put_device:
> +ÂÂÂÂÂÂ ndev->reg_state = NETREG_RELEASED;
> +ÂÂÂÂÂÂ put_device(dev);
> ÂÂÂÂÂÂÂ return error;
> Â}
> 
> å 2019/3/24 1:16, Andy Shevchenko åé:
> > Nice.
> > 
> > I looked briefly in the flow of this report and it looks like the patch above
> > should be reverted.
> > 
> > The problem is not so easy to fix. One approach is to initialize device
> > (and thus kobject) somewhere in alloc_netdev() and put device in free_netdev()
> > respectively, but this might produce more interesting regressions.
> 

-- 
With Best Regards,
Andy Shevchenko