diff mbox series

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

Message ID 20260810062521.1709379-4-congnt264@gmail.com (mailing list archive)
State New
Headers
Series media: sun4i-csi: fix probe/streaming lifecycle bugs |

Commit Message

Cong Nguyen Aug. 10, 2026, 6:25 a.m. UTC 
sun4i_csi_notify_ops only implements .bound and .complete. The .bound
callback caches the remote sensor's subdevice in csi->src_subdev. When
that subdevice goes away (e.g. its module is unloaded), the V4L2 async
core frees it, but without an .unbind callback the driver keeps the
stale pointer.

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 clears csi->src_subdev. The pointer is only
dereferenced by v4l2_subdev_call() (NULL-guarded, returns -ENODEV, so
streaming fails cleanly) and by sun4i_csi_notify_complete() (which only
runs while a subdev is bound), so clearing it is sufficient to prevent
the use-after-free.

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. 10, 2026, 6:42 a.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The newly added `.unbind` callback clears `csi->src_subdev` without holding `csi->lock`, creating a time-of-check to time-of-use (TOCTOU) race condition with streaming operations.

Pre-existing issues:
- [High] The `.unbind` callback fails to tear down video and media devices registered in `.complete`, leading to double-registration crashes and resource leaks upon dynamic rebind.
--

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

    media: sun4i-csi: add notifier unbind callback to drop the source subdev
    
    sun4i_csi_notify_ops only implements .bound and .complete. The .bound
    callback caches the remote sensor's subdevice in csi->src_subdev. When
    that subdevice goes away (e.g. its module is unloaded), the V4L2 async
    core frees it, but without an .unbind callback the driver keeps the
    stale pointer.

> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> index a8711336a7540..f19508f2c7ed4 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)
> +{

[Severity: High]
This is a pre-existing issue, but doesn't this callback need to clean up the
video and media devices that were registered in sun4i_csi_notify_complete()?

If the remote sensor module is unbound and then re-bound,
sun4i_csi_notify_complete() will execute again. Since the previous devices
were not unregistered, calling video_register_device() on an already-registered
device can perform a double list_add and corrupt video_device_list.

> +	struct sun4i_csi *csi = container_of(notifier, struct sun4i_csi,
> +					     notifier);
> +
> +	/*
> +	 * The remote subdev is being freed. Drop our cached pointer so that a
> +	 * subsequent sun4i_csi_start_streaming() does not issue a
> +	 * v4l2_subdev_call() on the now dangling source subdev. The call is
> +	 * NULL-guarded and returns -ENODEV, so streaming fails cleanly.
> +	 */
> +	csi->src_subdev = NULL;

[Severity: High]
Could this introduce a time-of-check to time-of-use race condition with
streaming operations?

This clears csi->src_subdev without holding csi->lock. Meanwhile, functions
like sun4i_csi_start_streaming() and sun4i_csi_stop_streaming() in sun4i_dma.c
evaluate csi->src_subdev via v4l2_subdev_call() while holding csi->lock.

If the streamon ioctl enters sun4i_csi_start_streaming(), v4l2_subdev_call()
will cache the csi->src_subdev pointer into a local variable. If the remote
sensor module is concurrently unloaded, sun4i_csi_notify_unbind() will clear
csi->src_subdev without acquiring the lock and return, allowing the async core
to free the subdevice. The streaming thread could then resume and dereference
the freed subdevice pointer.

> +}
> +
>  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..f19508f2c7ed 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. Drop our cached pointer so that a
+	 * subsequent sun4i_csi_start_streaming() does not issue a
+	 * v4l2_subdev_call() on the now dangling source subdev. The call is
+	 * NULL-guarded and returns -ENODEV, so streaming fails cleanly.
+	 */
+	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,
 };