linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Lukas Wunner <lukas@wunner.de>,
	Kamal Dasu <kdasu.kdev@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Subject: [PATCH 4.19 30/39] spi: bcm-qspi: Fix use-after-free on unbind
Date: Thu, 10 Dec 2020 15:27:09 +0100	[thread overview]
Message-ID: <20201210142603.764923655@linuxfoundation.org> (raw)
In-Reply-To: <20201210142602.272595094@linuxfoundation.org>

From: Lukas Wunner <lukas@wunner.de>

commit 63c5395bb7a9777a33f0e7b5906f2c0170a23692 upstream

bcm_qspi_remove() calls spi_unregister_master() even though
bcm_qspi_probe() calls devm_spi_register_master().  The spi_master is
therefore unregistered and freed twice on unbind.

Moreover, since commit 0392727c261b ("spi: bcm-qspi: Handle clock probe
deferral"), bcm_qspi_probe() leaks the spi_master allocation if the call
to devm_clk_get_optional() fails.

Fix by switching over to the new devm_spi_alloc_master() helper which
keeps the private data accessible until the driver has unbound and also
avoids the spi_master leak on probe.

While at it, fix an ordering issue in bcm_qspi_remove() wherein
spi_unregister_master() is called after uninitializing the hardware,
disabling the clock and freeing an IRQ data structure.  The correct
order is to call spi_unregister_master() *before* those teardown steps
because bus accesses may still be ongoing until that function returns.

Fixes: fa236a7ef240 ("spi: bcm-qspi: Add Broadcom MSPI driver")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v4.9+: 123456789abc: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v4.9+
Cc: Kamal Dasu <kdasu.kdev@gmail.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
Tested-by: Florian Fainelli <f.fainelli@gmail.com>
Link: https://lore.kernel.org/r/5e31a9a59fd1c0d0b795b2fe219f25e5ee855f9d.1605121038.git.lukas@wunner.de
Signed-off-by: Mark Brown <broonie@kernel.org>
[sudip: adjust context]
Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/spi/spi-bcm-qspi.c |   34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

--- a/drivers/spi/spi-bcm-qspi.c
+++ b/drivers/spi/spi-bcm-qspi.c
@@ -1223,7 +1223,7 @@ int bcm_qspi_probe(struct platform_devic
 	if (!of_match_node(bcm_qspi_of_match, dev->of_node))
 		return -ENODEV;
 
-	master = spi_alloc_master(dev, sizeof(struct bcm_qspi));
+	master = devm_spi_alloc_master(dev, sizeof(struct bcm_qspi));
 	if (!master) {
 		dev_err(dev, "error allocating spi_master\n");
 		return -ENOMEM;
@@ -1257,21 +1257,17 @@ int bcm_qspi_probe(struct platform_devic
 
 	if (res) {
 		qspi->base[MSPI]  = devm_ioremap_resource(dev, res);
-		if (IS_ERR(qspi->base[MSPI])) {
-			ret = PTR_ERR(qspi->base[MSPI]);
-			goto qspi_resource_err;
-		}
+		if (IS_ERR(qspi->base[MSPI]))
+			return PTR_ERR(qspi->base[MSPI]);
 	} else {
-		goto qspi_resource_err;
+		return 0;
 	}
 
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "bspi");
 	if (res) {
 		qspi->base[BSPI]  = devm_ioremap_resource(dev, res);
-		if (IS_ERR(qspi->base[BSPI])) {
-			ret = PTR_ERR(qspi->base[BSPI]);
-			goto qspi_resource_err;
-		}
+		if (IS_ERR(qspi->base[BSPI]))
+			return PTR_ERR(qspi->base[BSPI]);
 		qspi->bspi_mode = true;
 	} else {
 		qspi->bspi_mode = false;
@@ -1282,18 +1278,14 @@ int bcm_qspi_probe(struct platform_devic
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cs_reg");
 	if (res) {
 		qspi->base[CHIP_SELECT]  = devm_ioremap_resource(dev, res);
-		if (IS_ERR(qspi->base[CHIP_SELECT])) {
-			ret = PTR_ERR(qspi->base[CHIP_SELECT]);
-			goto qspi_resource_err;
-		}
+		if (IS_ERR(qspi->base[CHIP_SELECT]))
+			return PTR_ERR(qspi->base[CHIP_SELECT]);
 	}
 
 	qspi->dev_ids = kcalloc(num_irqs, sizeof(struct bcm_qspi_dev_id),
 				GFP_KERNEL);
-	if (!qspi->dev_ids) {
-		ret = -ENOMEM;
-		goto qspi_resource_err;
-	}
+	if (!qspi->dev_ids)
+		return -ENOMEM;
 
 	for (val = 0; val < num_irqs; val++) {
 		irq = -1;
@@ -1369,7 +1361,7 @@ int bcm_qspi_probe(struct platform_devic
 	qspi->xfer_mode.addrlen = -1;
 	qspi->xfer_mode.hp = -1;
 
-	ret = devm_spi_register_master(&pdev->dev, master);
+	ret = spi_register_master(master);
 	if (ret < 0) {
 		dev_err(dev, "can't register master\n");
 		goto qspi_reg_err;
@@ -1382,8 +1374,6 @@ qspi_reg_err:
 	clk_disable_unprepare(qspi->clk);
 qspi_probe_err:
 	kfree(qspi->dev_ids);
-qspi_resource_err:
-	spi_master_put(master);
 	return ret;
 }
 /* probe function to be called by SoC specific platform driver probe */
@@ -1393,10 +1383,10 @@ int bcm_qspi_remove(struct platform_devi
 {
 	struct bcm_qspi *qspi = platform_get_drvdata(pdev);
 
+	spi_unregister_master(qspi->master);
 	bcm_qspi_hw_uninit(qspi);
 	clk_disable_unprepare(qspi->clk);
 	kfree(qspi->dev_ids);
-	spi_unregister_master(qspi->master);
 
 	return 0;
 }



  parent reply	other threads:[~2020-12-10 18:30 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-10 14:26 [PATCH 4.19 00/39] 4.19.163-rc1 review Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 01/39] pinctrl: baytrail: Replace WARN with dev_info_once when setting direct-irq pin to output Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 02/39] pinctrl: baytrail: Fix pin being driven low for a while on gpiod_get(..., GPIOD_OUT_HIGH) Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 03/39] usb: gadget: f_fs: Use local copy of descriptors for userspace copy Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 04/39] USB: serial: kl5kusb105: fix memleak on open Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 05/39] USB: serial: ch341: add new Product ID for CH341A Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 06/39] USB: serial: ch341: sort device-id entries Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 07/39] USB: serial: option: add Fibocom NL668 variants Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 08/39] USB: serial: option: add support for Thales Cinterion EXS82 Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 09/39] USB: serial: option: fix Quectel BG96 matching Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 10/39] tty: Fix ->pgrp locking in tiocspgrp() Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 11/39] tty: Fix ->session locking Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 12/39] ALSA: hda/realtek: Add mute LED quirk to yet another HP x360 model Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 13/39] ALSA: hda/realtek: Enable headset of ASUS UX482EG & B9400CEA with ALC294 Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 14/39] ALSA: hda/realtek - Add new codec supported for ALC897 Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 15/39] ALSA: hda/generic: Add option to enforce preferred_dacs pairs Greg Kroah-Hartman
2020-12-11  8:36   ` Pavel Machek
2020-12-11  8:41     ` Takashi Iwai
2020-12-11 22:38       ` Pavel Machek
2020-12-11 22:40         ` Takashi Iwai
2020-12-10 14:26 ` [PATCH 4.19 16/39] ftrace: Fix updating FTRACE_FL_TRAMP Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 17/39] cifs: fix potential use-after-free in cifs_echo_request() Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 18/39] i2c: imx: Dont generate STOP condition if arbitration has been lost Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 19/39] scsi: mpt3sas: Fix ioctl timeout Greg Kroah-Hartman
2020-12-10 14:26 ` [PATCH 4.19 20/39] dm writecache: fix the maximum number of arguments Greg Kroah-Hartman
2020-12-10 14:27 ` [PATCH 4.19 21/39] dm: remove invalid sparse __acquires and __releases annotations Greg Kroah-Hartman
2020-12-10 14:27 ` [PATCH 4.19 22/39] mm: list_lru: set shrinker map bit when child nr_items is not zero Greg Kroah-Hartman
2020-12-10 14:27 ` [PATCH 4.19 23/39] mm/swapfile: do not sleep with a spin lock held Greg Kroah-Hartman
2020-12-10 14:27 ` [PATCH 4.19 24/39] x86/uprobes: Do not use prefixes.nbytes when looping over prefixes.bytes Greg Kroah-Hartman
2020-12-10 14:27 ` [PATCH 4.19 25/39] i2c: imx: Fix reset of I2SR_IAL flag Greg Kroah-Hartman
2020-12-10 14:27 ` [PATCH 4.19 26/39] i2c: imx: Check for I2SR_IAL after every byte Greg Kroah-Hartman
2020-12-10 14:27 ` [PATCH 4.19 27/39] speakup: Reject setting the speakup line discipline outside of speakup Greg Kroah-Hartman
2020-12-10 14:27 ` [PATCH 4.19 28/39] iommu/amd: Set DTE[IntTabLen] to represent 512 IRTEs Greg Kroah-Hartman
2020-12-10 14:27 ` [PATCH 4.19 29/39] spi: Introduce device-managed SPI controller allocation Greg Kroah-Hartman
2020-12-10 14:27 ` Greg Kroah-Hartman [this message]
2020-12-10 14:27 ` [PATCH 4.19 31/39] spi: bcm2835: Fix use-after-free on unbind Greg Kroah-Hartman
2020-12-10 14:27 ` [PATCH 4.19 32/39] spi: bcm2835: Release the DMA channel if probe fails after dma_init Greg Kroah-Hartman
2020-12-10 14:27 ` [PATCH 4.19 33/39] tracing: Fix userstacktrace option for instances Greg Kroah-Hartman
2020-12-10 14:27 ` [PATCH 4.19 34/39] gfs2: check for empty rgrp tree in gfs2_ri_update Greg Kroah-Hartman
2020-12-10 14:27 ` [PATCH 4.19 35/39] i2c: qup: Fix error return code in qup_i2c_bam_schedule_desc() Greg Kroah-Hartman
2020-12-10 14:27 ` [PATCH 4.19 36/39] dm writecache: remove BUG() and fail gracefully instead Greg Kroah-Hartman
2020-12-10 14:27 ` [PATCH 4.19 37/39] Input: i8042 - fix error return code in i8042_setup_aux() Greg Kroah-Hartman
2020-12-11 22:35   ` Pavel Machek
2020-12-10 14:27 ` [PATCH 4.19 38/39] netfilter: nf_tables: avoid false-postive lockdep splat Greg Kroah-Hartman
2020-12-10 14:27 ` [PATCH 4.19 39/39] x86/insn-eval: Use new for_each_insn_prefix() macro to loop over prefixes bytes Greg Kroah-Hartman
2020-12-10 21:06 ` [PATCH 4.19 00/39] 4.19.163-rc1 review Pavel Machek
2020-12-11 14:41   ` Greg Kroah-Hartman
2020-12-10 21:23 ` Shuah Khan
2020-12-10 23:44 ` Guenter Roeck
2020-12-11  7:54 ` Naresh Kamboju

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=20201210142603.764923655@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=broonie@kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=kdasu.kdev@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=stable@vger.kernel.org \
    --cc=sudipm.mukherjee@gmail.com \
    /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).