diff mbox series

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

Message ID 20260902200703.2016410-7-mukesh.ojha@oss.qualcomm.com (mailing list archive)
State New
Headers
Series media: Use devm_of_reserved_mem_device_init() |

Commit Message

Mukesh Ojha Sept. 2, 2026, 8:07 p.m. UTC 
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(-)

Comments

sashiko-bot@kernel.org Sept. 2, 2026, 8:26 p.m. UTC | #1 
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");

[ ... ]
diff mbox series

Patch

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