[PATCH] input: sur40: fix use-after-free in disconnect

[PATCH] input: sur40: fix use-after-free in disconnect

[Pei Xiao]

When the USB device is disconnected while userspace still holds
an open file descriptor to the V4L2 video device, sur40_disconnect()
calls kfree(sur40) immediately after video_unregister_device(). However,
video_unregister_device() only removes the device from the V4L2
framework and does not drop the kref. Userspace still holds a valid
reference to the embedded video_device, causing a use-after-free when
subsequently performing ioctls that access vdev->flags via
video_is_registered().

Fix this by setting vdev->release to a new callback sur40_video_release()
that frees the containing sur40_state via container_of(). Replace the
direct kfree(sur40) in sur40_disconnect() with a reliance on the kref
mechanism: video_unregister_device() triggers device_unregister() which
drops the kref on vdev->dev; when the last reference is released
(all userspace file descriptors closed), the V4L2 core automatically
calls vdev->release(), safely freeing sur40_state.

Also fix the error path err_unreg_video to properly free the
independently-allocated input device and return early, preventing
fall-through to cleanup labels that would access the already-freed
sur40_state.

Fixes: e831cd251fb9 ("[media] add raw video stream support for Samsung SUR40")
Reported-by: syzbot+4a6e6173b1fc7916e950@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/6a8e44ca.dbb3a75c.7844.002f.GAE@google.com/
Cc: stable@vger.kernel.org
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/input/touchscreen/sur40.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 09d8c5f8d09f..2d93234cc86a 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -237,6 +237,7 @@ static const struct video_device sur40_video_device;
 static const struct vb2_queue sur40_queue;
 static void sur40_process_video(struct sur40_state *sur40);
 static int sur40_s_ctrl(struct v4l2_ctrl *ctrl);
+static void sur40_video_release(struct video_device *vdev);
 
 static const struct v4l2_ctrl_ops sur40_ctrl_ops = {
 	.s_ctrl = sur40_s_ctrl,
@@ -750,6 +751,7 @@ static int sur40_probe(struct usb_interface *interface,
 	sur40->vdev.v4l2_dev = &sur40->v4l2;
 	sur40->vdev.lock = &sur40->lock;
 	sur40->vdev.queue = &sur40->queue;
+	sur40->vdev.release = sur40_video_release;
 	video_set_drvdata(&sur40->vdev, sur40);
 
 	/* initialize the control handler for 4 controls */
@@ -806,6 +808,8 @@ static int sur40_probe(struct usb_interface *interface,
 
 err_unreg_video:
 	video_unregister_device(&sur40->vdev);
+	input_free_device(input);
+	return error;
 err_free_ctrl:
 	v4l2_ctrl_handler_free(&sur40->hdl);
 err_unreg_v4l2:
@@ -820,19 +824,23 @@ static int sur40_probe(struct usb_interface *interface,
 	return error;
 }
 
-/* Unregister device & clean up. */
+static void sur40_video_release(struct video_device *vdev)
+{
+	struct sur40_state *sur40 = container_of(vdev, struct sur40_state, vdev);
+
+	v4l2_ctrl_handler_free(&sur40->hdl);
+	v4l2_device_unregister(&sur40->v4l2);
+	kfree(sur40->bulk_in_buffer);
+	kfree(sur40);
+}
+
 static void sur40_disconnect(struct usb_interface *interface)
 {
 	struct sur40_state *sur40 = usb_get_intfdata(interface);
 
 	input_unregister_device(sur40->input);
 
-	v4l2_ctrl_handler_free(&sur40->hdl);
 	video_unregister_device(&sur40->vdev);
-	v4l2_device_unregister(&sur40->v4l2);
-
-	kfree(sur40->bulk_in_buffer);
-	kfree(sur40);
 
 	usb_set_intfdata(interface, NULL);
 	dev_dbg(&interface->dev, "%s is now disconnected\n", DRIVER_DESC);
-- 
2.25.1

[PATCH] input: sur40: fix use-after-free in disconnect

[Pei Xiao]

When the USB device is disconnected while userspace still holds
an open file descriptor to the V4L2 video device, sur40_disconnect()
calls kfree(sur40) immediately after video_unregister_device(). However,
video_unregister_device() only removes the device from the V4L2
framework and does not drop the kref. Userspace still holds a valid
reference to the embedded video_device, causing a use-after-free when
subsequently performing ioctls that access vdev->flags via
video_is_registered().

Fix this by setting vdev->release to a new callback sur40_video_release()
that frees the containing sur40_state via container_of(). Replace the
direct kfree(sur40) in sur40_disconnect() with a reliance on the kref
mechanism: video_unregister_device() triggers device_unregister() which
drops the kref on vdev->dev; when the last reference is released
(all userspace file descriptors closed), the V4L2 core automatically
calls vdev->release(), safely freeing sur40_state.

Also fix the error path err_unreg_video to properly free the
independently-allocated input device and return early, preventing
fall-through to cleanup labels that would access the already-freed
sur40_state.

Fixes: e831cd251fb9 ("[media] add raw video stream support for Samsung SUR40")
Reported-by: syzbot+4a6e6173b1fc7916e950@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/6a8e44ca.dbb3a75c.7844.002f.GAE@google.com/
Cc: stable@vger.kernel.org
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/input/touchscreen/sur40.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 09d8c5f8d09f..2d93234cc86a 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -237,6 +237,7 @@ static const struct video_device sur40_video_device;
 static const struct vb2_queue sur40_queue;
 static void sur40_process_video(struct sur40_state *sur40);
 static int sur40_s_ctrl(struct v4l2_ctrl *ctrl);
+static void sur40_video_release(struct video_device *vdev);
 
 static const struct v4l2_ctrl_ops sur40_ctrl_ops = {
 	.s_ctrl = sur40_s_ctrl,
@@ -750,6 +751,7 @@ static int sur40_probe(struct usb_interface *interface,
 	sur40->vdev.v4l2_dev = &sur40->v4l2;
 	sur40->vdev.lock = &sur40->lock;
 	sur40->vdev.queue = &sur40->queue;
+	sur40->vdev.release = sur40_video_release;
 	video_set_drvdata(&sur40->vdev, sur40);
 
 	/* initialize the control handler for 4 controls */
@@ -806,6 +808,8 @@ static int sur40_probe(struct usb_interface *interface,
 
 err_unreg_video:
 	video_unregister_device(&sur40->vdev);
+	input_free_device(input);
+	return error;
 err_free_ctrl:
 	v4l2_ctrl_handler_free(&sur40->hdl);
 err_unreg_v4l2:
@@ -820,19 +824,23 @@ static int sur40_probe(struct usb_interface *interface,
 	return error;
 }
 
-/* Unregister device & clean up. */
+static void sur40_video_release(struct video_device *vdev)
+{
+	struct sur40_state *sur40 = container_of(vdev, struct sur40_state, vdev);
+
+	v4l2_ctrl_handler_free(&sur40->hdl);
+	v4l2_device_unregister(&sur40->v4l2);
+	kfree(sur40->bulk_in_buffer);
+	kfree(sur40);
+}
+
 static void sur40_disconnect(struct usb_interface *interface)
 {
 	struct sur40_state *sur40 = usb_get_intfdata(interface);
 
 	input_unregister_device(sur40->input);
 
-	v4l2_ctrl_handler_free(&sur40->hdl);
 	video_unregister_device(&sur40->vdev);
-	v4l2_device_unregister(&sur40->v4l2);
-
-	kfree(sur40->bulk_in_buffer);
-	kfree(sur40);
 
 	usb_set_intfdata(interface, NULL);
 	dev_dbg(&interface->dev, "%s is now disconnected\n", DRIVER_DESC);
-- 
2.25.1

Re: [PATCH] input: sur40: fix use-after-free in disconnect

[sashiko-bot]

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

Pre-existing issues:
- [High] Unkillable deadlock in `sur40_stop_streaming` during disconnect.
- [High] Hang in VIDIOC_DQBUF or poll due to missing queue abort in disconnect.
--

commit 8870e5b6bd950af7749d533c844ef1b23fd8c7bb
Author: Pei Xiao <xiaopei01@kylinos.cn>
Date:   Wed Aug 26 03:42:39 2026 +0000

    input: sur40: fix use-after-free in disconnect

This commit fixes a use-after-free in sur40_disconnect() by replacing the
direct kfree() with a new vdev->release callback. This ensures the sur40_state
structure is only freed when the last userspace reference is released. It also
adjusts the error paths in sur40_probe() to prevent accessing freed memory.

Reported-by: syzbot+4a6e6173b1fc7916e950@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/6a8e44ca.dbb3a75c.7844.002f.GAE@google.com/
Cc: stable@vger.kernel.org

> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index 09d8c5f8d09fc..2d93234cc86a0 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
[ ... ]
> @@ -820,19 +824,23 @@ static int sur40_probe(struct usb_interface *interface,
>  	return error;
>  }
>  
> -/* Unregister device & clean up. */
> +static void sur40_video_release(struct video_device *vdev)
> +{
[ ... ]
> +}
> +
>  static void sur40_disconnect(struct usb_interface *interface)
>  {
>  	struct sur40_state *sur40 = usb_get_intfdata(interface);
>  
>  	input_unregister_device(sur40->input);
>  
> -	v4l2_ctrl_handler_free(&sur40->hdl);
>  	video_unregister_device(&sur40->vdev);

[Severity: High]
This isn't a bug introduced by this patch, but is it possible that
unregistering the video device without aborting the pending V4L2 queue
leaves blocked ioctls waiting forever?

When the device is disconnected, video_unregister_device() is called here
without first waking up q->done_wq waiters with -EIO (e.g. by using
vb2_video_unregister_device()). 

Because the input poller is stopped synchronously by input_unregister_device()
just above, no further frames will arrive. 

Does this cause any thread blocked in VIDIOC_DQBUF or poll() to hang forever
until interrupted by a signal?


[Severity: High]
This is also a pre-existing issue, but does this disconnect sequence create an
unkillable deadlock later on?

If userspace closes the device after a disconnect, the V4L2 core invokes
sur40_stop_streaming(), which calls:

    vb2_wait_for_all_buffers(vq);
    sur40->sequence = -1;

Since input_unregister_device() was already called synchronously in
sur40_disconnect(), the input polling thread is permanently stopped. 

Does this cause vb2_wait_for_all_buffers() to block indefinitely in a
D-state, since the driver-owned buffers will never be processed and returned
via vb2_buffer_done()?

> -	v4l2_device_unregister(&sur40->v4l2);
> -
> -	kfree(sur40->bulk_in_buffer);
> -	kfree(sur40);
>  
>  	usb_set_intfdata(interface, NULL);
>  	dev_dbg(&interface->dev, "%s is now disconnected\n", DRIVER_DESC);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826034239.QMESl9TtJLm8R_oB5SkE5YErhpU_WDuou8ER3A1oZmI@z?part=1

Re: [PATCH] input: sur40: fix use-after-free in disconnect

[sashiko-bot]

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

Pre-existing issues:
- [High] Calling vb2_wait_for_all_buffers() in sur40_stop_streaming() can cause a permanent deadlock if the input device is not opened or if the USB device is disconnected while streaming.
- [High] Failing to use vb2_video_unregister_device() causes blocked video streams to hang forever, preventing memory release.
--

commit ef5f3b0676d23060459c7e109fbbbad456afb073
Author: Pei Xiao <xiaopei01@kylinos.cn>
input: sur40: fix use-after-free in disconnect

This commit fixes a use-after-free issue in sur40_disconnect() by providing
a proper release callback for the video device. It relies on the kref
mechanism triggered by device unregistration rather than immediately
freeing the state.

Closes: https://lore.kernel.org/all/6a8e44ca.dbb3a75c.7844.002f.GAE@google.com/

> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index 09d8c5f8d09fc..2d93234cc86a0 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c

[ ... ]

> @@ -820,19 +824,23 @@ static int sur40_probe(struct usb_interface *interface,
>  	return error;
>  }
>  
> -/* Unregister device & clean up. */
> +static void sur40_video_release(struct video_device *vdev)
> +{
> +	struct sur40_state *sur40 = container_of(vdev, struct sur40_state, vdev);
> +
> +	v4l2_ctrl_handler_free(&sur40->hdl);
> +	v4l2_device_unregister(&sur40->v4l2);
> +	kfree(sur40->bulk_in_buffer);
> +	kfree(sur40);
> +}
> +
>  static void sur40_disconnect(struct usb_interface *interface)
>  {
>  	struct sur40_state *sur40 = usb_get_intfdata(interface);
>  
>  	input_unregister_device(sur40->input);
>  
> -	v4l2_ctrl_handler_free(&sur40->hdl);
>  	video_unregister_device(&sur40->vdev);

[Severity: High]
This is a pre-existing issue, but does failing to use
vb2_video_unregister_device() here at line 843 of
drivers/input/touchscreen/sur40.c cause blocked video streams to hang forever?

During a physical USB disconnect, sur40_disconnect() calls
video_unregister_device() instead of vb2_video_unregister_device(). The
standard video_unregister_device() does not signal the vb2_queue to abort.

Because the input poller is stopped during disconnect, no new video frames
will arrive. Any userspace thread blocking in VIDIOC_DQBUF or poll() will
never wake up, preventing it from closing the file descriptor. This means
vdev->release is never executed and the sur40_state memory leaks.

[Severity: High]
This is also a pre-existing issue, but can calling
vb2_wait_for_all_buffers() in sur40_stop_streaming() cause a permanent
deadlock?

If the USB device is disconnected, input_unregister_device() stops the
poller. A subsequent close() of the video file descriptor will call
sur40_stop_streaming() (at line 940 in drivers/input/touchscreen/sur40.c),
which blocks waiting for buffers:

sur40_stop_streaming()
    ...
    vb2_wait_for_all_buffers(vq);

Since the poller (sur40_poll at line 525) is stopped, the buffers are never
returned, causing a hang in uninterruptible sleep. This can also be
triggered if a user opens only the video device and starts streaming without
opening the input device, as sur40_poll is only active while the input
device is opened.

> -	v4l2_device_unregister(&sur40->v4l2);
> -
> -	kfree(sur40->bulk_in_buffer);
> -	kfree(sur40);
>  
>  	usb_set_intfdata(interface, NULL);
>  	dev_dbg(&interface->dev, "%s is now disconnected\n", DRIVER_DESC);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/51e68e6de6d9e7a864a5729bac1b33bd083802f6.1787715605.git.xiaopei01@kylinos.cn?part=1

Re: [PATCH] input: sur40: fix use-after-free in disconnect

[Pei Xiao]

在 2026/8/26 11:42, Pei Xiao 写道:
> When the USB device is disconnected while userspace still holds
> an open file descriptor to the V4L2 video device, sur40_disconnect()
> calls kfree(sur40) immediately after video_unregister_device(). However,
> video_unregister_device() only removes the device from the V4L2
> framework and does not drop the kref. Userspace still holds a valid
> reference to the embedded video_device, causing a use-after-free when
> subsequently performing ioctls that access vdev->flags via
> video_is_registered().
> 
> Fix this by setting vdev->release to a new callback sur40_video_release()
> that frees the containing sur40_state via container_of(). Replace the
> direct kfree(sur40) in sur40_disconnect() with a reliance on the kref
> mechanism: video_unregister_device() triggers device_unregister() which
> drops the kref on vdev->dev; when the last reference is released
> (all userspace file descriptors closed), the V4L2 core automatically
> calls vdev->release(), safely freeing sur40_state.
> 
> Also fix the error path err_unreg_video to properly free the
> independently-allocated input device and return early, preventing
> fall-through to cleanup labels that would access the already-freed
> sur40_state.
> 
> Fixes: e831cd251fb9 ("[media] add raw video stream support for Samsung SUR40")
> Reported-by: syzbot+4a6e6173b1fc7916e950@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/all/6a8e44ca.dbb3a75c.7844.002f.GAE@google.com/
> Cc: stable@vger.kernel.org
> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
> ---
>  drivers/input/touchscreen/sur40.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index 09d8c5f8d09f..2d93234cc86a 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
> @@ -237,6 +237,7 @@ static const struct video_device sur40_video_device;
>  static const struct vb2_queue sur40_queue;
>  static void sur40_process_video(struct sur40_state *sur40);
>  static int sur40_s_ctrl(struct v4l2_ctrl *ctrl);
> +static void sur40_video_release(struct video_device *vdev);
>  
>  static const struct v4l2_ctrl_ops sur40_ctrl_ops = {
>  	.s_ctrl = sur40_s_ctrl,
> @@ -750,6 +751,7 @@ static int sur40_probe(struct usb_interface *interface,
>  	sur40->vdev.v4l2_dev = &sur40->v4l2;
>  	sur40->vdev.lock = &sur40->lock;
>  	sur40->vdev.queue = &sur40->queue;
> +	sur40->vdev.release = sur40_video_release;
>  	video_set_drvdata(&sur40->vdev, sur40);
>  
>  	/* initialize the control handler for 4 controls */
> @@ -806,6 +808,8 @@ static int sur40_probe(struct usb_interface *interface,
>  
>  err_unreg_video:
>  	video_unregister_device(&sur40->vdev);
> +	input_free_device(input);
> +	return error;
>  err_free_ctrl:
>  	v4l2_ctrl_handler_free(&sur40->hdl);
>  err_unreg_v4l2:
> @@ -820,19 +824,23 @@ static int sur40_probe(struct usb_interface *interface,
>  	return error;
>  }
>  
> -/* Unregister device & clean up. */
> +static void sur40_video_release(struct video_device *vdev)
> +{
> +	struct sur40_state *sur40 = container_of(vdev, struct sur40_state, vdev);
> +
> +	v4l2_ctrl_handler_free(&sur40->hdl);
> +	v4l2_device_unregister(&sur40->v4l2);
> +	kfree(sur40->bulk_in_buffer);
> +	kfree(sur40);
> +}
> +
>  static void sur40_disconnect(struct usb_interface *interface)
>  {
>  	struct sur40_state *sur40 = usb_get_intfdata(interface);
>  
>  	input_unregister_device(sur40->input);
>  
> -	v4l2_ctrl_handler_free(&sur40->hdl);
>  	video_unregister_device(&sur40->vdev);
> -	v4l2_device_unregister(&sur40->v4l2);
> -
> -	kfree(sur40->bulk_in_buffer);
> -	kfree(sur40);
>  
>  	usb_set_intfdata(interface, NULL);
>  	dev_dbg(&interface->dev, "%s is now disconnected\n", DRIVER_DESC);