linux-kernel - Re: [PATCH] spi: Fix spi device unregister flow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 4 May 2021 11:17:54 +0200
From:   Lukas Wunner <lukas@...ner.de>
To:     Saravana Kannan <saravanak@...gle.com>
Cc:     Mark Brown <broonie@...nel.org>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Guenter Roeck <linux@...ck-us.net>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Android Kernel Team <kernel-team@...roid.com>,
        linux-spi <linux-spi@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] spi: Fix spi device unregister flow

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@...ner.de> 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