[PATCH -next v2 0/3] spi: microchip-core: fix and cleanups

[PATCH -next v2 0/3] spi: microchip-core: fix and cleanups

[Yang Yingliang]

Patch #1 fix a UAF in mchp_corespi_remove().
Patch #2 and #3 some cleanups to simpify code.

v1 -> v2:
  add patch #3 to use dev_err_probe to simpify code.

Yang Yingliang (3):
  spi: microchip-core: fix UAF in mchp_corespi_remove()
  spi: microchip-core: switch to use devm_spi_alloc_master()
  spi: microchip-core: switch to use dev_err_probe()

 drivers/spi/spi-microchip-core.c | 56 ++++++++++++--------------------
 1 file changed, 20 insertions(+), 36 deletions(-)

-- 
2.25.1

[PATCH -next v2 1/3] spi: microchip-core: fix UAF in mchp_corespi_remove()

[Yang Yingliang]

When using devm_spi_register_master(), the unregister function will
be called in devres_release_all() which is called after ->remove(),
so remove spi_unregister_master() andspi_master_put().

Fixes: 9ac8d17694b6 ("spi: add support for microchip fpga spi controllers")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/spi/spi-microchip-core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/spi/spi-microchip-core.c b/drivers/spi/spi-microchip-core.c
index b3083075cd36..c26767343176 100644
--- a/drivers/spi/spi-microchip-core.c
+++ b/drivers/spi/spi-microchip-core.c
@@ -595,8 +595,6 @@ static int mchp_corespi_remove(struct platform_device *pdev)
 	struct mchp_corespi *spi = spi_master_get_devdata(master);
 
 	mchp_corespi_disable_ints(spi);
-	spi_unregister_master(master);
-	spi_master_put(master);
 	clk_disable_unprepare(spi->clk);
 	mchp_corespi_disable(spi);
 
-- 
2.25.1

[PATCH -next v2 2/3] spi: microchip-core: switch to use devm_spi_alloc_master()

[Yang Yingliang]

Switch to use devm_spi_alloc_master() to simpify error path.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/spi/spi-microchip-core.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/spi/spi-microchip-core.c b/drivers/spi/spi-microchip-core.c
index c26767343176..1a24e47f8305 100644
--- a/drivers/spi/spi-microchip-core.c
+++ b/drivers/spi/spi-microchip-core.c
@@ -513,7 +513,7 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 	u32 num_cs;
 	int ret = 0;
 
-	master = spi_alloc_master(&pdev->dev, sizeof(*spi));
+	master = devm_spi_alloc_master(&pdev->dev, sizeof(*spi));
 	if (!master)
 		return dev_err_probe(&pdev->dev, -ENOMEM,
 				     "unable to allocate master for SPI controller\n");
@@ -535,36 +535,32 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 	spi = spi_master_get_devdata(master);
 
 	spi->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
-	if (IS_ERR(spi->regs)) {
-		ret = PTR_ERR(spi->regs);
-		goto error_release_master;
-	}
+	if (IS_ERR(spi->regs))
+		return PTR_ERR(spi->regs);
 
 	spi->irq = platform_get_irq(pdev, 0);
 	if (spi->irq <= 0) {
 		dev_err(&pdev->dev, "invalid IRQ %d for SPI controller\n", spi->irq);
-		ret = -ENXIO;
-		goto error_release_master;
+		return -ENXIO;
 	}
 
 	ret = devm_request_irq(&pdev->dev, spi->irq, mchp_corespi_interrupt,
 			       IRQF_SHARED, dev_name(&pdev->dev), master);
 	if (ret) {
 		dev_err(&pdev->dev, "could not request irq: %d\n", ret);
-		goto error_release_master;
+		return ret;
 	}
 
 	spi->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(spi->clk)) {
-		ret = PTR_ERR(spi->clk);
 		dev_err(&pdev->dev, "could not get clk: %d\n", ret);
-		goto error_release_master;
+		return PTR_ERR(spi->clk);
 	}
 
 	ret = clk_prepare_enable(spi->clk);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to enable clock\n");
-		goto error_release_master;
+		return ret;
 	}
 
 	mchp_corespi_init(master, spi);
@@ -583,8 +579,6 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 error_release_hardware:
 	mchp_corespi_disable(spi);
 	clk_disable_unprepare(spi->clk);
-error_release_master:
-	spi_master_put(master);
 
 	return ret;
 }
-- 
2.25.1

[PATCH -next v2 3/3] spi: microchip-core: switch to use dev_err_probe()

[Yang Yingliang]

Switch to use dev_err_probe() to simpify error path.

Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/spi/spi-microchip-core.c | 42 +++++++++++++-------------------
 1 file changed, 17 insertions(+), 25 deletions(-)

diff --git a/drivers/spi/spi-microchip-core.c b/drivers/spi/spi-microchip-core.c
index 1a24e47f8305..ce4385330b19 100644
--- a/drivers/spi/spi-microchip-core.c
+++ b/drivers/spi/spi-microchip-core.c
@@ -539,48 +539,40 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 		return PTR_ERR(spi->regs);
 
 	spi->irq = platform_get_irq(pdev, 0);
-	if (spi->irq <= 0) {
-		dev_err(&pdev->dev, "invalid IRQ %d for SPI controller\n", spi->irq);
-		return -ENXIO;
-	}
+	if (spi->irq <= 0)
+		return dev_err_probe(&pdev->dev, -ENXIO,
+				     "invalid IRQ %d for SPI controller\n",
+				     spi->irq);
 
 	ret = devm_request_irq(&pdev->dev, spi->irq, mchp_corespi_interrupt,
 			       IRQF_SHARED, dev_name(&pdev->dev), master);
-	if (ret) {
-		dev_err(&pdev->dev, "could not request irq: %d\n", ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "could not request irq: %d\n", ret);
 
 	spi->clk = devm_clk_get(&pdev->dev, NULL);
-	if (IS_ERR(spi->clk)) {
-		dev_err(&pdev->dev, "could not get clk: %d\n", ret);
-		return PTR_ERR(spi->clk);
-	}
+	if (IS_ERR(spi->clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(spi->clk),
+				     "could not get clk: %d\n", ret);
 
 	ret = clk_prepare_enable(spi->clk);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to enable clock\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "failed to enable clock\n");
 
 	mchp_corespi_init(master, spi);
 
 	ret = devm_spi_register_master(&pdev->dev, master);
 	if (ret) {
-		dev_err(&pdev->dev,
-			"unable to register master for SPI controller\n");
-		goto error_release_hardware;
+		mchp_corespi_disable(spi);
+		clk_disable_unprepare(spi->clk);
+		return dev_err_probe(&pdev->dev, ret,
+				     "unable to register master for SPI controller\n");
 	}
 
 	dev_info(&pdev->dev, "Registered SPI controller %d\n", master->bus_num);
 
 	return 0;
-
-error_release_hardware:
-	mchp_corespi_disable(spi);
-	clk_disable_unprepare(spi->clk);
-
-	return ret;
 }
 
 static int mchp_corespi_remove(struct platform_device *pdev)
-- 
2.25.1

Re: [PATCH -next v2 0/3] spi: microchip-core: fix and cleanups

[Conor.Dooley]

On 13/07/2022 03:56, Yang Yingliang wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Patch #1 fix a UAF in mchp_corespi_remove().
> Patch #2 and #3 some cleanups to simpify code.

P sure I already gave you an R-b for patch 1, but for the series:

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

> 
> v1 -> v2:
>    add patch #3 to use dev_err_probe to simpify code.
> 
> Yang Yingliang (3):
>    spi: microchip-core: fix UAF in mchp_corespi_remove()
>    spi: microchip-core: switch to use devm_spi_alloc_master()
>    spi: microchip-core: switch to use dev_err_probe()
> 
>   drivers/spi/spi-microchip-core.c | 56 ++++++++++++--------------------
>   1 file changed, 20 insertions(+), 36 deletions(-)
> 
> --
> 2.25.1
> 

Re: [PATCH -next v2 0/3] spi: microchip-core: fix and cleanups

[Mark Brown]

On Wed, 13 Jul 2022 10:56:54 +0800, Yang Yingliang wrote:
> Patch #1 fix a UAF in mchp_corespi_remove().
> Patch #2 and #3 some cleanups to simpify code.
> 
> v1 -> v2:
>   add patch #3 to use dev_err_probe to simpify code.
> 
> Yang Yingliang (3):
>   spi: microchip-core: fix UAF in mchp_corespi_remove()
>   spi: microchip-core: switch to use devm_spi_alloc_master()
>   spi: microchip-core: switch to use dev_err_probe()
> 
> [...]

Applied to

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

Thanks!

[1/3] spi: microchip-core: fix UAF in mchp_corespi_remove()
      commit: e82c6d62a1f2347cde69c169fcf37e4d26f89b98
[2/3] spi: microchip-core: switch to use devm_spi_alloc_master()
      commit: 5d56d8974d1e5fa5d7d0761037377e03f6edcc66
[3/3] spi: microchip-core: switch to use dev_err_probe()
      commit: cdeaf3a99a02b6f8566bcaacc9c3501c6cceda74

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