diff mbox series

[-next] spi: cadence-quadspi: Remove spi_master_put() in probe failure path

Message ID 20220601071611.11853-1-vaishnav.a@ti.com (mailing list archive)
State Accepted
Commit 73d5fe046270281a46344e06bf986c607632f7ea
Headers show
Series [-next] spi: cadence-quadspi: Remove spi_master_put() in probe failure path | expand

Commit Message

Vaishnav Achath June 1, 2022, 7:16 a.m. UTC
Currently the spi_master is allocated by devm_spi_alloc_master()
and devres core manages the deallocation, but in probe failure
path spi_master_put() is being handled manually which causes
"refcount underflow use-after-free" warning when probe failure happens
after allocating spi_master.

Trimmed backtrace during failure:

refcount_t: underflow; use-after-free.
pc : refcount_warn_saturate+0xf4/0x144
Call trace:
refcount_warn_saturate
kobject_put
put_device
devm_spi_release_controller
devres_release_all

This commit makes relevant changes to remove spi_master_put() from probe
failure path.

Fixes: 606e5d408184 ("spi: cadence-quadspi: Handle spi_unregister_master() in remove()")

Signed-off-by: Vaishnav Achath <vaishnav.a@ti.com>
---
 drivers/spi/spi-cadence-quadspi.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

Comments

Mark Brown June 7, 2022, 10:46 a.m. UTC | #1
On Wed, 1 Jun 2022 12:46:11 +0530, Vaishnav Achath wrote:
> Currently the spi_master is allocated by devm_spi_alloc_master()
> and devres core manages the deallocation, but in probe failure
> path spi_master_put() is being handled manually which causes
> "refcount underflow use-after-free" warning when probe failure happens
> after allocating spi_master.
> 
> Trimmed backtrace during failure:
> 
> [...]

Applied to

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

Thanks!

[1/1] spi: cadence-quadspi: Remove spi_master_put() in probe failure path
      commit: 8523c96894e916b20ba3612e48e404fad5acfdd9

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
Pratyush Yadav July 14, 2022, 11:07 a.m. UTC | #2
Hi Mark,

On 07/06/22 11:46AM, Mark Brown wrote:
> On Wed, 1 Jun 2022 12:46:11 +0530, Vaishnav Achath wrote:
> > Currently the spi_master is allocated by devm_spi_alloc_master()
> > and devres core manages the deallocation, but in probe failure
> > path spi_master_put() is being handled manually which causes
> > "refcount underflow use-after-free" warning when probe failure happens
> > after allocating spi_master.
> > 
> > Trimmed backtrace during failure:
> > 
> > [...]
> 
> Applied to
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

I see this error in v5.19-rc6 as well. Can we get this patch merged as a 
fix in rc7? Sorry for spotting this so late in the cycle, but I thought 
you had already got this merged in one of the rcs.

> 
> Thanks!
> 
> [1/1] spi: cadence-quadspi: Remove spi_master_put() in probe failure path
>       commit: 8523c96894e916b20ba3612e48e404fad5acfdd9
> 
> 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
Mark Brown July 14, 2022, 1:57 p.m. UTC | #3
On Wed, 1 Jun 2022 12:46:11 +0530, Vaishnav Achath wrote:
> Currently the spi_master is allocated by devm_spi_alloc_master()
> and devres core manages the deallocation, but in probe failure
> path spi_master_put() is being handled manually which causes
> "refcount underflow use-after-free" warning when probe failure happens
> after allocating spi_master.
> 
> Trimmed backtrace during failure:
> 
> [...]

Applied to

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

Thanks!

[1/1] spi: cadence-quadspi: Remove spi_master_put() in probe failure path
      commit: 73d5fe046270281a46344e06bf986c607632f7ea

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
diff mbox series

Patch

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 2b9fc8449a62..72b1a5a2298c 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1578,8 +1578,7 @@  static int cqspi_probe(struct platform_device *pdev)
 	ret = cqspi_of_get_pdata(cqspi);
 	if (ret) {
 		dev_err(dev, "Cannot get mandatory OF data.\n");
-		ret = -ENODEV;
-		goto probe_master_put;
+		return -ENODEV;
 	}
 
 	/* Obtain QSPI clock. */
@@ -1587,7 +1586,7 @@  static int cqspi_probe(struct platform_device *pdev)
 	if (IS_ERR(cqspi->clk)) {
 		dev_err(dev, "Cannot claim QSPI clock.\n");
 		ret = PTR_ERR(cqspi->clk);
-		goto probe_master_put;
+		return ret;
 	}
 
 	/* Obtain and remap controller address. */
@@ -1596,7 +1595,7 @@  static int cqspi_probe(struct platform_device *pdev)
 	if (IS_ERR(cqspi->iobase)) {
 		dev_err(dev, "Cannot remap controller address.\n");
 		ret = PTR_ERR(cqspi->iobase);
-		goto probe_master_put;
+		return ret;
 	}
 
 	/* Obtain and remap AHB address. */
@@ -1605,7 +1604,7 @@  static int cqspi_probe(struct platform_device *pdev)
 	if (IS_ERR(cqspi->ahb_base)) {
 		dev_err(dev, "Cannot remap AHB address.\n");
 		ret = PTR_ERR(cqspi->ahb_base);
-		goto probe_master_put;
+		return ret;
 	}
 	cqspi->mmap_phys_base = (dma_addr_t)res_ahb->start;
 	cqspi->ahb_size = resource_size(res_ahb);
@@ -1614,15 +1613,13 @@  static int cqspi_probe(struct platform_device *pdev)
 
 	/* Obtain IRQ line. */
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		ret = -ENXIO;
-		goto probe_master_put;
-	}
+	if (irq < 0)
+		return -ENXIO;
 
 	pm_runtime_enable(dev);
 	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
-		goto probe_master_put;
+		return ret;
 
 	ret = clk_prepare_enable(cqspi->clk);
 	if (ret) {
@@ -1716,8 +1713,6 @@  static int cqspi_probe(struct platform_device *pdev)
 probe_clk_failed:
 	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);
-probe_master_put:
-	spi_master_put(master);
 	return ret;
 }