[PATCH v2 0/6] media: Use devm_of_reserved_mem_device_init()

[PATCH v2 0/6] media: Use devm_of_reserved_mem_device_init()

[Mukesh Ojha]

Several media drivers manually manage reserved memory lifetime by calling
of_reserved_mem_device_init() in probe and of_reserved_mem_device_release()
in remove. Drivers that fail to release on every error path between the two
calls leak the reserved memory region until the driver is unloaded.

devm_of_reserved_mem_device_init() was recently introduced to tie the
release to the device's devres lifetime automatically. Convert the affected
media drivers to use it, removing the manual release calls and any wrapper
devres actions.

Changes since v1: https://lore.kernel.org/lkml/20260703193855.110619-2-mukesh.ojha@oss.qualcomm.com/
 - Carried R-b tag if any.
 - Rebased it.

Mukesh Ojha (6):
  media: synopsys: hdmirx: Use devm_of_reserved_mem_device_init()
  media: aspeed: Use devm_of_reserved_mem_device_init()
  media: nuvoton: npcm-video: Use devm_of_reserved_mem_device_init()
  media: arm: mali-c55: Use devm_of_reserved_mem_device_init()
  media: mediatek: vpu: Use devm_of_reserved_mem_device_init()
  staging: media: cedrus: Use devm_of_reserved_mem_device_init()

 .../media/platform/arm/mali-c55/mali-c55-core.c  | 12 +++---------
 drivers/media/platform/aspeed/aspeed-video.c     | 12 ++++--------
 drivers/media/platform/mediatek/vpu/mtk_vpu.c    |  3 +--
 drivers/media/platform/nuvoton/npcm-video.c      |  9 ++-------
 .../media/platform/synopsys/hdmirx/snps_hdmirx.c | 16 ++--------------
 drivers/staging/media/sunxi/cedrus/cedrus_hw.c   |  9 ++-------
 6 files changed, 14 insertions(+), 47 deletions(-)

-- 
2.55.0

[PATCH v2 1/6] media: synopsys: hdmirx: Use devm_of_reserved_mem_device_init()

[Mukesh Ojha]

Replace the hand-rolled devm_hdmirx_of_reserved_mem_device_release()
devres action with devm_of_reserved_mem_device_init(), which handles the
cleanup automatically. This removes both the wrapper function and the
devm_add_action_or_reset() call.

Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 .../media/platform/synopsys/hdmirx/snps_hdmirx.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
index 25f8ca0d6d94..7c6acc6d7e88 100644
--- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
+++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
@@ -2265,11 +2265,6 @@ static const struct hdmirx_cec_ops hdmirx_cec_ops = {
 	.read = hdmirx_readl,
 };
 
-static void devm_hdmirx_of_reserved_mem_device_release(void *dev)
-{
-	of_reserved_mem_device_release(dev);
-}
-
 static int hdmirx_parse_dt(struct snps_hdmirx_dev *hdmirx_dev)
 {
 	struct device *dev = hdmirx_dev->dev;
@@ -2316,16 +2311,9 @@ static int hdmirx_parse_dt(struct snps_hdmirx_dev *hdmirx_dev)
 	if (!device_property_read_bool(dev, "hpd-is-active-low"))
 		hdmirx_dev->hpd_trigger_level_high = true;
 
-	ret = of_reserved_mem_device_init(dev);
-	if (ret) {
+	ret = devm_of_reserved_mem_device_init(dev);
+	if (ret)
 		dev_warn(dev, "no reserved memory for HDMIRX, use default CMA\n");
-	} else {
-		ret = devm_add_action_or_reset(dev,
-					       devm_hdmirx_of_reserved_mem_device_release,
-					       dev);
-		if (ret)
-			return ret;
-	}
 
 	return 0;
 }
-- 
2.55.0

Re: [PATCH v2 1/6] media: synopsys: hdmirx: Use devm_of_reserved_mem_device_init()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Probe deferral breakage due to overriding devm_clk_bulk_get_all() error code.
--

commit 9b061d6dbcd6ec9063d57f4982dfa6c2f6ec7b98
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
Subject: media: synopsys: hdmirx: Use devm_of_reserved_mem_device_init()

This commit replaces the hand-rolled devres action for releasing reserved
memory with devm_of_reserved_mem_device_init(), which handles the cleanup
automatically.

> diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> index 25f8ca0d6d946..7c6acc6d7e882 100644
> --- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> +++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> @@ -2265,11 +2265,6 @@ static const struct hdmirx_cec_ops hdmirx_cec_ops = {
>  	.read = hdmirx_readl,
>  };
>  
> -static void devm_hdmirx_of_reserved_mem_device_release(void *dev)
> -{
> -	of_reserved_mem_device_release(dev);
> -}
> -
>  static int hdmirx_parse_dt(struct snps_hdmirx_dev *hdmirx_dev)
>  {
>  	struct device *dev = hdmirx_dev->dev;

[Severity: High]
This isn't a bug introduced by this patch, but does this code break probe
deferral?

Looking at hdmirx_parse_dt() in snps_hdmirx.c, when devm_clk_bulk_get_all()
fails, the negative error code is discarded:

	hdmirx_dev->num_clks = devm_clk_bulk_get_all(dev, &hdmirx_dev->clks);
	if (hdmirx_dev->num_clks < 1)
		return -ENODEV;

If devm_clk_bulk_get_all() returns -EPROBE_DEFER because the clock providers
are not yet ready, returning -ENODEV masks the deferral request. This
converts a transient dependency delay into a permanent initialization failure.

Should this code be updated to propagate the negative error code instead?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902200703.2016410-1-mukesh.ojha@oss.qualcomm.com?part=1

[PATCH v2 2/6] media: aspeed: Use devm_of_reserved_mem_device_init()

[Mukesh Ojha]

Switch to devm_of_reserved_mem_device_init() so the reserved memory
region is released automatically on probe failure or device removal.
Remove the three explicit of_reserved_mem_device_release() calls and
rename the error label from err_release_reserved_mem to
err_unprepare_vclk to reflect that it now only unwinds clock state.

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/media/platform/aspeed/aspeed-video.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/aspeed/aspeed-video.c b/drivers/media/platform/aspeed/aspeed-video.c
index a292275f6b7b..7ba9314fdbd2 100644
--- a/drivers/media/platform/aspeed/aspeed-video.c
+++ b/drivers/media/platform/aspeed/aspeed-video.c
@@ -2267,19 +2267,19 @@ static int aspeed_video_init(struct aspeed_video *video)
 	if (rc)
 		goto err_unprepare_eclk;
 
-	of_reserved_mem_device_init(dev);
+	devm_of_reserved_mem_device_init(dev);
 
 	rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
 	if (rc) {
 		dev_err(dev, "Failed to set DMA mask\n");
-		goto err_release_reserved_mem;
+		goto err_unprepare_vclk;
 	}
 
 	if (!aspeed_video_alloc_buf(video, &video->jpeg,
 				    VE_JPEG_HEADER_SIZE)) {
 		dev_err(dev, "Failed to allocate DMA for JPEG header\n");
 		rc = -ENOMEM;
-		goto err_release_reserved_mem;
+		goto err_unprepare_vclk;
 	}
 	dev_info(video->dev, "alloc mem size(%d) at %pad for jpeg header\n",
 		 VE_JPEG_HEADER_SIZE, &video->jpeg.dma);
@@ -2288,8 +2288,7 @@ static int aspeed_video_init(struct aspeed_video *video)
 
 	return 0;
 
-err_release_reserved_mem:
-	of_reserved_mem_device_release(dev);
+err_unprepare_vclk:
 	clk_unprepare(video->vclk);
 err_unprepare_eclk:
 	clk_unprepare(video->eclk);
@@ -2343,7 +2342,6 @@ static int aspeed_video_probe(struct platform_device *pdev)
 	rc = aspeed_video_setup_video(video);
 	if (rc) {
 		aspeed_video_free_buf(video, &video->jpeg);
-		of_reserved_mem_device_release(&pdev->dev);
 		clk_unprepare(video->vclk);
 		clk_unprepare(video->eclk);
 		return rc;
@@ -2374,8 +2372,6 @@ static void aspeed_video_remove(struct platform_device *pdev)
 	v4l2_device_unregister(v4l2_dev);
 
 	aspeed_video_free_buf(video, &video->jpeg);
-
-	of_reserved_mem_device_release(dev);
 }
 
 static struct platform_driver aspeed_video_driver = {
-- 
2.55.0

Re: [PATCH v2 2/6] media: aspeed: Use devm_of_reserved_mem_device_init()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] Preexisting use-after-free of `struct aspeed_video` on driver unbind due to devm_kzalloc and embedded video_device.
- [High] Preexisting use-after-free/corruption of CMA reserved memory on driver unbind.
- [High] IRQ registered and unmasked before device clocks are enabled.
- [High] Uncanceled delayed work accesses freed memory on device removal.
--

commit 788dfc64dfdd64116ee0bc0902a2de61fae97d51
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

media: aspeed: Use devm_of_reserved_mem_device_init()

Switch to devm_of_reserved_mem_device_init() so the reserved memory
region is released automatically on probe failure or device removal.
Remove the three explicit of_reserved_mem_device_release() calls and
rename the error label from err_release_reserved_mem to
err_unprepare_vclk to reflect that it now only unwinds clock state.

> diff --git a/drivers/media/platform/aspeed/aspeed-video.c b/drivers/media/platform/aspeed/aspeed-video.c
> index a292275f6b7b9..7ba9314fdbd2a 100644
> --- a/drivers/media/platform/aspeed/aspeed-video.c
> +++ b/drivers/media/platform/aspeed/aspeed-video.c
> @@ -2267,19 +2267,19 @@ static int aspeed_video_init(struct aspeed_video *video)
>  	if (rc)
>  		goto err_unprepare_eclk;
>  
> -	of_reserved_mem_device_init(dev);
> +	devm_of_reserved_mem_device_init(dev);

[Severity: High]
This isn't a bug introduced by this patch, but does tying the CMA reserved
memory release to devres cleanup here risk a use-after-free or corruption
of CMA reserved memory on driver unbind?

If a user holds an open file descriptor when the device is unbound, active
VB2 DMA buffers may still exist. When the user later closes the file
descriptor, VB2 calls dma_free_coherent() on the buffers:

drivers/media/platform/aspeed/aspeed-video.c:aspeed_video_remove() {
    ...
    vb2_video_unregister_device(&video->vdev);
    ...
}

Since devres releases the CMA region immediately after remove() returns,
could the DMA framework incorrectly handle the free operation on the detached
region and corrupt allocator pools?

As another pre-existing issue in this initialization path, the interrupt
handler is registered earlier in this function:

drivers/media/platform/aspeed/aspeed-video.c:aspeed_video_init() {
    ...
    rc = devm_request_threaded_irq(dev, irq, NULL, aspeed_video_irq,
                                   IRQF_ONESHOT, DEVICE_NAME, video);
    ...
}

This registers and unmasks the IRQ before the device clocks are actually
enabled. If the IRQ line is shared or the bootloader left the device in an
active state, could the interrupt handler fire immediately and access device
registers via aspeed_video_read(), causing a bus fault because the clocks
are disabled?

[ ... ]
> @@ -2343,7 +2342,6 @@ static int aspeed_video_probe(struct platform_device *pdev)
>  	rc = aspeed_video_setup_video(video);
>  	if (rc) {
>  		aspeed_video_free_buf(video, &video->jpeg);
> -		of_reserved_mem_device_release(&pdev->dev);
>  		clk_unprepare(video->vclk);

[Severity: High]
This is a pre-existing issue, but could the use of devres for the driver
context allocation in probe lead to a use-after-free on unbind?

drivers/media/platform/aspeed/aspeed-video.c:aspeed_video_probe() {
    ...
    video = devm_kzalloc(&pdev->dev, sizeof(*video), GFP_KERNEL);
    ...
}

Because vdev->release is set to video_device_release_empty in
aspeed_video_setup_video(), the V4L2 core does not defer freeing the memory:

drivers/media/platform/aspeed/aspeed-video.c:aspeed_video_setup_video() {
    ...
    vdev->release = video_device_release_empty;
    ...
}

When a user closes the file descriptor after an unbind, devres will have
already freed the struct aspeed_video, but aspeed_video_release() will
still try to access it:

drivers/media/platform/aspeed/aspeed-video.c:aspeed_video_release() {
    ...
    aspeed_video_stop(video);
    ...
}

Could this result in memory corruption when the last user closes the file
descriptor?

[ ... ]
> @@ -2374,8 +2372,6 @@ static void aspeed_video_remove(struct platform_device *pdev)
>  	v4l2_device_unregister(v4l2_dev);
>  
>  	aspeed_video_free_buf(video, &video->jpeg);
> -
> -	of_reserved_mem_device_release(dev);
>  }

[Severity: High]
This is a pre-existing issue, but does this remove function lack a
cancellation of the delayed resolution work, potentially causing a
use-after-free?

A resolution change interrupt can schedule video->res_work:

drivers/media/platform/aspeed/aspeed-video.c:aspeed_video_irq_res_change() {
    ...
    schedule_delayed_work(&video->res_work, delay);
    ...
}

If a driver unbind occurs before the delay expires, aspeed_video_remove()
turns off the hardware but does not call
cancel_delayed_work_sync(&video->res_work).

Could the workqueue execute aspeed_video_resolution_work() after devres
has freed the struct aspeed_video, resulting in a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902200703.2016410-1-mukesh.ojha@oss.qualcomm.com?part=2

[PATCH v2 3/6] media: nuvoton: npcm-video: Use devm_of_reserved_mem_device_init()

[Mukesh Ojha]

Switch to devm_of_reserved_mem_device_init() so the reserved memory
region is released automatically on probe failure or device removal.
Remove the four explicit of_reserved_mem_device_release() calls and
collapse the now-empty err_release_mem label into err_free.

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/media/platform/nuvoton/npcm-video.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/nuvoton/npcm-video.c b/drivers/media/platform/nuvoton/npcm-video.c
index 52505af35c08..6786736c8f07 100644
--- a/drivers/media/platform/nuvoton/npcm-video.c
+++ b/drivers/media/platform/nuvoton/npcm-video.c
@@ -1715,17 +1715,15 @@ static int npcm_video_init(struct npcm_video *video)
 		return rc;
 	}
 
-	of_reserved_mem_device_init(dev);
+	devm_of_reserved_mem_device_init(dev);
 	rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
 	if (rc) {
 		dev_err(dev, "Failed to set DMA mask\n");
-		of_reserved_mem_device_release(dev);
 		return rc;
 	}
 
 	rc = npcm_video_ece_init(video);
 	if (rc) {
-		of_reserved_mem_device_release(dev);
 		dev_err(dev, "Failed to initialize ECE\n");
 		return rc;
 	}
@@ -1789,13 +1787,11 @@ static int npcm_video_probe(struct platform_device *pdev)
 
 	rc = npcm_video_setup_video(video);
 	if (rc)
-		goto err_release_mem;
+		goto err_free;
 
 	dev_info(video->dev, "NPCM video driver probed\n");
 	return 0;
 
-err_release_mem:
-	of_reserved_mem_device_release(&pdev->dev);
 err_free:
 	kfree(video);
 	return rc;
@@ -1814,7 +1810,6 @@ static void npcm_video_remove(struct platform_device *pdev)
 	if (video->ece.enable)
 		npcm_video_ece_stop(video);
 	kfree(video);
-	of_reserved_mem_device_release(dev);
 }
 
 static const struct of_device_id npcm_video_match[] = {
-- 
2.55.0

Re: [PATCH v2 3/6] media: nuvoton: npcm-video: Use devm_of_reserved_mem_device_init()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential use-after-free of the driver's private `video` structure in the interrupt handler due to devres lifecycle mismatch.
- [High] V4L2 lifecycle mismatch causes a use-after-free of the embedded `video_device` structure.
- [High] Device-managed allocations are incorrectly attached to an external child device, causing memory leaks and preventing reprobing.
--

commit 5d2eb460720d228622a94970ff2802d5f5591843
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

media: nuvoton: npcm-video: Use devm_of_reserved_mem_device_init()

Switch to devm_of_reserved_mem_device_init() so the reserved memory
region is released automatically on probe failure or device removal.
Remove the four explicit of_reserved_mem_device_release() calls and
collapse the now-empty err_release_mem label into err_free.

> diff --git a/drivers/media/platform/nuvoton/npcm-video.c b/drivers/media/platform/nuvoton/npcm-video.c
> index 52505af35c087..6786736c8f07c 100644
> --- a/drivers/media/platform/nuvoton/npcm-video.c
> +++ b/drivers/media/platform/nuvoton/npcm-video.c
> @@ -1715,17 +1715,15 @@ static int npcm_video_init(struct npcm_video *video)
>  		return rc;
>  	}
>  
> -	of_reserved_mem_device_init(dev);
> +	devm_of_reserved_mem_device_init(dev);
>  	rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
>  	if (rc) {
>  		dev_err(dev, "Failed to set DMA mask\n");
> -		of_reserved_mem_device_release(dev);
>  		return rc;
>  	}
>  
>  	rc = npcm_video_ece_init(video);

[Severity: High]
This is a pre-existing issue, but could this leak resources when the
npcm-video driver is unloaded?

In npcm_video_ece_init() in drivers/media/platform/nuvoton/npcm-video.c,
the driver fetches the external device node ece_pdev and uses its device
pointer for devres allocations:

	regs = devm_platform_ioremap_resource(ece_pdev, 0);
	...
	video->ece.reset = devm_reset_control_get(ece_dev, NULL);

Since these allocations are tied to the external nuvoton,ece device's
lifecycle rather than the driver's device, the resources won't be released
when the npcm-video driver unbinds. If the driver is re-probed, could the
ioremap request fail with -EBUSY and permanently break the driver?

>  	if (rc) {
> -		of_reserved_mem_device_release(dev);
>  		dev_err(dev, "Failed to initialize ECE\n");
>  		return rc;
>  	}

[ ... ]

> @@ -1789,13 +1787,11 @@ static int npcm_video_probe(struct platform_device *pdev)
>  
>  	rc = npcm_video_setup_video(video);
>  	if (rc)
> -		goto err_release_mem;
> +		goto err_free;
>  
>  	dev_info(video->dev, "NPCM video driver probed\n");
>  	return 0;
>  
> -err_release_mem:
> -	of_reserved_mem_device_release(&pdev->dev);
>  err_free:
>  	kfree(video);

[Severity: High]
This is a pre-existing issue, but can this cause a use-after-free of the
driver's private video structure in the interrupt handler?

If npcm_video_probe() in drivers/media/platform/nuvoton/npcm-video.c fails
here, the driver manually calls kfree(video). However, earlier in
npcm_video_init(), the interrupt was registered using
devm_request_threaded_irq().

Because devres cleanup happens after the probe function returns, the IRQ
remains registered and capable of firing while the video structure has already
been freed. If an interrupt fires during this window, won't the handler
execute and access the freed structure?

>  	return rc;
> @@ -1814,7 +1810,6 @@ static void npcm_video_remove(struct platform_device *pdev)
>  	if (video->ece.enable)
>  		npcm_video_ece_stop(video);
>  	kfree(video);

[Severity: High]
This is a pre-existing issue, but can this kfree() also trigger a
use-after-free due to the devm-managed IRQ lifecycle, similar to the probe
error path?

In npcm_video_remove() in drivers/media/platform/nuvoton/npcm-video.c, the
devres cleanup won't disable the interrupt until after the remove function
finishes. This leaves a window where the IRQ can fire and access the freed
video structure.

[Severity: High]
This isn't a bug introduced by this patch, but can this cause a
use-after-free if userspace still has the V4L2 device open?

In npcm_video_setup_video() in drivers/media/platform/nuvoton/npcm-video.c,
the driver embeds struct video_device inside its dynamically allocated
struct npcm_video and sets its .release callback to
video_device_release_empty.

Since video_unregister_device() in npcm_video_remove() does not wait for file
descriptors to be closed, could subsequent userspace operations access the
embedded vdev after this kfree() has freed the enclosing structure?

> -	of_reserved_mem_device_release(dev);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902200703.2016410-1-mukesh.ojha@oss.qualcomm.com?part=3

[PATCH v2 4/6] media: arm: mali-c55: Use devm_of_reserved_mem_device_init()

[Mukesh Ojha]

Use the devres-managed devm_of_reserved_mem_device_init() instead of
the manual of_reserved_mem_device_init()/of_reserved_mem_device_release()
pair, letting the device resource manager handle cleanup automatically.

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/media/platform/arm/mali-c55/mali-c55-core.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-core.c b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
index f28e9f4354ac..3d37cd446583 100644
--- a/drivers/media/platform/arm/mali-c55/mali-c55-core.c
+++ b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
@@ -803,14 +803,12 @@ static int mali_c55_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to acquire resets\n");
 
-	of_reserved_mem_device_init(dev);
+	devm_of_reserved_mem_device_init(dev);
 	vb2_dma_contig_set_max_seg_size(dev, UINT_MAX);
 
 	ret = __mali_c55_power_on(mali_c55);
-	if (ret) {
-		dev_err_probe(dev, ret, "failed to power on\n");
-		goto err_release_mem;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to power on\n");
 
 	ret = mali_c55_check_hwcfg(mali_c55);
 	if (ret)
@@ -849,9 +847,6 @@ static int mali_c55_probe(struct platform_device *pdev)
 	kfree(mali_c55->context.registers);
 err_power_off:
 	__mali_c55_power_off(mali_c55);
-err_release_mem:
-	of_reserved_mem_device_release(dev);
-
 	return ret;
 }
 
@@ -866,7 +861,6 @@ static void mali_c55_remove(struct platform_device *pdev)
 	}
 	pm_runtime_disable(&pdev->dev);
 	kfree(mali_c55->context.registers);
-	of_reserved_mem_device_release(&pdev->dev);
 }
 
 static const struct of_device_id mali_c55_of_match[] = {
-- 
2.55.0

Re: [PATCH v2 4/6] media: arm: mali-c55: Use devm_of_reserved_mem_device_init()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Race condition between runtime PM autosuspend and manual power-off during device removal and probe error paths.
- [High] The IRQ handler is never freed if the device is removed while in the active runtime PM state.
- [High] Media nodes are registered and exposed to userspace before the device's IRQ is requested, breaking initial video capture.
--

commit c75be1887c52469e855778c08a0a6679f1af97b0
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

media: arm: mali-c55: Use devm_of_reserved_mem_device_init()

This commit refactors the driver to use the devres-managed
devm_of_reserved_mem_device_init() instead of manual initialization and
cleanup.

> diff --git a/drivers/media/platform/arm/mali-c55/mali-c55-core.c b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> index f28e9f4354aca..3d37cd4465838 100644
> --- a/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> +++ b/drivers/media/platform/arm/mali-c55/mali-c55-core.c
> @@ -803,14 +803,12 @@ static int mali_c55_probe(struct platform_device *pdev)
>  	if (ret)
>  		return dev_err_probe(dev, ret, "failed to acquire resets\n");
>  
> -	of_reserved_mem_device_init(dev);
> +	devm_of_reserved_mem_device_init(dev);
>  	vb2_dma_contig_set_max_seg_size(dev, UINT_MAX);
>  
>  	ret = __mali_c55_power_on(mali_c55);

[Severity: High]
This is a pre-existing issue, but are the media nodes registered and exposed
to userspace before the device's IRQ is requested?

Later in mali_c55_probe():

	ret = mali_c55_media_frameworks_init(mali_c55);
	if (ret)
		goto err_pm_runtime_disable;

	pm_runtime_idle(&pdev->dev);

	mali_c55->irqnum = platform_get_irq(pdev, 0);

The device is explicitly set to RPM_ACTIVE during probe, but the IRQ handler
is only requested inside mali_c55_runtime_resume().

If userspace opens the device and begins streaming before the autosuspend
delay forces a suspend cycle, the device will process frames without an IRQ
handler. Will this cause video streaming to hang indefinitely because the
capture buffers are never completed?

> -	if (ret) {
> -		dev_err_probe(dev, ret, "failed to power on\n");
> -		goto err_release_mem;
> -	}
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to power on\n");
>  
>  	ret = mali_c55_check_hwcfg(mali_c55);
>  	if (ret)

[ ... ]

> @@ -849,9 +847,6 @@ static int mali_c55_probe(struct platform_device *pdev)
>  	kfree(mali_c55->context.registers);
>  err_power_off:
>  	__mali_c55_power_off(mali_c55);

[Severity: High]
This is a pre-existing issue, but is there a risk of a double power-off
here if the device successfully auto-suspended before pm_runtime_disable()
is called?

If the autosuspend timer triggered, calling __mali_c55_power_off() again
could lead to imbalanced clock disables (calling clk_bulk_disable_unprepare()
without a matching enable) and reset assertions, potentially causing kernel
warnings in the clock and reset cores.

> -err_release_mem:
> -	of_reserved_mem_device_release(dev);
> -
>  	return ret;
>  }
>  
> @@ -866,7 +861,6 @@ static void mali_c55_remove(struct platform_device *pdev)
>  	}

[Severity: High]
This is a pre-existing issue, but does this manual power-off in
mali_c55_remove() create a race window with concurrent autosuspend?

In mali_c55_remove(), if the device is not suspended:

	if (!pm_runtime_suspended(&pdev->dev)) {
		__mali_c55_power_off(mali_c55);
		pm_runtime_set_suspended(&pdev->dev);
	}

Because pm_runtime_disable() is called after this check, an asynchronous
autosuspend could execute concurrently. Could this result in both threads
calling __mali_c55_power_off() at the same time?

[Severity: High]
This is a pre-existing issue, but is it possible that bypassing the runtime
suspend callback here causes an IRQ resource leak?

The driver dynamically registers its IRQ handler in mali_c55_runtime_resume()
and frees it in mali_c55_runtime_suspend(). If the device is active during
removal, open-coding the power-off with __mali_c55_power_off() bypasses the
free_irq() call.

Could a subsequent interrupt on this line (or shared IRQ) trigger the handler
and cause a use-after-free when accessing the freed device context?

>  	pm_runtime_disable(&pdev->dev);
>  	kfree(mali_c55->context.registers);
> -	of_reserved_mem_device_release(&pdev->dev);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902200703.2016410-1-mukesh.ojha@oss.qualcomm.com?part=4

[PATCH v2 5/6] media: mediatek: vpu: Use devm_of_reserved_mem_device_init()

[Mukesh Ojha]

Use the devres-managed devm_of_reserved_mem_device_init() instead of
the manual of_reserved_mem_device_init()/of_reserved_mem_device_release()
pair, letting the device resource manager handle cleanup automatically.

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/media/platform/mediatek/vpu/mtk_vpu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/platform/mediatek/vpu/mtk_vpu.c b/drivers/media/platform/mediatek/vpu/mtk_vpu.c
index 8d8319f0cd22..557bffb2c2f1 100644
--- a/drivers/media/platform/mediatek/vpu/mtk_vpu.c
+++ b/drivers/media/platform/mediatek/vpu/mtk_vpu.c
@@ -888,7 +888,7 @@ static int mtk_vpu_probe(struct platform_device *pdev)
 	dev_info(dev, "4GB mode %u\n", vpu->enable_4GB);
 
 	if (vpu->enable_4GB) {
-		ret = of_reserved_mem_device_init(dev);
+		ret = devm_of_reserved_mem_device_init(dev);
 		if (ret)
 			dev_info(dev, "init reserved memory failed\n");
 			/* continue to use dynamic allocation if failed */
@@ -930,7 +930,6 @@ static int mtk_vpu_probe(struct platform_device *pdev)
 free_d_mem:
 	vpu_free_ext_mem(vpu, D_FW);
 remove_debugfs:
-	of_reserved_mem_device_release(dev);
 #ifdef CONFIG_DEBUG_FS
 	debugfs_remove(vpu_debugfs);
 #endif
-- 
2.55.0

Re: [PATCH v2 5/6] media: mediatek: vpu: Use devm_of_reserved_mem_device_init()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing race condition leading to a use-after-free. The driver uses `devm_request_irq()` to manage its interrupt, but manually destroys the workqueue (`vpu->wdt.wq`) in `mtk_vpu_remove()`. Because devres cleans up the IRQ only after `remove()` completes, the interrupt remains active while the workqueue is freed.
--

commit 246438dea9de2810275787060608206f1c3a4872
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

media: mediatek: vpu: Use devm_of_reserved_mem_device_init()

This patch simplifies the driver by replacing manual
of_reserved_mem_device_init() and of_reserved_mem_device_release()
calls with the managed devm_of_reserved_mem_device_init() equivalent.

> diff --git a/drivers/media/platform/mediatek/vpu/mtk_vpu.c b/drivers/media/platform/mediatek/vpu/mtk_vpu.c
> --- a/drivers/media/platform/mediatek/vpu/mtk_vpu.c
> +++ b/drivers/media/platform/mediatek/vpu/mtk_vpu.c
[ ... ]
> @@ -930,7 +930,6 @@ static int mtk_vpu_probe(struct platform_device *pdev)
>  free_d_mem:
>  	vpu_free_ext_mem(vpu, D_FW);
>  remove_debugfs:
> -	of_reserved_mem_device_release(dev);

[Severity: High]
This is a pre-existing issue, but while reviewing this devres conversion, I
noticed a potential use-after-free race condition related to device teardown.

In mtk_vpu_probe(), the interrupt is registered as managed using
devm_request_irq(), meaning it will remain active until devres cleanup runs
after mtk_vpu_remove() completes.

However, the workqueue is manually destroyed during mtk_vpu_remove():

drivers/media/platform/mediatek/vpu/mtk_vpu.c:mtk_vpu_remove() {
    ...
    if (vpu->wdt.wq)
        destroy_workqueue(vpu->wdt.wq);
    ...
}

If a hardware interrupt fires after destroy_workqueue() executes but before
the devres cleanup disables the IRQ, vpu_irq_handler() will attempt to
access the freed workqueue structure:

drivers/media/platform/mediatek/vpu/mtk_vpu.c:vpu_irq_handler() {
    ...
    queue_work(vpu->wdt.wq, &vpu->wdt.ws);
    ...
}

Can this sequence cause a use-after-free if the hardware triggers an
interrupt late during the unbind process?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902200703.2016410-1-mukesh.ojha@oss.qualcomm.com?part=5

Re: [PATCH v2 5/6] media: mediatek: vpu: Use devm_of_reserved_mem_device_init()

[Chen-Yu Tsai]

On Thu, Sep 3, 2026 at 4:08 AM Mukesh Ojha <mukesh.ojha@oss.qualcomm.com> wrote:
>
> Use the devres-managed devm_of_reserved_mem_device_init() instead of
> the manual of_reserved_mem_device_init()/of_reserved_mem_device_release()
> pair, letting the device resource manager handle cleanup automatically.
>
> Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
> ---
>  drivers/media/platform/mediatek/vpu/mtk_vpu.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/vpu/mtk_vpu.c b/drivers/media/platform/mediatek/vpu/mtk_vpu.c
> index 8d8319f0cd22..557bffb2c2f1 100644
> --- a/drivers/media/platform/mediatek/vpu/mtk_vpu.c
> +++ b/drivers/media/platform/mediatek/vpu/mtk_vpu.c
> @@ -888,7 +888,7 @@ static int mtk_vpu_probe(struct platform_device *pdev)
>         dev_info(dev, "4GB mode %u\n", vpu->enable_4GB);
>
>         if (vpu->enable_4GB) {
> -               ret = of_reserved_mem_device_init(dev);
> +               ret = devm_of_reserved_mem_device_init(dev);

There are non-devm calls before this. Inserting a devm call in here
messes up the unwind order and leads to confusion. In other words
this is a bad idea.


OOTH, the remove function is missing a release call.

>                 if (ret)
>                         dev_info(dev, "init reserved memory failed\n");
>                         /* continue to use dynamic allocation if failed */
> @@ -930,7 +930,6 @@ static int mtk_vpu_probe(struct platform_device *pdev)
>  free_d_mem:
>         vpu_free_ext_mem(vpu, D_FW);
>  remove_debugfs:
> -       of_reserved_mem_device_release(dev);
>  #ifdef CONFIG_DEBUG_FS
>         debugfs_remove(vpu_debugfs);
>  #endif
> --
> 2.55.0
>
>

[PATCH v2 6/6] staging: media: cedrus: Use devm_of_reserved_mem_device_init()

[Mukesh Ojha]

Use the devres-managed devm_of_reserved_mem_device_init() instead of
the manual of_reserved_mem_device_init()/of_reserved_mem_device_release()
pair, letting the device resource manager handle cleanup automatically.

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus_hw.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
index 444fb53878d1..4a15f45ad077 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
@@ -266,7 +266,7 @@ int cedrus_hw_probe(struct cedrus_dev *dev)
 		return ret;
 	}
 
-	ret = of_reserved_mem_device_init(dev->dev);
+	ret = devm_of_reserved_mem_device_init(dev->dev);
 	if (ret && ret != -ENODEV) {
 		dev_err(dev->dev, "Failed to reserve memory\n");
 
@@ -277,7 +277,7 @@ int cedrus_hw_probe(struct cedrus_dev *dev)
 	if (ret) {
 		dev_err(dev->dev, "Failed to claim SRAM\n");
 
-		goto err_mem;
+		return ret;
 	}
 
 	dev->ahb_clk = devm_clk_get(dev->dev, "ahb");
@@ -340,9 +340,6 @@ int cedrus_hw_probe(struct cedrus_dev *dev)
 	pm_runtime_disable(dev->dev);
 err_sram:
 	sunxi_sram_release(dev->dev);
-err_mem:
-	of_reserved_mem_device_release(dev->dev);
-
 	return ret;
 }
 
@@ -353,6 +350,4 @@ void cedrus_hw_remove(struct cedrus_dev *dev)
 		cedrus_hw_suspend(dev->dev);
 
 	sunxi_sram_release(dev->dev);
-
-	of_reserved_mem_device_release(dev->dev);
 }
-- 
2.55.0

Re: [PATCH v2 6/6] staging: media: cedrus: Use devm_of_reserved_mem_device_init()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Early IRQ registration and incorrect devres initialization order causes kernel panics.
- [High] Devres cleanup clears CMA area before userspace file descriptors are closed, causing memory corruption on free.
--

commit be60086a6fce8e7046d627a68c0fd2fc94983ddb
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

staging: media: cedrus: Use devm_of_reserved_mem_device_init()

Use the devres-managed devm_of_reserved_mem_device_init() instead of
the manual of_reserved_mem_device_init()/of_reserved_mem_device_release()
pair, letting the device resource manager handle cleanup automatically.

> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> index 444fb53878d12..4a15f45ad077f 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> @@ -266,7 +266,7 @@ int cedrus_hw_probe(struct cedrus_dev *dev)
>  		return ret;
>  	}
>  
> -	ret = of_reserved_mem_device_init(dev->dev);
> +	ret = devm_of_reserved_mem_device_init(dev->dev);

[Severity: High]
This is a pre-existing issue, but does tying the reserved memory cleanup to
devres cause memory corruption when userspace holds file descriptors open?

The driver unregisters the video device via video_unregister_device() in
cedrus_remove(), which does not wait for userspace to close open file
descriptors. Userspace can still hold v4l2 buffers allocated from the
contiguous memory area pool.

By using devm_of_reserved_mem_device_init(), the device's memory area
(dev->cma_area) is automatically cleared on driver unbind.

When userspace eventually closes the file descriptor, cedrus_release() cleans
up the context:

drivers/staging/media/sunxi/cedrus/cedrus.c:cedrus_release() {
    ...
    v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
    ...
}

This will eventually call dma_free_contiguous() to free the buffers. Because
dev->cma_area is now null, dma_free_contiguous() assumes the memory was not
allocated from the device's reserved pool and falls back to __free_pages(page,
get_order(size)).

Since the contiguous allocator returns exactly the requested number of pages
while __free_pages() frees a power-of-two block, won't this free memory
belonging to other allocations and cause severe memory corruption?


[Severity: High]
This isn't a bug introduced by this patch, but is it safe to request the
interrupt this early in cedrus_hw_probe()?

Just above this diff in cedrus_hw_probe(), devm_request_irq() is called
before dev->watchdog_work and dev->m2m_dev are initialized (which happens
later in cedrus_probe()).

If the bootloader leaves the hardware active, an interrupt can fire
immediately, and cedrus_irq() will dereference dev->m2m_dev:

drivers/staging/media/sunxi/cedrus/cedrus_hw.c:cedrus_irq() {
    ...
    ctx = v4l2_m2m_get_curr_priv(dev->m2m_dev);
    ...
}

Since dev->m2m_dev is still null, won't this cause a null pointer dereference
inside spin_lock_irqsave() in v4l2_m2m_get_curr_priv()?

Additionally, devm_request_irq() is called before
devm_platform_ioremap_resource() maps dev->base. Because devres cleans up
resources in reverse order, dev->base will be unmapped before the interrupt
is freed.

If an interrupt fires during this unmapped window (such as during driver
unbind), won't cedrus_irq() access unmapped I/O memory and cause a page fault
or similar abort?

>  	if (ret && ret != -ENODEV) {
>  		dev_err(dev->dev, "Failed to reserve memory\n");

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902200703.2016410-1-mukesh.ojha@oss.qualcomm.com?part=6