All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/17] SPI probe/remove sanitization for 5.11
@ 2020-12-07  8:17 Lukas Wunner
  2020-12-07  8:17 ` [PATCH 01/17] spi: davinci: Fix use-after-free on unbind Lukas Wunner
                   ` (17 more replies)
  0 siblings, 18 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown
  Cc: linux-spi, Chuanhong Guo, Tomer Maimon, Piotr Bugalski, Tudor Ambarus

Patches [01/17] to [14/17] are reposts, now based on spi.git/for-5.11
and amended with all collected Acked-by + Reviewed-by tags.

Links to their original submission:
https://lore.kernel.org/linux-spi/dd060534490eca5e946eb9165916542b01a9358d.1604874488.git.lukas@wunner.de/
https://lore.kernel.org/linux-spi/73adc6ba84a4f968f2e1499a776e5c928fbdde56.1605512876.git.lukas@wunner.de/T/#t

Patches [15/17] to [17/17] are new material.

Lukas Wunner (17):
  spi: davinci: Fix use-after-free on unbind
  spi: spi-geni-qcom: Fix use-after-free on unbind
  spi: spi-qcom-qspi: Fix use-after-free on unbind
  spi: spi-sh: Fix use-after-free on unbind
  spi: pxa2xx: Fix use-after-free on unbind
  spi: rpc-if: Fix use-after-free on unbind
  spi: mxic: Don't leak SPI master in probe error path
  spi: spi-mtk-nor: Don't leak SPI master in probe error path
  spi: gpio: Don't leak SPI master in probe error path
  spi: rb4xx: Don't leak SPI master in probe error path
  spi: sc18is602: Don't leak SPI master in probe error path
  media: netup_unidvb: Don't leak SPI master in probe error path
  spi: mt7621: Disable clock in probe error path
  spi: mt7621: Don't leak SPI master in probe error path
  spi: ar934x: Don't leak SPI master in probe error path
  spi: npcm-fiu: Disable clock in probe error path
  spi: atmel-quadspi: Fix use-after-free on unbind

 .../media/pci/netup_unidvb/netup_unidvb_spi.c |  5 ++-
 drivers/spi/atmel-quadspi.c                   | 15 +++----
 drivers/spi/spi-ar934x.c                      | 14 +++++--
 drivers/spi/spi-davinci.c                     |  2 +-
 drivers/spi/spi-geni-qcom.c                   |  3 +-
 drivers/spi/spi-gpio.c                        | 15 +------
 drivers/spi/spi-mt7621.c                      |  9 +++-
 drivers/spi/spi-mtk-nor.c                     |  2 +-
 drivers/spi/spi-mxic.c                        | 10 +----
 drivers/spi/spi-npcm-fiu.c                    |  8 +++-
 drivers/spi/spi-pxa2xx.c                      |  5 +--
 drivers/spi/spi-qcom-qspi.c                   | 42 +++++++------------
 drivers/spi/spi-rb4xx.c                       |  2 +-
 drivers/spi/spi-rpc-if.c                      |  9 +---
 drivers/spi/spi-sc18is602.c                   | 13 +-----
 drivers/spi/spi-sh.c                          | 13 ++----
 16 files changed, 66 insertions(+), 101 deletions(-)

-- 
2.29.2


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 01/17] spi: davinci: Fix use-after-free on unbind
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07  8:17 ` [PATCH 02/17] spi: spi-geni-qcom: " Lukas Wunner
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi

davinci_spi_remove() accesses the driver's private data after it's been
freed with spi_master_put().

Fix by moving the spi_master_put() to the end of the function.

Fixes: fe5fd2540947 ("spi: davinci: Use dma_request_chan() for requesting DMA channel")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Acked-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Cc: <stable@vger.kernel.org> # v4.7+
---
 drivers/spi/spi-davinci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-davinci.c b/drivers/spi/spi-davinci.c
index 818f2b22875d..7453a1dbbc06 100644
--- a/drivers/spi/spi-davinci.c
+++ b/drivers/spi/spi-davinci.c
@@ -1040,13 +1040,13 @@ static int davinci_spi_remove(struct platform_device *pdev)
 	spi_bitbang_stop(&dspi->bitbang);
 
 	clk_disable_unprepare(dspi->clk);
-	spi_master_put(master);
 
 	if (dspi->dma_rx) {
 		dma_release_channel(dspi->dma_rx);
 		dma_release_channel(dspi->dma_tx);
 	}
 
+	spi_master_put(master);
 	return 0;
 }
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 02/17] spi: spi-geni-qcom: Fix use-after-free on unbind
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
  2020-12-07  8:17 ` [PATCH 01/17] spi: davinci: Fix use-after-free on unbind Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07  8:17 ` [PATCH 03/17] spi: spi-qcom-qspi: " Lukas Wunner
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi

spi_geni_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.

Moreover, since commit 1a9e489e6128 ("spi: spi-geni-qcom: Use OPP API to
set clk/perf state"), spi_geni_probe() leaks the spi_master allocation
if the calls to dev_pm_opp_set_clkname() or dev_pm_opp_of_add_table()
fail.

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.

Fixes: 561de45f72bd ("spi: spi-geni-qcom: Add SPI driver support for GENI based QUP")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v4.20+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v4.20+
Cc: Rajendra Nayak <rnayak@codeaurora.org>
Cc: Girish Mahadevan <girishm@codeaurora.org>
---
 drivers/spi/spi-geni-qcom.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 25810a7eef10..0e3d8e6c08f4 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -603,7 +603,7 @@ static int spi_geni_probe(struct platform_device *pdev)
 	if (IS_ERR(clk))
 		return PTR_ERR(clk);
 
-	spi = spi_alloc_master(dev, sizeof(*mas));
+	spi = devm_spi_alloc_master(dev, sizeof(*mas));
 	if (!spi)
 		return -ENOMEM;
 
@@ -673,7 +673,6 @@ static int spi_geni_probe(struct platform_device *pdev)
 	free_irq(mas->irq, spi);
 spi_geni_probe_runtime_disable:
 	pm_runtime_disable(dev);
-	spi_master_put(spi);
 	dev_pm_opp_of_remove_table(&pdev->dev);
 put_clkname:
 	dev_pm_opp_put_clkname(mas->se.opp_table);
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 03/17] spi: spi-qcom-qspi: Fix use-after-free on unbind
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
  2020-12-07  8:17 ` [PATCH 01/17] spi: davinci: Fix use-after-free on unbind Lukas Wunner
  2020-12-07  8:17 ` [PATCH 02/17] spi: spi-geni-qcom: " Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07  8:17 ` [PATCH 04/17] spi: spi-sh: " Lukas Wunner
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi

qcom_qspi_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: f79a158d37c2 ("spi: spi-qcom-qspi: Use OPP API to set clk/perf state")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v5.9+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v5.9+
Cc: Rajendra Nayak <rnayak@codeaurora.org>
---
 drivers/spi/spi-qcom-qspi.c | 42 ++++++++++++++-----------------------
 1 file changed, 16 insertions(+), 26 deletions(-)

diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c
index 5eed88af6899..8863be370884 100644
--- a/drivers/spi/spi-qcom-qspi.c
+++ b/drivers/spi/spi-qcom-qspi.c
@@ -462,7 +462,7 @@ static int qcom_qspi_probe(struct platform_device *pdev)
 
 	dev = &pdev->dev;
 
-	master = spi_alloc_master(dev, sizeof(*ctrl));
+	master = devm_spi_alloc_master(dev, sizeof(*ctrl));
 	if (!master)
 		return -ENOMEM;
 
@@ -473,54 +473,49 @@ static int qcom_qspi_probe(struct platform_device *pdev)
 	spin_lock_init(&ctrl->lock);
 	ctrl->dev = dev;
 	ctrl->base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(ctrl->base)) {
-		ret = PTR_ERR(ctrl->base);
-		goto exit_probe_master_put;
-	}
+	if (IS_ERR(ctrl->base))
+		return PTR_ERR(ctrl->base);
 
 	ctrl->clks = devm_kcalloc(dev, QSPI_NUM_CLKS,
 				  sizeof(*ctrl->clks), GFP_KERNEL);
-	if (!ctrl->clks) {
-		ret = -ENOMEM;
-		goto exit_probe_master_put;
-	}
+	if (!ctrl->clks)
+		return -ENOMEM;
 
 	ctrl->clks[QSPI_CLK_CORE].id = "core";
 	ctrl->clks[QSPI_CLK_IFACE].id = "iface";
 	ret = devm_clk_bulk_get(dev, QSPI_NUM_CLKS, ctrl->clks);
 	if (ret)
-		goto exit_probe_master_put;
+		return ret;
 
 	ctrl->icc_path_cpu_to_qspi = devm_of_icc_get(dev, "qspi-config");
-	if (IS_ERR(ctrl->icc_path_cpu_to_qspi)) {
-		ret = dev_err_probe(dev, PTR_ERR(ctrl->icc_path_cpu_to_qspi),
-				    "Failed to get cpu path\n");
-		goto exit_probe_master_put;
-	}
+	if (IS_ERR(ctrl->icc_path_cpu_to_qspi))
+		return dev_err_probe(dev, PTR_ERR(ctrl->icc_path_cpu_to_qspi),
+				     "Failed to get cpu path\n");
+
 	/* Set BW vote for register access */
 	ret = icc_set_bw(ctrl->icc_path_cpu_to_qspi, Bps_to_icc(1000),
 				Bps_to_icc(1000));
 	if (ret) {
 		dev_err(ctrl->dev, "%s: ICC BW voting failed for cpu: %d\n",
 				__func__, ret);
-		goto exit_probe_master_put;
+		return ret;
 	}
 
 	ret = icc_disable(ctrl->icc_path_cpu_to_qspi);
 	if (ret) {
 		dev_err(ctrl->dev, "%s: ICC disable failed for cpu: %d\n",
 				__func__, ret);
-		goto exit_probe_master_put;
+		return ret;
 	}
 
 	ret = platform_get_irq(pdev, 0);
 	if (ret < 0)
-		goto exit_probe_master_put;
+		return ret;
 	ret = devm_request_irq(dev, ret, qcom_qspi_irq,
 			IRQF_TRIGGER_HIGH, dev_name(dev), ctrl);
 	if (ret) {
 		dev_err(dev, "Failed to request irq %d\n", ret);
-		goto exit_probe_master_put;
+		return ret;
 	}
 
 	master->max_speed_hz = 300000000;
@@ -537,10 +532,8 @@ static int qcom_qspi_probe(struct platform_device *pdev)
 	master->auto_runtime_pm = true;
 
 	ctrl->opp_table = dev_pm_opp_set_clkname(&pdev->dev, "core");
-	if (IS_ERR(ctrl->opp_table)) {
-		ret = PTR_ERR(ctrl->opp_table);
-		goto exit_probe_master_put;
-	}
+	if (IS_ERR(ctrl->opp_table))
+		return PTR_ERR(ctrl->opp_table);
 	/* OPP table is optional */
 	ret = dev_pm_opp_of_add_table(&pdev->dev);
 	if (ret && ret != -ENODEV) {
@@ -562,9 +555,6 @@ static int qcom_qspi_probe(struct platform_device *pdev)
 exit_probe_put_clkname:
 	dev_pm_opp_put_clkname(ctrl->opp_table);
 
-exit_probe_master_put:
-	spi_master_put(master);
-
 	return ret;
 }
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 04/17] spi: spi-sh: Fix use-after-free on unbind
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
                   ` (2 preceding siblings ...)
  2020-12-07  8:17 ` [PATCH 03/17] spi: spi-qcom-qspi: " Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07  8:17 ` [PATCH 05/17] spi: pxa2xx: " Lukas Wunner
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi

spi_sh_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: 680c1305e259 ("spi/spi_sh: use spi_unregister_master instead of spi_master_put in remove path")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v3.0+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v3.0+
Cc: Axel Lin <axel.lin@ingics.com>
---
 drivers/spi/spi-sh.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-sh.c b/drivers/spi/spi-sh.c
index 20bdae5fdf3b..15123a8f41e1 100644
--- a/drivers/spi/spi-sh.c
+++ b/drivers/spi/spi-sh.c
@@ -440,7 +440,7 @@ static int spi_sh_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
-	master = spi_alloc_master(&pdev->dev, sizeof(struct spi_sh_data));
+	master = devm_spi_alloc_master(&pdev->dev, sizeof(struct spi_sh_data));
 	if (master == NULL) {
 		dev_err(&pdev->dev, "spi_alloc_master error.\n");
 		return -ENOMEM;
@@ -458,16 +458,14 @@ static int spi_sh_probe(struct platform_device *pdev)
 		break;
 	default:
 		dev_err(&pdev->dev, "No support width\n");
-		ret = -ENODEV;
-		goto error1;
+		return -ENODEV;
 	}
 	ss->irq = irq;
 	ss->master = master;
 	ss->addr = devm_ioremap(&pdev->dev, res->start, resource_size(res));
 	if (ss->addr == NULL) {
 		dev_err(&pdev->dev, "ioremap error.\n");
-		ret = -ENOMEM;
-		goto error1;
+		return -ENOMEM;
 	}
 	INIT_LIST_HEAD(&ss->queue);
 	spin_lock_init(&ss->lock);
@@ -477,7 +475,7 @@ static int spi_sh_probe(struct platform_device *pdev)
 	ret = request_irq(irq, spi_sh_irq, 0, "spi_sh", ss);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "request_irq error\n");
-		goto error1;
+		return ret;
 	}
 
 	master->num_chipselect = 2;
@@ -496,9 +494,6 @@ static int spi_sh_probe(struct platform_device *pdev)
 
  error3:
 	free_irq(irq, ss);
- error1:
-	spi_master_put(master);
-
 	return ret;
 }
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 05/17] spi: pxa2xx: Fix use-after-free on unbind
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
                   ` (3 preceding siblings ...)
  2020-12-07  8:17 ` [PATCH 04/17] spi: spi-sh: " Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07  8:17 ` [PATCH 06/17] spi: rpc-if: " Lukas Wunner
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi

pxa2xx_spi_remove() accesses the driver's private data after calling
spi_unregister_controller() even though that function releases the last
reference on the spi_controller and thereby frees the private data.

Fix by switching over to the new devm_spi_alloc_master/slave() helper
which keeps the private data accessible until the driver has unbound.

Fixes: 32e5b57232c0 ("spi: pxa2xx: Fix controller unregister order")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v2.6.17+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v2.6.17+: 32e5b57232c0: spi: pxa2xx: Fix controller unregister order
Cc: <stable@vger.kernel.org> # v2.6.17+
---
 drivers/spi/spi-pxa2xx.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 62a0f0f86553..bd2354fd438d 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1691,9 +1691,9 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 	}
 
 	if (platform_info->is_slave)
-		controller = spi_alloc_slave(dev, sizeof(struct driver_data));
+		controller = devm_spi_alloc_slave(dev, sizeof(*drv_data));
 	else
-		controller = spi_alloc_master(dev, sizeof(struct driver_data));
+		controller = devm_spi_alloc_master(dev, sizeof(*drv_data));
 
 	if (!controller) {
 		dev_err(&pdev->dev, "cannot alloc spi_controller\n");
@@ -1916,7 +1916,6 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
 	free_irq(ssp->irq, drv_data);
 
 out_error_controller_alloc:
-	spi_controller_put(controller);
 	pxa_ssp_free(ssp);
 	return status;
 }
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 06/17] spi: rpc-if: Fix use-after-free on unbind
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
                   ` (4 preceding siblings ...)
  2020-12-07  8:17 ` [PATCH 05/17] spi: pxa2xx: " Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07  8:17 ` [PATCH 07/17] spi: mxic: Don't leak SPI master in probe error path Lukas Wunner
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi

rpcif_spi_remove() accesses the driver's private data after calling
spi_unregister_controller() even though that function releases the last
reference on the spi_controller 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: eb8d6d464a27 ("spi: add Renesas RPC-IF driver")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v5.9+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v5.9+
Cc: Sergei Shtylyov <s.shtylyov@omprussia.ru>
---
 drivers/spi/spi-rpc-if.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-rpc-if.c b/drivers/spi/spi-rpc-if.c
index ed3e548227f4..3579675485a5 100644
--- a/drivers/spi/spi-rpc-if.c
+++ b/drivers/spi/spi-rpc-if.c
@@ -134,7 +134,7 @@ static int rpcif_spi_probe(struct platform_device *pdev)
 	struct rpcif *rpc;
 	int error;
 
-	ctlr = spi_alloc_master(&pdev->dev, sizeof(*rpc));
+	ctlr = devm_spi_alloc_master(&pdev->dev, sizeof(*rpc));
 	if (!ctlr)
 		return -ENOMEM;
 
@@ -159,13 +159,8 @@ static int rpcif_spi_probe(struct platform_device *pdev)
 	error = spi_register_controller(ctlr);
 	if (error) {
 		dev_err(&pdev->dev, "spi_register_controller failed\n");
-		goto err_put_ctlr;
+		rpcif_disable_rpm(rpc);
 	}
-	return 0;
-
-err_put_ctlr:
-	rpcif_disable_rpm(rpc);
-	spi_controller_put(ctlr);
 
 	return error;
 }
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 07/17] spi: mxic: Don't leak SPI master in probe error path
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
                   ` (5 preceding siblings ...)
  2020-12-07  8:17 ` [PATCH 06/17] spi: rpc-if: " Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07  8:17 ` [PATCH 08/17] spi: spi-mtk-nor: " Lukas Wunner
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi

If the calls to devm_clk_get() or devm_ioremap_resource() fail on probe
of the Macronix SPI driver, the spi_master struct is erroneously not freed.

Fix by switching over to the new devm_spi_alloc_master() helper.

Fixes: b942d80b0a39 ("spi: Add MXIC controller driver")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v5.0+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v5.0+
Cc: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/spi/spi-mxic.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-mxic.c b/drivers/spi/spi-mxic.c
index 8c630acb0110..96b418293bf2 100644
--- a/drivers/spi/spi-mxic.c
+++ b/drivers/spi/spi-mxic.c
@@ -529,7 +529,7 @@ static int mxic_spi_probe(struct platform_device *pdev)
 	struct mxic_spi *mxic;
 	int ret;
 
-	master = spi_alloc_master(&pdev->dev, sizeof(struct mxic_spi));
+	master = devm_spi_alloc_master(&pdev->dev, sizeof(struct mxic_spi));
 	if (!master)
 		return -ENOMEM;
 
@@ -574,15 +574,9 @@ static int mxic_spi_probe(struct platform_device *pdev)
 	ret = spi_register_master(master);
 	if (ret) {
 		dev_err(&pdev->dev, "spi_register_master failed\n");
-		goto err_put_master;
+		pm_runtime_disable(&pdev->dev);
 	}
 
-	return 0;
-
-err_put_master:
-	spi_master_put(master);
-	pm_runtime_disable(&pdev->dev);
-
 	return ret;
 }
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 08/17] spi: spi-mtk-nor: Don't leak SPI master in probe error path
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
                   ` (6 preceding siblings ...)
  2020-12-07  8:17 ` [PATCH 07/17] spi: mxic: Don't leak SPI master in probe error path Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07  8:17 ` [PATCH 09/17] spi: gpio: " Lukas Wunner
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi

If the call to devm_spi_register_controller() fails on probe of the
MediaTek SPI NOR driver, the spi_controller struct is erroneously not
freed.

Since commit a1daaa991ed1 ("spi: spi-mtk-nor: use dma_alloc_coherent()
for bounce buffer"), the same happens if the call to
dmam_alloc_coherent() fails.

Since commit 3bfd9103c7af ("spi: spi-mtk-nor: Add power management
support"), the same happens if the call to mtk_nor_enable_clk() fails.

Fix by switching over to the new devm_spi_alloc_master() helper.

Fixes: 881d1ee9fe81 ("spi: add support for mediatek spi-nor controller")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Reviewed-by: Ikjoon Jang <ikjn@chromium.org>
Cc: <stable@vger.kernel.org> # v5.7+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v5.7+
Cc: Chuanhong Guo <gch981213@gmail.com>
---
 drivers/spi/spi-mtk-nor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
index bf2d0f9cdd19..2e2f36a2e385 100644
--- a/drivers/spi/spi-mtk-nor.c
+++ b/drivers/spi/spi-mtk-nor.c
@@ -781,7 +781,7 @@ static int mtk_nor_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	ctlr = spi_alloc_master(&pdev->dev, sizeof(*sp));
+	ctlr = devm_spi_alloc_master(&pdev->dev, sizeof(*sp));
 	if (!ctlr) {
 		dev_err(&pdev->dev, "failed to allocate spi controller\n");
 		return -ENOMEM;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 09/17] spi: gpio: Don't leak SPI master in probe error path
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
                   ` (7 preceding siblings ...)
  2020-12-07  8:17 ` [PATCH 08/17] spi: spi-mtk-nor: " Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07  8:17 ` [PATCH 10/17] spi: rb4xx: " Lukas Wunner
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi

If the call to devm_spi_register_master() fails on probe of the GPIO SPI
driver, the spi_master struct is erroneously not freed:

After allocating the spi_master, its reference count is 1.  The driver
unconditionally decrements the reference count on unbind using a devm
action.  Before calling devm_spi_register_master(), the driver
unconditionally increments the reference count because on success,
that function will decrement the reference count on unbind.  However on
failure, devm_spi_register_master() does *not* decrement the reference
count, so the spi_master is leaked.

The issue was introduced by commits 8b797490b4db ("spi: gpio: Make sure
spi_master_put() is called in every error path") and 79567c1a321e ("spi:
gpio: Use devm_spi_register_master()"), which sought to plug leaks
introduced by 9b00bc7b901f ("spi: spi-gpio: Rewrite to use GPIO
descriptors") but missed this remaining leak.

The situation was later aggravated by commit d3b0ffa1d75d ("spi: gpio:
prevent memory leak in spi_gpio_probe"), which introduced a
use-after-free because it releases a reference on the spi_master if
devm_add_action_or_reset() fails even though the function already
does that.

Fix by switching over to the new devm_spi_alloc_master() helper.

Fixes: 9b00bc7b901f ("spi: spi-gpio: Rewrite to use GPIO descriptors")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Cc: <stable@vger.kernel.org> # v4.17+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v5.1-: 8b797490b4db: spi: gpio: Make sure spi_master_put() is called in every error path
Cc: <stable@vger.kernel.org> # v5.1-: 45beec351998: spi: bitbang: Introduce spi_bitbang_init()
Cc: <stable@vger.kernel.org> # v5.1-: 79567c1a321e: spi: gpio: Use devm_spi_register_master()
Cc: <stable@vger.kernel.org> # v5.4-: d3b0ffa1d75d: spi: gpio: prevent memory leak in spi_gpio_probe
Cc: <stable@vger.kernel.org> # v4.17+
Cc: Navid Emamdoost <navid.emamdoost@gmail.com>
Cc: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/spi/spi-gpio.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/spi/spi-gpio.c b/drivers/spi/spi-gpio.c
index 7ceb0ba27b75..0584f4d2fde2 100644
--- a/drivers/spi/spi-gpio.c
+++ b/drivers/spi/spi-gpio.c
@@ -350,11 +350,6 @@ static int spi_gpio_probe_pdata(struct platform_device *pdev,
 	return 0;
 }
 
-static void spi_gpio_put(void *data)
-{
-	spi_master_put(data);
-}
-
 static int spi_gpio_probe(struct platform_device *pdev)
 {
 	int				status;
@@ -363,16 +358,10 @@ static int spi_gpio_probe(struct platform_device *pdev)
 	struct device			*dev = &pdev->dev;
 	struct spi_bitbang		*bb;
 
-	master = spi_alloc_master(dev, sizeof(*spi_gpio));
+	master = devm_spi_alloc_master(dev, sizeof(*spi_gpio));
 	if (!master)
 		return -ENOMEM;
 
-	status = devm_add_action_or_reset(&pdev->dev, spi_gpio_put, master);
-	if (status) {
-		spi_master_put(master);
-		return status;
-	}
-
 	if (pdev->dev.of_node)
 		status = spi_gpio_probe_dt(pdev, master);
 	else
@@ -432,7 +421,7 @@ static int spi_gpio_probe(struct platform_device *pdev)
 	if (status)
 		return status;
 
-	return devm_spi_register_master(&pdev->dev, spi_master_get(master));
+	return devm_spi_register_master(&pdev->dev, master);
 }
 
 MODULE_ALIAS("platform:" DRIVER_NAME);
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 10/17] spi: rb4xx: Don't leak SPI master in probe error path
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
                   ` (8 preceding siblings ...)
  2020-12-07  8:17 ` [PATCH 09/17] spi: gpio: " Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07  8:17 ` [PATCH 11/17] spi: sc18is602: " Lukas Wunner
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi

If the calls to devm_clk_get(), devm_spi_register_master() or
clk_prepare_enable() fail on probe of the Mikrotik RB4xx SPI driver,
the spi_master struct is erroneously not freed.

Fix by switching over to the new devm_spi_alloc_master() helper.

Fixes: 05aec357871f ("spi: Add SPI driver for Mikrotik RB4xx series boards")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v4.2+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v4.2+
Cc: Bert Vermeulen <bert@biot.com>
---
 drivers/spi/spi-rb4xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-rb4xx.c b/drivers/spi/spi-rb4xx.c
index 8aa51beb4ff3..9f97d18a05c1 100644
--- a/drivers/spi/spi-rb4xx.c
+++ b/drivers/spi/spi-rb4xx.c
@@ -143,7 +143,7 @@ static int rb4xx_spi_probe(struct platform_device *pdev)
 	if (IS_ERR(spi_base))
 		return PTR_ERR(spi_base);
 
-	master = spi_alloc_master(&pdev->dev, sizeof(*rbspi));
+	master = devm_spi_alloc_master(&pdev->dev, sizeof(*rbspi));
 	if (!master)
 		return -ENOMEM;
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 11/17] spi: sc18is602: Don't leak SPI master in probe error path
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
                   ` (9 preceding siblings ...)
  2020-12-07  8:17 ` [PATCH 10/17] spi: rb4xx: " Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07  8:17 ` [PATCH 12/17] media: netup_unidvb: " Lukas Wunner
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi

If the call to devm_gpiod_get_optional() fails on probe of the NXP
SC18IS602/603 SPI driver, the spi_master struct is erroneously not freed.

Fix by switching over to the new devm_spi_alloc_master() helper.

Fixes: f99008013e19 ("spi: sc18is602: Add reset control via gpio pin.")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v4.9+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v4.9+
Cc: Phil Reid <preid@electromag.com.au>
---
 drivers/spi/spi-sc18is602.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi-sc18is602.c b/drivers/spi/spi-sc18is602.c
index ee0f3edf49cd..297c512069a5 100644
--- a/drivers/spi/spi-sc18is602.c
+++ b/drivers/spi/spi-sc18is602.c
@@ -238,13 +238,12 @@ static int sc18is602_probe(struct i2c_client *client,
 	struct sc18is602_platform_data *pdata = dev_get_platdata(dev);
 	struct sc18is602 *hw;
 	struct spi_master *master;
-	int error;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
 				     I2C_FUNC_SMBUS_WRITE_BYTE_DATA))
 		return -EINVAL;
 
-	master = spi_alloc_master(dev, sizeof(struct sc18is602));
+	master = devm_spi_alloc_master(dev, sizeof(struct sc18is602));
 	if (!master)
 		return -ENOMEM;
 
@@ -298,15 +297,7 @@ static int sc18is602_probe(struct i2c_client *client,
 	master->min_speed_hz = hw->freq / 128;
 	master->max_speed_hz = hw->freq / 4;
 
-	error = devm_spi_register_master(dev, master);
-	if (error)
-		goto error_reg;
-
-	return 0;
-
-error_reg:
-	spi_master_put(master);
-	return error;
+	return devm_spi_register_master(dev, master);
 }
 
 static const struct i2c_device_id sc18is602_id[] = {
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 12/17] media: netup_unidvb: Don't leak SPI master in probe error path
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
                   ` (10 preceding siblings ...)
  2020-12-07  8:17 ` [PATCH 11/17] spi: sc18is602: " Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07  8:17 ` [PATCH 13/17] spi: mt7621: Disable clock " Lukas Wunner
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi

If the call to spi_register_master() fails on probe of the NetUP
Universal DVB driver, the spi_master struct is erroneously not freed.

Likewise, if spi_new_device() fails, the spi_controller struct is
not unregistered.  Plug the leaks.

While at it, fix an ordering issue in netup_spi_release() wherein
spi_unregister_master() is called after fiddling with the IRQ control
register.  The correct order is to call spi_unregister_master() *before*
this teardown step because bus accesses may still be ongoing until that
function returns.

Fixes: 52b1eaf4c59a ("[media] netup_unidvb: NetUP Universal DVB-S/S2/T/T2/C PCI-E card driver")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Reviewed-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc: <stable@vger.kernel.org> # v4.3+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v4.3+
Cc: Kozlov Sergey <serjk@netup.ru>
---
 drivers/media/pci/netup_unidvb/netup_unidvb_spi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/netup_unidvb/netup_unidvb_spi.c b/drivers/media/pci/netup_unidvb/netup_unidvb_spi.c
index d4f12c250f91..526042d8afae 100644
--- a/drivers/media/pci/netup_unidvb/netup_unidvb_spi.c
+++ b/drivers/media/pci/netup_unidvb/netup_unidvb_spi.c
@@ -175,7 +175,7 @@ int netup_spi_init(struct netup_unidvb_dev *ndev)
 	struct spi_master *master;
 	struct netup_spi *nspi;
 
-	master = spi_alloc_master(&ndev->pci_dev->dev,
+	master = devm_spi_alloc_master(&ndev->pci_dev->dev,
 		sizeof(struct netup_spi));
 	if (!master) {
 		dev_err(&ndev->pci_dev->dev,
@@ -208,6 +208,7 @@ int netup_spi_init(struct netup_unidvb_dev *ndev)
 		ndev->pci_slot,
 		ndev->pci_func);
 	if (!spi_new_device(master, &netup_spi_board)) {
+		spi_unregister_master(master);
 		ndev->spi = NULL;
 		dev_err(&ndev->pci_dev->dev,
 			"%s(): unable to create SPI device\n", __func__);
@@ -226,13 +227,13 @@ void netup_spi_release(struct netup_unidvb_dev *ndev)
 	if (!spi)
 		return;
 
+	spi_unregister_master(spi->master);
 	spin_lock_irqsave(&spi->lock, flags);
 	reg = readw(&spi->regs->control_stat);
 	writew(reg | NETUP_SPI_CTRL_IRQ, &spi->regs->control_stat);
 	reg = readw(&spi->regs->control_stat);
 	writew(reg & ~NETUP_SPI_CTRL_IMASK, &spi->regs->control_stat);
 	spin_unlock_irqrestore(&spi->lock, flags);
-	spi_unregister_master(spi->master);
 	ndev->spi = NULL;
 }
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 13/17] spi: mt7621: Disable clock in probe error path
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
                   ` (11 preceding siblings ...)
  2020-12-07  8:17 ` [PATCH 12/17] media: netup_unidvb: " Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07  8:17 ` [PATCH 14/17] spi: mt7621: Don't leak SPI master " Lukas Wunner
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi, Qinglang Miao

Commit 702b15cb9712 ("spi: mt7621: fix missing clk_disable_unprepare()
on error in mt7621_spi_probe") sought to disable the SYS clock on probe
errors, but only did so for 2 of 3 potentially failing calls:  The clock
needs to be disabled on failure of devm_spi_register_controller() as
well.

Moreover, the commit purports to fix a bug in commit cbd66c626e16 ("spi:
mt7621: Move SPI driver out of staging") but in reality the bug has
existed since the driver was first introduced.

Fixes: 1ab7f2a43558 ("staging: mt7621-spi: add mt7621 support")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v4.17+: 702b15cb9712: spi: mt7621: fix missing clk_disable_unprepare() on error in mt7621_spi_probe
Cc: <stable@vger.kernel.org> # v4.17+
Cc: Qinglang Miao <miaoqinglang@huawei.com>
---
 drivers/spi/spi-mt7621.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-mt7621.c b/drivers/spi/spi-mt7621.c
index 2cdae7994e2a..e227700808cb 100644
--- a/drivers/spi/spi-mt7621.c
+++ b/drivers/spi/spi-mt7621.c
@@ -382,7 +382,11 @@ static int mt7621_spi_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	return devm_spi_register_controller(&pdev->dev, master);
+	ret = devm_spi_register_controller(&pdev->dev, master);
+	if (ret)
+		clk_disable_unprepare(clk);
+
+	return ret;
 }
 
 static int mt7621_spi_remove(struct platform_device *pdev)
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 14/17] spi: mt7621: Don't leak SPI master in probe error path
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
                   ` (12 preceding siblings ...)
  2020-12-07  8:17 ` [PATCH 13/17] spi: mt7621: Disable clock " Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07  8:17 ` [PATCH 15/17] spi: ar934x: " Lukas Wunner
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi

If the calls to device_reset() or devm_spi_register_controller() fail on
probe of the MediaTek MT7621 SPI driver, the spi_controller struct is
erroneously not freed.  Fix by switching over to the new
devm_spi_alloc_master() helper.

Additionally, there's an ordering issue in mt7621_spi_remove() wherein
the spi_controller is unregistered after disabling the SYS clock.
The correct order is to call spi_unregister_controller() *before* this
teardown step because bus accesses may still be ongoing until that
function returns.

All of these bugs have existed since the driver was first introduced,
so it seems fair to fix them together in a single commit.

Fixes: 1ab7f2a43558 ("staging: mt7621-spi: add mt7621 support")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Reviewed-by: Stefan Roese <sr@denx.de>
Cc: <stable@vger.kernel.org> # v4.17+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v4.17+
---
 drivers/spi/spi-mt7621.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-mt7621.c b/drivers/spi/spi-mt7621.c
index e227700808cb..b4b9b7309b5e 100644
--- a/drivers/spi/spi-mt7621.c
+++ b/drivers/spi/spi-mt7621.c
@@ -350,7 +350,7 @@ static int mt7621_spi_probe(struct platform_device *pdev)
 	if (status)
 		return status;
 
-	master = spi_alloc_master(&pdev->dev, sizeof(*rs));
+	master = devm_spi_alloc_master(&pdev->dev, sizeof(*rs));
 	if (!master) {
 		dev_info(&pdev->dev, "master allocation failed\n");
 		clk_disable_unprepare(clk);
@@ -382,7 +382,7 @@ static int mt7621_spi_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = devm_spi_register_controller(&pdev->dev, master);
+	ret = spi_register_controller(master);
 	if (ret)
 		clk_disable_unprepare(clk);
 
@@ -397,6 +397,7 @@ static int mt7621_spi_remove(struct platform_device *pdev)
 	master = dev_get_drvdata(&pdev->dev);
 	rs = spi_controller_get_devdata(master);
 
+	spi_unregister_controller(master);
 	clk_disable_unprepare(rs->clk);
 
 	return 0;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 15/17] spi: ar934x: Don't leak SPI master in probe error path
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
                   ` (13 preceding siblings ...)
  2020-12-07  8:17 ` [PATCH 14/17] spi: mt7621: Don't leak SPI master " Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07  8:17 ` [PATCH 16/17] spi: npcm-fiu: Disable clock " Lukas Wunner
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi, Chuanhong Guo

If the call to devm_spi_register_controller() fails on probe of the
Qualcomm Atheros AR934x/QCA95xx SPI driver, the spi_controller struct is
erroneously not freed.  Fix by switching over to the new
devm_spi_alloc_master() helper.

Moreover, the controller's clock is enabled on probe but not disabled if
any of the subsequent probe steps fail.

Finally, there's an ordering issue in ar934x_spi_remove() wherein the
clock is disabled even though the controller is not yet unregistered.
It is unregistered after ar934x_spi_remove() by the devres framework.
As long as it is not unregistered, SPI transfers may still be ongoing
and disabling the clock may break them.  It is not possible to use
devm_spi_register_controller() in this case, so move to the non-devm
variant.

All of these bugs have existed since the driver was first introduced,
so it seems fair to fix them together in a single commit.

Fixes: 047980c582af ("spi: add driver for ar934x spi controller")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v5.7+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v5.7+
Cc: Chuanhong Guo <gch981213@gmail.com>
---
 drivers/spi/spi-ar934x.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-ar934x.c b/drivers/spi/spi-ar934x.c
index d08dec09d423..def32e0aaefe 100644
--- a/drivers/spi/spi-ar934x.c
+++ b/drivers/spi/spi-ar934x.c
@@ -176,10 +176,11 @@ static int ar934x_spi_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	ctlr = spi_alloc_master(&pdev->dev, sizeof(*sp));
+	ctlr = devm_spi_alloc_master(&pdev->dev, sizeof(*sp));
 	if (!ctlr) {
 		dev_info(&pdev->dev, "failed to allocate spi controller\n");
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto err_clk_disable;
 	}
 
 	/* disable flash mapping and expose spi controller registers */
@@ -202,7 +203,13 @@ static int ar934x_spi_probe(struct platform_device *pdev)
 	sp->clk_freq = clk_get_rate(clk);
 	sp->ctlr = ctlr;
 
-	return devm_spi_register_controller(&pdev->dev, ctlr);
+	ret = spi_register_controller(ctlr);
+	if (!ret)
+		return 0;
+
+err_clk_disable:
+	clk_disable_unprepare(clk);
+	return ret;
 }
 
 static int ar934x_spi_remove(struct platform_device *pdev)
@@ -213,6 +220,7 @@ static int ar934x_spi_remove(struct platform_device *pdev)
 	ctlr = dev_get_drvdata(&pdev->dev);
 	sp = spi_controller_get_devdata(ctlr);
 
+	spi_unregister_controller(ctlr);
 	clk_disable_unprepare(sp->clk);
 
 	return 0;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 16/17] spi: npcm-fiu: Disable clock in probe error path
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
                   ` (14 preceding siblings ...)
  2020-12-07  8:17 ` [PATCH 15/17] spi: ar934x: " Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07  8:17 ` [PATCH 17/17] spi: atmel-quadspi: Fix use-after-free on unbind Lukas Wunner
  2020-12-07 17:22 ` [PATCH 00/17] SPI probe/remove sanitization for 5.11 Mark Brown
  17 siblings, 0 replies; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi, Tomer Maimon

If the call to devm_spi_register_master() fails on probe of the NPCM FIU
SPI driver, the clock "fiu->clk" is erroneously not unprepared and
disabled.  Fix it.

Fixes: ace55c411b11 ("spi: npcm-fiu: add NPCM FIU controller driver")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v5.4+
Cc: Tomer Maimon <tmaimon77@gmail.com>
---
 drivers/spi/spi-npcm-fiu.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-npcm-fiu.c b/drivers/spi/spi-npcm-fiu.c
index 1cb9329de945..b62471ab6d7f 100644
--- a/drivers/spi/spi-npcm-fiu.c
+++ b/drivers/spi/spi-npcm-fiu.c
@@ -677,7 +677,7 @@ static int npcm_fiu_probe(struct platform_device *pdev)
 	struct npcm_fiu_spi *fiu;
 	void __iomem *regbase;
 	struct resource *res;
-	int id;
+	int id, ret;
 
 	ctrl = devm_spi_alloc_master(dev, sizeof(*fiu));
 	if (!ctrl)
@@ -735,7 +735,11 @@ static int npcm_fiu_probe(struct platform_device *pdev)
 	ctrl->num_chipselect = fiu->info->max_cs;
 	ctrl->dev.of_node = dev->of_node;
 
-	return devm_spi_register_master(dev, ctrl);
+	ret = devm_spi_register_master(dev, ctrl);
+	if (ret)
+		clk_disable_unprepare(fiu->clk);
+
+	return ret;
 }
 
 static int npcm_fiu_remove(struct platform_device *pdev)
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 17/17] spi: atmel-quadspi: Fix use-after-free on unbind
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
                   ` (15 preceding siblings ...)
  2020-12-07  8:17 ` [PATCH 16/17] spi: npcm-fiu: Disable clock " Lukas Wunner
@ 2020-12-07  8:17 ` Lukas Wunner
  2020-12-07 15:03   ` Tudor.Ambarus
  2020-12-07 17:22 ` [PATCH 00/17] SPI probe/remove sanitization for 5.11 Mark Brown
  17 siblings, 1 reply; 20+ messages in thread
From: Lukas Wunner @ 2020-12-07  8:17 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi, Piotr Bugalski, Tudor Ambarus

atmel_qspi_remove() accesses the driver's private data after calling
spi_unregister_controller() even though that function releases the last
reference on the spi_controller 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: 2d30ac5ed633 ("mtd: spi-nor: atmel-quadspi: Use spi-mem interface for atmel-quadspi driver")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: <stable@vger.kernel.org> # v5.0+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
Cc: <stable@vger.kernel.org> # v5.0+
Cc: Piotr Bugalski <bugalski.piotr@gmail.com>
---
 drivers/spi/atmel-quadspi.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
index b44521d4a245..1742ccc1ad0e 100644
--- a/drivers/spi/atmel-quadspi.c
+++ b/drivers/spi/atmel-quadspi.c
@@ -535,7 +535,7 @@ static int atmel_qspi_probe(struct platform_device *pdev)
 	struct resource *res;
 	int irq, err = 0;
 
-	ctrl = spi_alloc_master(&pdev->dev, sizeof(*aq));
+	ctrl = devm_spi_alloc_master(&pdev->dev, sizeof(*aq));
 	if (!ctrl)
 		return -ENOMEM;
 
@@ -557,8 +557,7 @@ static int atmel_qspi_probe(struct platform_device *pdev)
 	aq->regs = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(aq->regs)) {
 		dev_err(&pdev->dev, "missing registers\n");
-		err = PTR_ERR(aq->regs);
-		goto exit;
+		return PTR_ERR(aq->regs);
 	}
 
 	/* Map the AHB memory */
@@ -566,8 +565,7 @@ static int atmel_qspi_probe(struct platform_device *pdev)
 	aq->mem = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(aq->mem)) {
 		dev_err(&pdev->dev, "missing AHB memory\n");
-		err = PTR_ERR(aq->mem);
-		goto exit;
+		return PTR_ERR(aq->mem);
 	}
 
 	aq->mmap_size = resource_size(res);
@@ -579,15 +577,14 @@ static int atmel_qspi_probe(struct platform_device *pdev)
 
 	if (IS_ERR(aq->pclk)) {
 		dev_err(&pdev->dev, "missing peripheral clock\n");
-		err = PTR_ERR(aq->pclk);
-		goto exit;
+		return PTR_ERR(aq->pclk);
 	}
 
 	/* Enable the peripheral clock */
 	err = clk_prepare_enable(aq->pclk);
 	if (err) {
 		dev_err(&pdev->dev, "failed to enable the peripheral clock\n");
-		goto exit;
+		return err;
 	}
 
 	aq->caps = of_device_get_match_data(&pdev->dev);
@@ -638,8 +635,6 @@ static int atmel_qspi_probe(struct platform_device *pdev)
 	clk_disable_unprepare(aq->qspick);
 disable_pclk:
 	clk_disable_unprepare(aq->pclk);
-exit:
-	spi_controller_put(ctrl);
 
 	return err;
 }
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 17/17] spi: atmel-quadspi: Fix use-after-free on unbind
  2020-12-07  8:17 ` [PATCH 17/17] spi: atmel-quadspi: Fix use-after-free on unbind Lukas Wunner
@ 2020-12-07 15:03   ` Tudor.Ambarus
  0 siblings, 0 replies; 20+ messages in thread
From: Tudor.Ambarus @ 2020-12-07 15:03 UTC (permalink / raw)
  To: lukas, broonie; +Cc: linux-spi, bugalski.piotr

On 12/7/20 10:17 AM, Lukas Wunner wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> atmel_qspi_remove() accesses the driver's private data after calling
> spi_unregister_controller() even though that function releases the last
> reference on the spi_controller 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: 2d30ac5ed633 ("mtd: spi-nor: atmel-quadspi: Use spi-mem interface for atmel-quadspi driver")
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Cc: <stable@vger.kernel.org> # v5.0+: 5e844cc37a5c: spi: Introduce device-managed SPI controller allocation
> Cc: <stable@vger.kernel.org> # v5.0+
> Cc: Piotr Bugalski <bugalski.piotr@gmail.com>

Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

> ---
>  drivers/spi/atmel-quadspi.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
> index b44521d4a245..1742ccc1ad0e 100644
> --- a/drivers/spi/atmel-quadspi.c
> +++ b/drivers/spi/atmel-quadspi.c
> @@ -535,7 +535,7 @@ static int atmel_qspi_probe(struct platform_device *pdev)
>         struct resource *res;
>         int irq, err = 0;
> 
> -       ctrl = spi_alloc_master(&pdev->dev, sizeof(*aq));
> +       ctrl = devm_spi_alloc_master(&pdev->dev, sizeof(*aq));
>         if (!ctrl)
>                 return -ENOMEM;
> 
> @@ -557,8 +557,7 @@ static int atmel_qspi_probe(struct platform_device *pdev)
>         aq->regs = devm_ioremap_resource(&pdev->dev, res);
>         if (IS_ERR(aq->regs)) {
>                 dev_err(&pdev->dev, "missing registers\n");
> -               err = PTR_ERR(aq->regs);
> -               goto exit;
> +               return PTR_ERR(aq->regs);
>         }
> 
>         /* Map the AHB memory */
> @@ -566,8 +565,7 @@ static int atmel_qspi_probe(struct platform_device *pdev)
>         aq->mem = devm_ioremap_resource(&pdev->dev, res);
>         if (IS_ERR(aq->mem)) {
>                 dev_err(&pdev->dev, "missing AHB memory\n");
> -               err = PTR_ERR(aq->mem);
> -               goto exit;
> +               return PTR_ERR(aq->mem);
>         }
> 
>         aq->mmap_size = resource_size(res);
> @@ -579,15 +577,14 @@ static int atmel_qspi_probe(struct platform_device *pdev)
> 
>         if (IS_ERR(aq->pclk)) {
>                 dev_err(&pdev->dev, "missing peripheral clock\n");
> -               err = PTR_ERR(aq->pclk);
> -               goto exit;
> +               return PTR_ERR(aq->pclk);
>         }
> 
>         /* Enable the peripheral clock */
>         err = clk_prepare_enable(aq->pclk);
>         if (err) {
>                 dev_err(&pdev->dev, "failed to enable the peripheral clock\n");
> -               goto exit;
> +               return err;
>         }
> 
>         aq->caps = of_device_get_match_data(&pdev->dev);
> @@ -638,8 +635,6 @@ static int atmel_qspi_probe(struct platform_device *pdev)
>         clk_disable_unprepare(aq->qspick);
>  disable_pclk:
>         clk_disable_unprepare(aq->pclk);
> -exit:
> -       spi_controller_put(ctrl);
> 
>         return err;
>  }
> --
> 2.29.2
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 00/17] SPI probe/remove sanitization for 5.11
  2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
                   ` (16 preceding siblings ...)
  2020-12-07  8:17 ` [PATCH 17/17] spi: atmel-quadspi: Fix use-after-free on unbind Lukas Wunner
@ 2020-12-07 17:22 ` Mark Brown
  17 siblings, 0 replies; 20+ messages in thread
From: Mark Brown @ 2020-12-07 17:22 UTC (permalink / raw)
  To: Lukas Wunner
  Cc: Tudor Ambarus, Piotr Bugalski, Chuanhong Guo, linux-spi, Tomer Maimon

On Mon, 7 Dec 2020 09:17:00 +0100, Lukas Wunner wrote:
> Patches [01/17] to [14/17] are reposts, now based on spi.git/for-5.11
> and amended with all collected Acked-by + Reviewed-by tags.
> 
> Links to their original submission:
> https://lore.kernel.org/linux-spi/dd060534490eca5e946eb9165916542b01a9358d.1604874488.git.lukas@wunner.de/
> https://lore.kernel.org/linux-spi/73adc6ba84a4f968f2e1499a776e5c928fbdde56.1605512876.git.lukas@wunner.de/T/#t
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[01/17] spi: davinci: Fix use-after-free on unbind
        commit: 373afef350a93519b4b8d636b0895da8650b714b
[02/17] spi: spi-geni-qcom: Fix use-after-free on unbind
        commit: 8f96c434dfbc85ffa755d6634c8c1cb2233fcf24
[03/17] spi: spi-qcom-qspi: Fix use-after-free on unbind
        commit: 6cfd39e212dee2e77a0227ce4e0f55fa06d79f46
[04/17] spi: spi-sh: Fix use-after-free on unbind
        commit: e77df3eca12be4b17f13cf9f215cff248c57d98f
[05/17] spi: pxa2xx: Fix use-after-free on unbind
        commit: 5626308bb94d9f930aa5f7c77327df4c6daa7759
[06/17] spi: rpc-if: Fix use-after-free on unbind
        commit: 393f981ca5f797b58b882d42b7621fb6e43c7f5b
[07/17] spi: mxic: Don't leak SPI master in probe error path
        commit: cc53711b2191cf3b3210283ae89bf0abb98c70a3
[08/17] spi: spi-mtk-nor: Don't leak SPI master in probe error path
        commit: 0f4ad8d59f33b24dd86739f3be23e6af1a86f5a9
[09/17] spi: gpio: Don't leak SPI master in probe error path
        commit: 7174dc655ef0578877b0b4598e69619d2be28b4d
[10/17] spi: rb4xx: Don't leak SPI master in probe error path
        commit: a4729c3506c3eb1a6ca5c0289f4e7cafa4115065
[11/17] spi: sc18is602: Don't leak SPI master in probe error path
        commit: 5b8c88462d83331dacb48aeaec8388117fef82e0
[12/17] media: netup_unidvb: Don't leak SPI master in probe error path
        commit: e297ddf296de35037fa97f4302782def196d350a
[13/17] spi: mt7621: Disable clock in probe error path
        commit: 24f7033405abe195224ec793dbc3d7a27dec0b98
[14/17] spi: mt7621: Don't leak SPI master in probe error path
        commit: 46b5c4fb87ce8211e0f9b0383dbde72c3652d2ba
[15/17] spi: ar934x: Don't leak SPI master in probe error path
        commit: 236924ee531d6251c8d10e9177b7742a60534ed5
[16/17] spi: npcm-fiu: Disable clock in probe error path
        commit: 234266a5168bbe8220d263e3aa7aa80cf921c483
[17/17] spi: atmel-quadspi: Fix use-after-free on unbind
        commit: c7b884561cb5b641f3dbba950094110794119a6d

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-12-07 17:23 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-07  8:17 [PATCH 00/17] SPI probe/remove sanitization for 5.11 Lukas Wunner
2020-12-07  8:17 ` [PATCH 01/17] spi: davinci: Fix use-after-free on unbind Lukas Wunner
2020-12-07  8:17 ` [PATCH 02/17] spi: spi-geni-qcom: " Lukas Wunner
2020-12-07  8:17 ` [PATCH 03/17] spi: spi-qcom-qspi: " Lukas Wunner
2020-12-07  8:17 ` [PATCH 04/17] spi: spi-sh: " Lukas Wunner
2020-12-07  8:17 ` [PATCH 05/17] spi: pxa2xx: " Lukas Wunner
2020-12-07  8:17 ` [PATCH 06/17] spi: rpc-if: " Lukas Wunner
2020-12-07  8:17 ` [PATCH 07/17] spi: mxic: Don't leak SPI master in probe error path Lukas Wunner
2020-12-07  8:17 ` [PATCH 08/17] spi: spi-mtk-nor: " Lukas Wunner
2020-12-07  8:17 ` [PATCH 09/17] spi: gpio: " Lukas Wunner
2020-12-07  8:17 ` [PATCH 10/17] spi: rb4xx: " Lukas Wunner
2020-12-07  8:17 ` [PATCH 11/17] spi: sc18is602: " Lukas Wunner
2020-12-07  8:17 ` [PATCH 12/17] media: netup_unidvb: " Lukas Wunner
2020-12-07  8:17 ` [PATCH 13/17] spi: mt7621: Disable clock " Lukas Wunner
2020-12-07  8:17 ` [PATCH 14/17] spi: mt7621: Don't leak SPI master " Lukas Wunner
2020-12-07  8:17 ` [PATCH 15/17] spi: ar934x: " Lukas Wunner
2020-12-07  8:17 ` [PATCH 16/17] spi: npcm-fiu: Disable clock " Lukas Wunner
2020-12-07  8:17 ` [PATCH 17/17] spi: atmel-quadspi: Fix use-after-free on unbind Lukas Wunner
2020-12-07 15:03   ` Tudor.Ambarus
2020-12-07 17:22 ` [PATCH 00/17] SPI probe/remove sanitization for 5.11 Mark Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.