Re: [PATCH 4.19-stable 4/5] spi: bcm2835aux: Fix use-after-free on unbind

[Lukas Wunner <lukas@wunner.de>]

On Tue, Dec 08, 2020 at 08:47:39AM -0500, Sasha Levin wrote:
> On Tue, Dec 08, 2020 at 08:32:41AM +0100, Lukas Wunner wrote:
> > On Mon, Dec 07, 2020 at 05:49:01PM -0700, Nathan Chancellor wrote:
> > > On Sun, Dec 06, 2020 at 01:31:03PM +0100, Lukas Wunner wrote:
> > > > [ Upstream commit e13ee6cc4781edaf8c7321bee19217e3702ed481 ]
> > > >
> > > > bcm2835aux_spi_remove() accesses the driver's private data after calling
> > > > spi_unregister_master() even though that function releases the last
> > > > reference on the spi_master and thereby frees the private data.
> > > >
> > > > Fix by switching over to the new devm_spi_alloc_master() helper which
> > > > keeps the private data accessible until the driver has unbound.
> > > >
> > > > Fixes: b9dd3f6d4172 ("spi: bcm2835aux: Fix controller unregister order")
> > > > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > > > Cc: <stable@vger.kernel.org> # v4.4+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
> > > > Cc: <stable@vger.kernel.org> # v4.4+: b9dd3f6d4172: spi: bcm2835aux: Fix controller unregister order
> > > > Cc: <stable@vger.kernel.org> # v4.4+
> > > > Link: https://lore.kernel.org/r/b290b06357d0c0bdee9cecc539b840a90630f101.1605121038.git.lukas@wunner.de
> > > > Signed-off-by: Mark Brown <broonie@kernel.org>
> > > 
> > > Please ensure that commit d853b3406903 ("spi: bcm2835aux: Restore err
> > > assignment in bcm2835aux_spi_probe") is picked up with this patch in all
> > > of the stable trees that it is applied to.
> > 
> > That shouldn't be necessary as I've made sure that the backports to
> > 4.19 and earlier do not exhibit the issue fixed by d853b3406903.
> > 
> > However, nobody is perfect, so if I've missed anything, please let
> > me know.
> 
> Could we instead have the backports exhibit the issue (like they did
> upstream) and then take d853b3406903 on top?

The upstream commit e13ee6cc4781 did not apply cleanly to 4.19 and earlier,
several adjustments were required.  Could I have made it so that the fixup
d853b3406903 would have still been required?  Probably, but it seems a
little silly to submit a known-bad patch.

Thanks,

Lukas