Re: Use after free in bcm2835_spi_remove()

2020-10-29 Thread Mark Brown
On Wed, Oct 28, 2020 at 10:59:46AM +0100, Lukas Wunner wrote: > On Thu, Oct 15, 2020 at 01:53:35PM +0100, Mark Brown wrote: > > This feels a bit icky - we're masking a standard use after free bug that > > affects devm in general, not just this instance, and so while it will > > work it doesn't fee

Re: Use after free in bcm2835_spi_remove()

2020-10-28 Thread Lukas Wunner
On Thu, Oct 15, 2020 at 01:53:35PM +0100, Mark Brown wrote: > On Thu, Oct 15, 2020 at 07:38:29AM +0200, Lukas Wunner wrote: > > On Wed, Oct 14, 2020 at 09:25:05PM +0100, Mark Brown wrote: > > How about holding a ref on the controller as long as the SPI driver > > is bound to the controller's parent

Re: Use after free in bcm2835_spi_remove()

2020-10-22 Thread Lukas Wunner
On Wed, Oct 14, 2020 at 02:20:16PM -0700, Florian Fainelli wrote: > In bcm2835_spi_remove(), spi_controller_unregister() will free the ctlr > reference which will lead to an use after free in bcm2835_release_dma(). > > To avoid this use after free, allocate the bcm2835_spi structure with a > diffe

Re: Use after free in bcm2835_spi_remove()

2020-10-15 Thread Mark Brown
On Thu, Oct 15, 2020 at 07:38:29AM +0200, Lukas Wunner wrote: > On Wed, Oct 14, 2020 at 09:25:05PM +0100, Mark Brown wrote: > > 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 i

Re: Use after free in bcm2835_spi_remove()

2020-10-14 Thread Lukas Wunner
[cc += Sascha] On Wed, Oct 14, 2020 at 09:25:05PM +0100, Mark Brown 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 > > >

Re: Use after free in bcm2835_spi_remove()

2020-10-14 Thread Florian Fainelli
ring 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 From a4ee9da1ef09f9ddb040

Re: Use after free in bcm2835_spi_remove()

2020-10-14 Thread Mark Brown
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 control

Re: Use after free in bcm2835_spi_remove()

2020-10-14 Thread Vladimir Oltean
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_c

Re: Use after free in bcm2835_spi_remove()

2020-10-14 Thread Lukas Wunner
On Tue, Oct 13, 2020 at 04:48:42PM -0700, Florian Fainelli wrote: > With KASAN now working on ARM 32-bit, I was able to get the following > trace upon reboot which invokes bcm2835_spi_shutdown() calling > bcm2835_spi_remove(), the same can be triggered by doing a driver unbind: Thank you for the r

Use after free in bcm2835_spi_remove()

2020-10-14 Thread Florian Fainelli
Hi Lukas, With KASAN now working on ARM 32-bit, I was able to get the following trace upon reboot which invokes bcm2835_spi_shutdown() calling bcm2835_spi_remove(), the same can be triggered by doing a driver unbind: # pwd /sys/devices/platform/rdb/47e204800.spi/driver # echo 47e204800.spi > unbi