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

From: Lukas Wunner
Date: Tue May 04 2021 - 05:18:58 EST


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