Messages in this thread | | | Date | Wed, 10 Nov 2021 18:28:47 +0100 | From | Uwe Kleine-König <> | Subject | Re: [RFC PATCH] spi: fix use-after-free of the add_lock mutex |
| |
Hello,
On Wed, Nov 10, 2021 at 05:08:36PM +0100, Michael Walle wrote: > Commit 6098475d4cb4 ("spi: Fix deadlock when adding SPI controllers on > SPI buses") introduced a per-controller mutex. But mutex_unlock() of > said lock is called after the controller is already freed: > > spi_unregister_controller(ctlr) > -> put_device(&ctlr->dev) > -> spi_controller_release(dev) > mutex_unlock(&ctrl->add_lock)
This is indented in a misleading way. mutex_unlock() has to be on the same level as put_device().
> Move the put_device() after the mutex_unlock(). > > Fixes: 6098475d4cb4 ("spi: Fix deadlock when adding SPI controllers on SPI buses") > Signed-off-by: Michael Walle <michael@walle.cc>
I first thought this was wrong, and the put_device must be dropped altogether, but after some code reading I agree this is the right fix.
Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> --- > I'm not sure if this is the correct fix. I don't know if the put_device() > will have to be protected by the add_lock (remember before, the add_lock > was a global lock).
No, put_device doesn't need to be protected by this lock.
Best regards and thanks for the report and diagnosis, Uwe
-- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [unhandled content-type:application/pgp-signature] | |