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@xxxxxxxxx> > > > Cc: <stable@xxxxxxxxxxxxxxx> # v4.4+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation > > > Cc: <stable@xxxxxxxxxxxxxxx> # v4.4+: b9dd3f6d4172: spi: bcm2835aux: Fix controller unregister order > > > Cc: <stable@xxxxxxxxxxxxxxx> # v4.4+ > > > Link: https://lore.kernel.org/r/b290b06357d0c0bdee9cecc539b840a90630f101.1605121038.git.lukas@xxxxxxxxx > > > Signed-off-by: Mark Brown <broonie@xxxxxxxxxx> > > > > 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.
Not silly, there are two motives here: 1. We don't want to diverge too much with backports, which means that they need to be minimal. 2. It'll make auditing later easier. What will happen now is that after this patch is merges, we'll trigger a warning saying that there's a fix upstream for one of these patches, and we'll end up wasting the time (of probably a few folks) figuring this out. Note I'm not asking to submit a broken patch, but I'm asking to submit a minimal backport followed by the upstream fix to that upstream bug :) -- Thanks, Sasha
