Re: [PATCH v2 1/3] media: sun4i-csi: fix video device and subdev leak in notify_complete()

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

Pre-existing issues:
- [High] Race Condition / Use-After-Free via TOCTOU during probe failure
--

--- Patch [1]: [PATCH v2 1/3] media: sun4i-csi: fix video device and subdev leak in notify_complete() ---
commit 5302a06684e2be656ddb94e9e33fdbca0536135f
Author: Cong Nguyen <[email protected]>
media: sun4i-csi: fix video device and subdev leak in notify_complete()

This commit fixes resource leaks in sun4i_csi_notify_complete() by unwinding
registrations in reverse order on error paths. It explicitly unregisters the
video device and bridge subdev if intermediate registration steps fail.

Cc: [email protected]

> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
[ ... ]
> @@ -114,6 +114,10 @@ static int sun4i_csi_notify_complete(struct v4l2_async_notifier *notifier)
>  
>  err_clean_media:
>  	media_device_unregister(&csi->mdev);
> +err_unregister_video:
> +	vb2_video_unregister_device(&csi->vdev);

[Severity: High]
This is a pre-existing issue, but does this error path leave a use-after-free
vulnerability if userspace opens the device node before it gets unregistered?

The video_device is embedded in struct sun4i_csi which is allocated with
devm_kzalloc() during sun4i_csi_probe(). In sun4i_csi_v4l2_register(), the
release callback is overridden:

vdev->release = video_device_release_empty;

If sun4i_csi_notify_complete() executes synchronously during probe and exposes
the device node, userspace can open it immediately. If a subsequent setup step
fails, this error path unregisters the video device and returns the error back
up to probe().

When probe() fails, the devres core automatically frees the sun4i_csi memory.
Because video_device_release_empty is used instead of a refcounted release,
any active file descriptors held by userspace will now point to freed memory,
causing a use-after-free on subsequent access or close.

> +err_unregister_subdev:
> +	v4l2_device_unregister_subdev(subdev);
>  
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1