| 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 |
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
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 --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; }
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(-)