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 <[email protected]>
---
drivers/spi/spi-cadence-quadspi.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
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;
}
--
2.17.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
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
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
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