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

[sashiko-bot@kernel.org]

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