Re: [PATCH] spi: Fix spi device unregister flow

[Lukas Wunner <lukas@xxxxxxxxx>]

On Mon, May 03, 2021 at 11:15:50AM -0700, Saravana Kannan wrote:
> On Mon, May 3, 2021 at 10:56 AM Lukas Wunner <lukas@xxxxxxxxx> wrote:
> > Without your patch:
> >
> > spi_unregister_device()
> >   device_unregister()
> >     device_del()
> >       bus_remove_device()
> >         device_release_driver() # access to physical SPI device in ->remove()
> >     put_device()
> >       kobject_put()
> >         kref_put()
> >           kobject_release()
> >             kobject_cleanup()
> >               device_release()
> >                 spidev_release()
> >                   spi->controller->cleanup() # controller_state freed
> >
> > With your patch:
> >
> > spi_unregister_device()
> >   spi_cleanup()
> >     spi->controller->cleanup() # controller_state freed
> >   device_unregister()
> >     device_del()
> >       bus_remove_device()
> >         device_release_driver() # access to physical SPI device in ->remove()
[...]
> So, it looks like the fix is simple. We just need to move
> spi_cleanup() to the bottom of spi_unregister_device(). I'll send a
> patch for that rather than reverting this and bringing back the other
> bugs.

That would result in a use-after-free if the call to device_unregister()
indeed releases the last ref to the spi_device (which I'd expect is
usually the case).

However, something like this might work (in spi_unregister_device()):

	device_del(&spi->dev);
	spi_cleanup(spi);
	put_device(&spi->dev);

Thanks,

Lukas