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

[Nathan Chancellor <natechancellor@gmail.com>]

On Tue, Dec 08, 2020 at 06:11:45PM +0100, Lukas Wunner wrote:
> 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

I did not really look at the actual patches themselves, just the fact
that I saw the commit title without my patch as a follow up in the
series. If your backport avoids the issue entirely, that is fine by me.

Cheers,
Nathan