* Re: [PATCH 5/5] staging: media: ipu7: fix double-free of pdata in error paths
[not found] <20260412205057.386856-5-hossu.alexandru@gmail.com>
@ 2026-04-13 10:02 ` Alexandru Hossu
2026-04-13 10:15 ` [PATCH v2] staging: media: ipu7: fix double-free and use-after-free " Alexandru Hossu
1 sibling, 0 replies; 3+ messages in thread
From: Alexandru Hossu @ 2026-04-13 10:02 UTC (permalink / raw)
To: Dan Carpenter
Cc: linux-media, sakari.ailus, bingbu.cao, mchehab, gregkh,
linux-staging, linux-kernel
On Mon, Apr 13, 2026, Dan Carpenter wrote:
> We need a Fixes tag.
>
> The put_device() frees isys_adev as well so this is a use after free.
>
> ret = dev_err_probe(dev, PTR_ERR(isys_adev->mmu), ...
> put_device();
> return ret;
Good catch on the use-after-free in the return value. Will send v2 with:
- Fixes: b7fe4c0019b1 ("media: staging/ipu7: add Intel IPU7 PCI device driver")
- Save error code via ret = dev_err_probe() before put_device() in both
ipu7_isys_init() and ipu7_psys_init(), then return ERR_PTR(ret)
Alexandru
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH v2] staging: media: ipu7: fix double-free and use-after-free in error paths
[not found] <20260412205057.386856-5-hossu.alexandru@gmail.com>
2026-04-13 10:02 ` [PATCH 5/5] staging: media: ipu7: fix double-free of pdata in error paths Alexandru Hossu
@ 2026-04-13 10:15 ` Alexandru Hossu
2026-04-13 10:27 ` Dan Carpenter
1 sibling, 1 reply; 3+ messages in thread
From: Alexandru Hossu @ 2026-04-13 10:15 UTC (permalink / raw)
To: linux-media
Cc: error27, sakari.ailus, bingbu.cao, mchehab, gregkh, linux-staging,
linux-kernel, hossu.alexandru
In both ipu7_isys_init() and ipu7_psys_init(), pdata is allocated and
then passed to ipu7_bus_initialize_device(), which stores it in
adev->pdata. The ipu7_bus_release() function frees adev->pdata when the
device's reference count drops to zero.
Two error paths incorrectly call kfree(pdata) after the device teardown
has already freed it:
1. When ipu7_mmu_init() fails: put_device() is called, which drops the
reference count to zero and triggers ipu7_bus_release() ->
kfree(pdata). The subsequent kfree(pdata) is a double-free.
2. When ipu7_bus_add_device() fails: it calls auxiliary_device_uninit()
internally, which calls put_device() -> ipu7_bus_release() ->
kfree(pdata). The subsequent kfree(pdata) is again a double-free.
Note that the kfree(pdata) when ipu7_bus_initialize_device() itself
fails is correct, because in that case auxiliary_device_init() failed
and the release function was never set up, so pdata must be freed
manually.
Additionally, the error code was not saved before calling put_device(),
causing ERR_CAST() to dereference the already-freed adev pointer when
constructing the return value. Fix this by saving the error from
dev_err_probe() before put_device() and returning ERR_PTR() instead.
Remove the redundant kfree(pdata) calls and fix the use-after-free in
the return values of the two affected error paths.
Fixes: b7fe4c0019b1 ("media: staging/ipu7: add Intel IPU7 PCI device driver")
Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
---
v2:
- Add Fixes tag (Dan Carpenter)
- Save error before put_device() to avoid use-after-free in ERR_CAST()
return value; use ERR_PTR(ret) instead (Dan Carpenter)
- Apply same fix to ipu7_psys_init() (Dan Carpenter)
drivers/staging/media/ipu7/ipu7.c | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/media/ipu7/ipu7.c b/drivers/staging/media/ipu7/ipu7.c
index c771e763f8c5..310e3f24e571 100644
--- a/drivers/staging/media/ipu7/ipu7.c
+++ b/drivers/staging/media/ipu7/ipu7.c
@@ -2169,21 +2169,18 @@ ipu7_isys_init(struct pci_dev *pdev, struct device *parent,
isys_adev->mmu = ipu7_mmu_init(dev, base, ISYS_MMID,
&ipdata->hw_variant);
if (IS_ERR(isys_adev->mmu)) {
- dev_err_probe(dev, PTR_ERR(isys_adev->mmu),
- "ipu7_mmu_init(isys_adev->mmu) failed\n");
+ ret = dev_err_probe(dev, PTR_ERR(isys_adev->mmu),
+ "ipu7_mmu_init(isys_adev->mmu) failed\n");
put_device(&isys_adev->auxdev.dev);
- kfree(pdata);
- return ERR_CAST(isys_adev->mmu);
+ return ERR_PTR(ret);
}
isys_adev->mmu->dev = &isys_adev->auxdev.dev;
isys_adev->subsys = IPU_IS;
ret = ipu7_bus_add_device(isys_adev);
- if (ret) {
- kfree(pdata);
+ if (ret)
return ERR_PTR(ret);
- }
return isys_adev;
}
@@ -2216,21 +2213,18 @@ ipu7_psys_init(struct pci_dev *pdev, struct device *parent,
psys_adev->mmu = ipu7_mmu_init(&pdev->dev, base, PSYS_MMID,
&ipdata->hw_variant);
if (IS_ERR(psys_adev->mmu)) {
- dev_err_probe(&pdev->dev, PTR_ERR(psys_adev->mmu),
- "ipu7_mmu_init(psys_adev->mmu) failed\n");
+ ret = dev_err_probe(&pdev->dev, PTR_ERR(psys_adev->mmu),
+ "ipu7_mmu_init(psys_adev->mmu) failed\n");
put_device(&psys_adev->auxdev.dev);
- kfree(pdata);
- return ERR_CAST(psys_adev->mmu);
+ return ERR_PTR(ret);
}
psys_adev->mmu->dev = &psys_adev->auxdev.dev;
psys_adev->subsys = IPU_PS;
ret = ipu7_bus_add_device(psys_adev);
- if (ret) {
- kfree(pdata);
+ if (ret)
return ERR_PTR(ret);
- }
return psys_adev;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 3+ messages in thread