diff mbox series

[07/22] media: platform: sun4i_csi: Add missing media_entity_cleanup()

Message ID 20260614165630.3896-8-birenpandya@gmail.com (mailing list archive)
State New
Headers
Series None |

Commit Message

Biren Pandya June 14, 2026, 4:55 p.m. UTC 
Fixes: 577bbf23b758 ("media: sunxi: Add A10 CSI driver")
Signed-off-by: Biren Pandya <birenpandya@gmail.com>
---
 drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

sashiko-bot@kernel.org June 14, 2026, 5:11 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The error path in `sun4i_csi_probe` fails to clean up the V4L2 async notifier, leaking the async connection memory and fwnode reference.
- [High] If `sun4i_csi_notify_complete` fails after registering the video device, the node is exposed to userspace but not cleaned up on probe abort, leading to a Use-After-Free.
- [Medium] Early error paths in `sun4i_csi_probe` fail to call `media_device_cleanup()`, leaking IDA internal structures and mutexes initialized by `media_device_init()`.
--

commit 0bea5104c6b556049740c1a93c1ab60799f7c769
Author: Biren Pandya <birenpandya@gmail.com>

media: platform: sun4i_csi: Add missing media_entity_cleanup()
    
This commit adds calls to media_entity_cleanup() in the sun4i_csi probe
error paths and the remove function to properly release media entities.

> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> index e53a07b770b7e..f50d97ca795f8 100644
> --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> @@ -242,11 +242,11 @@ static int sun4i_csi_probe(struct platform_device *pdev)
>  	vdev->entity.ops = &sun4i_csi_video_entity_ops;
>  	ret = media_entity_pads_init(&vdev->entity, 1, &csi->vdev_pad);
>  	if (ret < 0)
> -		return ret;
> +		goto err_clean_subdev_entity;
>  
>  	ret = sun4i_csi_dma_register(csi, irq);
>  	if (ret)
> -		goto err_clean_pad;
> +		goto err_clean_vdev_entity;
>  
>  	ret = sun4i_csi_notifier_init(csi);
>  	if (ret)

[ ... ]

> @@ -266,6 +266,10 @@ static int sun4i_csi_probe(struct platform_device *pdev)
>  	media_device_unregister(&csi->mdev);
>  	sun4i_csi_dma_unregister(csi);

[Severity: High]
This isn't a bug introduced by this patch, but does the error path in
sun4i_csi_probe() fail to clean up the V4L2 async notifier, leaking the
async connection memory and fwnode reference?

sun4i_csi_notifier_init() allocates memory for a v4l2_async_connection via
v4l2_async_nf_add_fwnode_remote() and takes a reference to the fwnode. If
v4l2_async_nf_register() subsequently fails, the error path jumps to
err_unregister_media. However, neither this label nor the cascading error
paths below it call v4l2_async_nf_cleanup(&csi->notifier).

[Severity: High]
This is a pre-existing issue, but could this error path lead to a
use-after-free if sun4i_csi_notify_complete() fails after registering
the video device?

In sun4i_csi_notify_complete(), sun4i_csi_v4l2_register() successfully
registers the /dev/videoX node. If a subsequent step (such as
media_device_register() or media_create_pad_link()) fails, the function
returns an error without rolling back and unregistering the video device.

This failure bubbles up, causing v4l2_async_nf_register() in probe to fail.
The probe error path here entirely omits vb2_video_unregister_device().
Consequently, devres frees the csi memory (which embeds csi->vdev), but
the /dev/videoX node remains active. If userspace opens the stranded video
node, the kernel will dereference the freed csi struct via
video_drvdata(file).

> +err_clean_vdev_entity:
> +	media_entity_cleanup(&vdev->entity);
> +err_clean_subdev_entity:
> +	media_entity_cleanup(&subdev->entity);
>  err_clean_pad:
>  	media_device_cleanup(&csi->mdev);

[Severity: Medium]
This is a pre-existing issue, but do early error paths in sun4i_csi_probe()
leak IDA internal structures and mutexes initialized by media_device_init()?

For example, if devm_platform_ioremap_resource() fails:

drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c:sun4i_csi_probe() {
    ...
    media_device_init(&csi->mdev);
    csi->v4l.mdev = &csi->mdev;

    csi->regs = devm_platform_ioremap_resource(pdev, 0);
    if (IS_ERR(csi->regs))
        return PTR_ERR(csi->regs);
    ...
}

It returns directly instead of jumping to the err_clean_pad cleanup label
seen above, bypassing the necessary media_device_cleanup(&csi->mdev) call.
diff mbox series

Patch

diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
index e53a07b770b7..f50d97ca795f 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
@@ -242,11 +242,11 @@  static int sun4i_csi_probe(struct platform_device *pdev)
 	vdev->entity.ops = &sun4i_csi_video_entity_ops;
 	ret = media_entity_pads_init(&vdev->entity, 1, &csi->vdev_pad);
 	if (ret < 0)
-		return ret;
+		goto err_clean_subdev_entity;
 
 	ret = sun4i_csi_dma_register(csi, irq);
 	if (ret)
-		goto err_clean_pad;
+		goto err_clean_vdev_entity;
 
 	ret = sun4i_csi_notifier_init(csi);
 	if (ret)
@@ -266,6 +266,10 @@  static int sun4i_csi_probe(struct platform_device *pdev)
 	media_device_unregister(&csi->mdev);
 	sun4i_csi_dma_unregister(csi);
 
+err_clean_vdev_entity:
+	media_entity_cleanup(&vdev->entity);
+err_clean_subdev_entity:
+	media_entity_cleanup(&subdev->entity);
 err_clean_pad:
 	media_device_cleanup(&csi->mdev);
 
@@ -282,6 +286,8 @@  static void sun4i_csi_remove(struct platform_device *pdev)
 	vb2_video_unregister_device(&csi->vdev);
 	media_device_unregister(&csi->mdev);
 	sun4i_csi_dma_unregister(csi);
+	media_entity_cleanup(&csi->subdev.entity);
+	media_entity_cleanup(&csi->vdev.entity);
 	media_device_cleanup(&csi->mdev);
 }