diff mbox series

[v1,3/3] media: sun4i-csi: add notifier unbind callback to drop the source subdev

Message ID 61d4901af20a4d2d0f9484328c173bbdfc52ec05.1786184456.git.congnt264@gmail.com (mailing list archive)
State New
Headers
Series media: sun4i-csi: fix lifecycle bugs (leak, IRQ, unbind) |

Commit Message

Cong Nguyen Aug. 8, 2026, 11:17 a.m. UTC 
sun4i_csi_notify_ops only implements .bound and .complete. When the remote
sensor's subdevice goes away (e.g. its module is unloaded), the V4L2 async
core unbinds and frees it, but the driver keeps the stale pointer in
csi->src_subdev and leaves the video node registered.

A subsequent VIDIOC_STREAMON reaches sun4i_csi_start_streaming(), which
calls v4l2_subdev_call(csi->src_subdev, video, s_stream, 1) on the freed
subdev, resulting in a use-after-free.

Add an .unbind callback that unregisters the video device so userspace can
no longer start streaming, and clears csi->src_subdev. Unregistering the
already-unregistered video device again in sun4i_csi_remove() is harmless
(vb2_video_unregister_device() is a no-op when it is not registered).

Fixes: 577bbf23b758 ("media: sunxi: Add A10 CSI driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4
Signed-off-by: Cong Nguyen <congnt264@gmail.com>
---
 .../media/platform/sunxi/sun4i-csi/sun4i_csi.c  | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

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

New issues:
- [High] The driver attempts to re-register an embedded `video_device` after it has been unregistered, which violates kobject lifecycle rules.

Pre-existing issues:
- [High] The `.unbind` callback performs incomplete resource teardown, leading to list corruption on re-bind.
- [High] The `sun4i_csi` structure is allocated using `devm_kzalloc`, but its embedded `video_device` relies on `video_device_release_empty`, creating a Use-After-Free.
--

--- Patch [3]: [PATCH v1 3/3] media: sun4i-csi: add notifier unbind callback to drop the source subdev ---
commit 682f5e1386d2733cf9627a62093a8bdd3a593a37
Author: Cong Nguyen <congnt264@gmail.com>

media: sun4i-csi: add notifier unbind callback to drop the source subdev

This commit adds an .unbind callback to the V4L2 async notifier for the
sun4i-csi driver. It unregisters the video device and drops the source
subdev pointer. This is intended to prevent a use-after-free when the
remote sensor module is unloaded.

> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> index a8711336a7540..6610ada1c06d1 100644
> --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> @@ -122,8 +122,25 @@ static int sun4i_csi_notify_complete(struct v4l2_async_notifier *notifier)
>  	return ret;
>  }
>  
> +static void sun4i_csi_notify_unbind(struct v4l2_async_notifier *notifier,
> +				    struct v4l2_subdev *subdev,
> +				    struct v4l2_async_connection *asd)
> +{
> +	struct sun4i_csi *csi = container_of(notifier, struct sun4i_csi,
> +					     notifier);
> +
> +	/*
> +	 * The remote subdev is being freed. Tear down the video node so
> +	 * userspace can no longer reach sun4i_csi_start_streaming() and
> +	 * dereference the now dangling source subdev, and drop the pointer.
> +	 */
> +	vb2_video_unregister_device(&csi->vdev);

[Severity: High]
This is a pre-existing issue, but the new .unbind callback performs an 
asymmetric teardown.

If the remote sensor module is unloaded, the .unbind callback is triggered 
but fails to undo the local subdev and media device registrations. When the 
sensor is reloaded, sun4i_csi_notify_complete() runs again and calls 
v4l2_device_register_subdev() on the already-registered subdev.

Can this lead to list corruption and a potential kernel panic?


[Severity: High]
When this unbind callback unregisters the embedded video device (csi->vdev),
it ends the lifecycle of its kobject.

When the sensor is re-bound, sun4i_csi_notify_complete() invokes 
sun4i_csi_v4l2_register(), which calls video_register_device() on the 
same unregistered, embedded video_device. Re-initializing a released 
kobject is forbidden and can lead to kernel warnings and potential panics.

Can the driver be restructured to avoid re-registering an embedded video
device that has already been unregistered?


[Severity: High]
This is a pre-existing issue, but the sun4i_csi structure is allocated using 
devm_kzalloc() in sun4i_csi_probe(), while its embedded video_device 
relies on video_device_release_empty.

If an unprivileged user opens /dev/videoX and the driver is unbound, 
the video device is unregistered. This eventually causes devres to free the 
sun4i_csi structure. When the user later closes the file descriptor, 
sun4i_csi_release() accesses the freed csi pointer to lock csi->lock.

Does this create a Use-After-Free condition when the file descriptor is 
closed after the driver is unbound?

> +	csi->src_subdev = NULL;
> +}
> +
>  static const struct v4l2_async_notifier_operations sun4i_csi_notify_ops = {
>  	.bound		= sun4i_csi_notify_bound,
> +	.unbind		= sun4i_csi_notify_unbind,
>  	.complete	= sun4i_csi_notify_complete,
>  };
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 a8711336a754..6610ada1c06d 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
@@ -122,8 +122,25 @@  static int sun4i_csi_notify_complete(struct v4l2_async_notifier *notifier)
 	return ret;
 }
 
+static void sun4i_csi_notify_unbind(struct v4l2_async_notifier *notifier,
+				    struct v4l2_subdev *subdev,
+				    struct v4l2_async_connection *asd)
+{
+	struct sun4i_csi *csi = container_of(notifier, struct sun4i_csi,
+					     notifier);
+
+	/*
+	 * The remote subdev is being freed. Tear down the video node so
+	 * userspace can no longer reach sun4i_csi_start_streaming() and
+	 * dereference the now dangling source subdev, and drop the pointer.
+	 */
+	vb2_video_unregister_device(&csi->vdev);
+	csi->src_subdev = NULL;
+}
+
 static const struct v4l2_async_notifier_operations sun4i_csi_notify_ops = {
 	.bound		= sun4i_csi_notify_bound,
+	.unbind		= sun4i_csi_notify_unbind,
 	.complete	= sun4i_csi_notify_complete,
 };