linux-kernel - Re: Use after free in bcm2835_spi

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <b094c266-99ce-4462-9041-7d1659b13300@gmail.com>
Date:   Wed, 14 Oct 2020 14:20:16 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Mark Brown <broonie@...nel.org>,
        Vladimir Oltean <olteanv@...il.com>
Cc:     Lukas Wunner <lukas@...ner.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        linux-spi <linux-spi@...r.kernel.org>
Subject: Re: Use after free in bcm2835_spi_remove()

On 10/14/20 1:25 PM, Mark Brown wrote:
> On Wed, Oct 14, 2020 at 10:40:35PM +0300, Vladimir Oltean wrote:
>> On Wed, Oct 14, 2020 at 04:09:12PM +0200, Lukas Wunner wrote:
> 
>>> Apparently the problem is that spi_unregister_controller() drops the
>>> last ref on the controller, causing it to be freed, and afterwards we
>>> access the controller's private data, which is part of the same
>>> allocation as struct spi_controller:
> 
>>> bcm2835_spi_remove()
>>>   spi_unregister_controller()
>>>     device_unregister()
>>>       put_device()
>>>         spi_controller_release()  #  spi_master_class.dev_release()
>>> 	  kfree(ctlr)
>>>   bcm2835_dma_release(ctlr, bs)
> 
>> Also see these threads:
>> https://lore.kernel.org/linux-spi/20200922112241.GO4792@sirena.org.uk/T/#t
>> https://lore.kernel.org/linux-spi/270b94fd1e546d0c17a735c1f55500e58522da04.camel@suse.de/T/#u
> 
> Right, the proposed patch is yet another way to fix the issue - it all
> comes back to the fact that you shouldn't be using the driver data after
> unregistering if it was allocated as part of allocating the controller.
> This framework feature is unfortunately quite error prone.

Lukas, your patch works fine for me and is only two lines, so maybe
better suited for stable. How about the attached patch?
-- 
Florian

View attachment "0001-spi-bcm2835-Fix-use-after-free-in-bcm2835_spi_remove.patch" of type "text/x-patch" (4825 bytes)