Re: [PATCH 5/5] staging: media: ipu7: fix double-free of pdata in error paths

Re: [PATCH 5/5] staging: media: ipu7: fix double-free of pdata in error paths

[Alexandru Hossu]

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

[PATCH v2] staging: media: ipu7: fix double-free and use-after-free in error paths

[Alexandru Hossu]

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

Re: [PATCH v2] staging: media: ipu7: fix double-free and use-after-free in error paths

[Dan Carpenter]

On Mon, Apr 13, 2026 at 12:15:33PM +0200, Alexandru Hossu wrote:
> 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>
> ---

Both the resent patches are fine now.

Reviewed-by: Dan Carpenter <error27@gmail.com>

However, please could you resend the whole patch series as a new thread
(with the first two or whatever dropped)?

regards,
dan carpenter

[PATCH 5/5] staging: media: ipu7: fix double-free of pdata in error paths

[Alexandru Hossu]

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.

Remove the redundant kfree(pdata) calls from the two affected error
paths.

Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
---
 drivers/staging/media/ipu7/ipu7.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/media/ipu7/ipu7.c b/drivers/staging/media/ipu7/ipu7.c
index c771e763f8c5..043b67dfe19a 100644
--- a/drivers/staging/media/ipu7/ipu7.c
+++ b/drivers/staging/media/ipu7/ipu7.c
@@ -2172,7 +2172,6 @@ ipu7_isys_init(struct pci_dev *pdev, struct device *parent,
 		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);
 	}
 
@@ -2180,10 +2179,8 @@ ipu7_isys_init(struct pci_dev *pdev, struct device *parent,
 	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;
 }
@@ -2219,7 +2216,6 @@ ipu7_psys_init(struct pci_dev *pdev, struct device *parent,
 		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);
 	}
 
@@ -2227,10 +2223,8 @@ ipu7_psys_init(struct pci_dev *pdev, struct device *parent,
 	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

Re: [PATCH 5/5] staging: media: ipu7: fix double-free of pdata in error paths

[Dan Carpenter]

On Sun, Apr 12, 2026 at 10:51:28PM +0200, Alexandru Hossu wrote:
> 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.
> 
> Remove the redundant kfree(pdata) calls from the two affected error
> paths.
> 
> Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>

We need a Fixes tag.

> ---
>  drivers/staging/media/ipu7/ipu7.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/media/ipu7/ipu7.c b/drivers/staging/media/ipu7/ipu7.c
> index c771e763f8c5..043b67dfe19a 100644
> --- a/drivers/staging/media/ipu7/ipu7.c
> +++ b/drivers/staging/media/ipu7/ipu7.c
> @@ -2172,7 +2172,6 @@ ipu7_isys_init(struct pci_dev *pdev, struct device *parent,
>  		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);
                                ^^^^^^^^^^^^^^
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;

>  	}
>  
> @@ -2180,10 +2179,8 @@ ipu7_isys_init(struct pci_dev *pdev, struct device *parent,
>  	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;
>  }
> @@ -2219,7 +2216,6 @@ ipu7_psys_init(struct pci_dev *pdev, struct device *parent,
>  		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);

Same here?

regards,
dan carpenter


>  	}
>