linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Mark Brown <broonie@kernel.org>, Florian Fainelli <f.fainelli@gmail.com>
Cc: linux-spi@vger.kernel.org, Vladimir Oltean <olteanv@gmail.com>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Kamal Dasu <kdasu.kdev@gmail.com>,
	"Nicolas Saenz Julienne" <nsaenzjulienne@suse.de>
Subject: [PATCH 0/4] Use-after-free be gone
Date: Wed, 11 Nov 2020 20:07:00 +0100	[thread overview]
Message-ID: <cover.1605121038.git.lukas@wunner.de> (raw)
In-Reply-To: <bd6eaa71-46cc-0aca-65ff-ae716864cbe3@gmail.com>

Here's my proposal to fix the use-after-free bugs reported by
Sascha Hauer and Florian Fainelli:

I scrutinized all SPI drivers in the v5.10 tree:

* There are 9 drivers with a use-after-free in the ->remove() hook
  caused by accessing driver private data after spi_unregister_controller().

* There are 8 drivers which leak the spi_controller in the ->probe()
  error path because of a missing spi_controller_put().

I'm introducing devm_spi_alloc_master/slave() which automatically
calls spi_controller_put() on ->remove().  This fixes both classes
of bugs while at the same time reducing code amount and complexity
in the ->probe() hook.

I propose that spi_controller_unregister() should no longer release
a reference on the spi_controller.  Instead, drivers need to either
do it themselves or use one of the devm functions introduced herein.
The vast majority of drivers can be converted to the devm functions.
See the commit message of patch [1/4] for the rationale and details.

Enclosed are patches for 3 Broadcom drivers.
Patches for the other drivers are on this branch:
https://github.com/l1k/linux/commits/spi_fixes

@Florian Fainelli:  Could you verify that there are no KASAN splats or
leaks with these patches?  Unfortunately I do not have any SPI-capable
hardware at my disposal right now, so can only compile-test.  You may
want to augment spi_controller_release() with a printk() to log when
the spi_controller is freed.

@Mark Brown:  Patches [2/4] to [4/4] reference the SHA-1 of patch [1/4]
in their stable tags.  Because the hash is unknown to me until you apply
the patch, I've used "123456789abc" as a placeholder.  You'll have to
replace the hash if/when applying.  Alternatively, only apply patch [1/4]
and I'll repost the other patches with the hash fixed up.

Thanks!


Lukas Wunner (4):
  spi: Introduce device-managed SPI controller allocation
  spi: bcm2835: Fix use-after-free on unbind
  spi: bcm2835aux: Fix use-after-free on unbind
  spi: bcm-qspi: Fix use-after-free on unbind

 drivers/spi/spi-bcm-qspi.c   | 34 ++++++++-------------
 drivers/spi/spi-bcm2835.c    | 24 +++++----------
 drivers/spi/spi-bcm2835aux.c | 21 +++++--------
 drivers/spi/spi.c            | 58 +++++++++++++++++++++++++++++++++++-
 include/linux/spi/spi.h      | 19 ++++++++++++
 5 files changed, 103 insertions(+), 53 deletions(-)

-- 
2.28.0


  parent reply	other threads:[~2020-11-11 19:07 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-13 23:48 Use after free in bcm2835_spi_remove() Florian Fainelli
2020-10-14 14:09 ` Lukas Wunner
2020-10-14 19:40   ` Vladimir Oltean
2020-10-14 20:25     ` Mark Brown
2020-10-14 21:20       ` Florian Fainelli
2020-10-22 12:12         ` Lukas Wunner
2020-10-15  5:38       ` Lukas Wunner
2020-10-15 12:53         ` Mark Brown
2020-10-28  9:59           ` Lukas Wunner
2020-10-29 22:24             ` Mark Brown
2020-11-11 19:07 ` Lukas Wunner [this message]
2020-11-11 19:07   ` [PATCH 1/4] spi: Introduce device-managed SPI controller allocation Lukas Wunner
2020-11-11 19:07   ` [PATCH 2/4] spi: bcm2835: Fix use-after-free on unbind Lukas Wunner
2020-11-11 20:18     ` Florian Fainelli
2020-11-11 19:07   ` [PATCH 3/4] spi: bcm2835aux: " Lukas Wunner
2020-11-11 19:07   ` [PATCH 4/4] spi: bcm-qspi: " Lukas Wunner
2020-11-11 21:12     ` Florian Fainelli
2020-11-12 13:50   ` [PATCH 0/4] Use-after-free be gone Mark Brown
2020-11-12 19:39   ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1605121038.git.lukas@wunner.de \
    --to=lukas@wunner.de \
    --cc=broonie@kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=kdasu.kdev@gmail.com \
    --cc=linux-spi@vger.kernel.org \
    --cc=nsaenzjulienne@suse.de \
    --cc=olteanv@gmail.com \
    --cc=s.hauer@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).