Re: [PATCH net v2 1/1] mlxbf_gige: Fix kernel panic at shutdown

[Leon Romanovsky <leon@kernel.org>]

On Tue, Jun 13, 2023 at 12:35:01PM +0300, Vladimir Oltean wrote:
> On Tue, Jun 13, 2023 at 12:09:20PM +0300, Leon Romanovsky wrote:
> > On Tue, Jun 13, 2023 at 11:30:02AM +0300, Vladimir Oltean wrote:
> > > On Tue, Jun 13, 2023 at 10:19:59AM +0300, Leon Romanovsky wrote:
> > > > But once child finishes device_shutdown(), it will be removed from devices_kset
> > > > list and dev->driver should be NULL at that point for the child.
> > > 
> > > What piece of code would make dev->driver be NULL for devices that have
> > > been shut down by device_shutdown()?
> > 
> > You are right here and I'm wrong on that point, dev->driver is set to
> > NULL in all other places where the device is going to be reused and not
> > in device_shutdown().
> > 
> > Unfortunately, it doesn't change a lot in our conversation, as device_shutdown()
> > is very specific call which is called in two flows: kernel_halt() and kernel_restart().
> > 
> > In both flows, it is end game.
> > 
> > Thanks
> 
> Except for the fact that, as mentioned in my first reply to this thread,
> bus drivers may implement .shutdown() the same way as .remove(), so in
> that case, someone *will* unbind the drivers from those child devices,
> *after* .shutdown() was called on the child - and if the child device
> driver isn't prepared to handle that, it can dereference NULL pointers
> and bye bye reboot - the kernel hangs.
> 
> Not really sure where you're aiming with your replies at this stage.

My goal is to explain that "bus drivers may implement .shutdown() 
the same way as .remove()" is wrong implementation and expectation
that all drivers will add "if (!priv) return ..." now is not viable.

Thanks