[PATCH v1 0/2] Input: sur40 - fix UAF/hang on closing the video node after unplug

[PATCH v1 0/2] Input: sur40 - fix UAF/hang on closing the video node after unplug

[Nguyen Ngoc Thang]

Hi,

syzbot reported a slab-use-after-free in vb2_core_queue_release() [1]:
closing /dev/v4l-touch* after the SUR40 was unplugged reads freed memory.

Cause: sur40_disconnect() kfree()s struct sur40_state, which embeds the
video_device, v4l2_device and vb2_queue, even though a video node can
still be open. v4l2_release() and vb2_fop_release() then dereference
freed memory. Patch 2 fixes this by giving the v4l2_device a release()
callback that frees the state, and dropping the disconnect path's
reference with v4l2_device_put(), so the last close does the freeing.
The probe error paths never expose the node and still free directly.

Patch 1 is needed first: once the state outlives disconnect, closing a
node that is still streaming hangs forever, because
sur40_stop_streaming() waits (vb2_wait_for_all_buffers) for buffers
that only the input poll callback completes, and that is gone after
unplug. Returning the queued buffers before the wait fixes it. This was
masked by the UAF above.

Testing: no hardware, so I emulated a SUR40 (045e:0775) with raw-gadget
on dummy_hcd in QEMU with KASAN. The reproducer enumerates the device,
starts a non-blocking read() on the video node (making the fd the queue
owner), disconnects the gadget, then close()s the fd.
  - before: KASAN: slab-use-after-free in v4l2_release(), allocated in
    sur40_probe(), freed in sur40_disconnect() (same alloc/free stacks
    as the syzbot report)
  - patch 1 only: no KASAN, but close() blocks in
    vb2_wait_for_all_buffers()  [only meaningful with patch 2 applied]
  - both patches: 5 consecutive enumerate/disconnect/close cycles, no
    KASAN, no hang.
(The dma_map_sg WARNING during read() in the log comes from dummy_hcd
having no DMA mask; it is unrelated.)

Nguyen Ngoc Thang (2):
  Input: sur40 - don't wait for buffers nothing will complete
  Input: sur40 - keep device state alive until the video node is
    released

 drivers/input/touchscreen/sur40.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

-- 
2.43.0

[PATCH v1 1/2] Input: sur40 - don't wait for buffers nothing will complete

[Nguyen Ngoc Thang]

sur40_stop_streaming() calls vb2_wait_for_all_buffers() before handing
the queued buffers back. Those buffers are only completed from the input
poll callback, which is gone once the device is unplugged. Closing a
video node that is still streaming after a disconnect then sleeps
forever in vb2_wait_for_all_buffers():

  vb2_wait_for_all_buffers+0x20f/0x330
  sur40_stop_streaming+0x45/0x310
  __vb2_queue_cancel+0xc5/0xf70
  vb2_core_streamoff+0x5d/0x180
  __vb2_cleanup_fileio+0x6e/0x190
  vb2_core_queue_release+0x1f/0x190
  _vb2_fop_release+0xe8/0x280
  v4l2_release+0x280/0x430

It has not been noticed so far because sur40_disconnect() frees the
device state under the open file, and the close then crashes earlier.

Return the queued buffers first. A buffer that sur40_process_video()
has already taken off the list still completes by itself, so the wait
afterwards only covers that one.

Fixes: 6a8588156657 ("[media] sur40: fix occasional oopses on device close")
Cc: stable@vger.kernel.org
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
 drivers/input/touchscreen/sur40.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 09d8c5f8d09f..7020bcf9b81a 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -929,11 +929,11 @@ static int sur40_start_streaming(struct vb2_queue *vq, unsigned int count)
 static void sur40_stop_streaming(struct vb2_queue *vq)
 {
 	struct sur40_state *sur40 = vb2_get_drv_priv(vq);
-	vb2_wait_for_all_buffers(vq);
-	sur40->sequence = -1;
 
-	/* Release all active buffers */
+	/* Release queued buffers first: nothing completes them after unplug */
 	return_all_buffers(sur40, VB2_BUF_STATE_ERROR);
+	vb2_wait_for_all_buffers(vq);
+	sur40->sequence = -1;
 }
 
 /* V4L ioctl */
-- 
2.43.0

Re: [PATCH v1 1/2] Input: sur40 - don't wait for buffers nothing will complete

[Hans Verkuil]

On 20/09/2026 13:39, Nguyen Ngoc Thang wrote:
> sur40_stop_streaming() calls vb2_wait_for_all_buffers() before handing
> the queued buffers back. Those buffers are only completed from the input
> poll callback, which is gone once the device is unplugged. Closing a
> video node that is still streaming after a disconnect then sleeps
> forever in vb2_wait_for_all_buffers():
> 
>   vb2_wait_for_all_buffers+0x20f/0x330
>   sur40_stop_streaming+0x45/0x310
>   __vb2_queue_cancel+0xc5/0xf70
>   vb2_core_streamoff+0x5d/0x180
>   __vb2_cleanup_fileio+0x6e/0x190
>   vb2_core_queue_release+0x1f/0x190
>   _vb2_fop_release+0xe8/0x280
>   v4l2_release+0x280/0x430
> 
> It has not been noticed so far because sur40_disconnect() frees the
> device state under the open file, and the close then crashes earlier.
> 
> Return the queued buffers first. A buffer that sur40_process_video()
> has already taken off the list still completes by itself, so the wait
> afterwards only covers that one.
> 
> Fixes: 6a8588156657 ("[media] sur40: fix occasional oopses on device close")
> Cc: stable@vger.kernel.org
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> ---
>  drivers/input/touchscreen/sur40.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index 09d8c5f8d09f..7020bcf9b81a 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
> @@ -929,11 +929,11 @@ static int sur40_start_streaming(struct vb2_queue *vq, unsigned int count)
>  static void sur40_stop_streaming(struct vb2_queue *vq)
>  {
>  	struct sur40_state *sur40 = vb2_get_drv_priv(vq);
> -	vb2_wait_for_all_buffers(vq);
> -	sur40->sequence = -1;
>  
> -	/* Release all active buffers */
> +	/* Release queued buffers first: nothing completes them after unplug */
>  	return_all_buffers(sur40, VB2_BUF_STATE_ERROR);
> +	vb2_wait_for_all_buffers(vq);
> +	sur40->sequence = -1;
>  }
>  
>  /* V4L ioctl */

I think you should make one more change: replace the video_unregister_device calls by
vb2_video_unregister_device(). This ensures that at unregister time all streaming is
canceled and stop_streaming is called.

Having streaming continue after unregistering a device is in general not a good idea,
which is why we added that vb2_video_unregister_device call.

I think you still need to have that change in stop_streaming for this specific driver,
but it might be worth testing without it.

Regards,

	Hans

[PATCH v1 2/2] Input: sur40 - keep device state alive until the video node is released

[Nguyen Ngoc Thang]

sur40_disconnect() frees the sur40_state, which embeds the video_device,
the v4l2_device and the vb2_queue, while a video node may still be open.
Closing that file afterwards touches freed memory:

  BUG: KASAN: slab-use-after-free in vb2_core_queue_release+0x12d/0x150
  Read of size 4 at addr ffff888066fa8aa8 by task v4l_id/28733
   vb2_core_queue_release+0x12d/0x150
   vb2_fop_release+0x16e/0x200
   v4l2_release+0x22c/0x350
   __fput+0x418/0xa50
  Freed by task 16811:
   kfree+0x1c5/0x650
   sur40_disconnect+0xaf/0x140

Give the v4l2_device a release callback that frees the bulk buffer and
the state, and drop the disconnect path's own reference with
v4l2_device_put(). The last closer of the node then does the freeing.
The probe error paths never open the node and keep freeing directly.

Reported-by: syzbot+eb4706daf505f9c4b547@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=eb4706daf505f9c4b547
Fixes: e831cd251fb9 ("[media] add raw video stream support for Samsung SUR40")
Cc: stable@vger.kernel.org
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
 drivers/input/touchscreen/sur40.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 7020bcf9b81a..efd4fe557cef 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -648,6 +648,15 @@ static int sur40_input_setup_events(struct input_dev *input_dev)
 }
 
 /* Check candidate USB interface. */
+/* Runs once the last video node reference is gone. */
+static void sur40_release(struct v4l2_device *v4l2)
+{
+	struct sur40_state *sur40 = container_of(v4l2, struct sur40_state, v4l2);
+
+	kfree(sur40->bulk_in_buffer);
+	kfree(sur40);
+}
+
 static int sur40_probe(struct usb_interface *interface,
 		       const struct usb_device_id *id)
 {
@@ -733,6 +742,7 @@ static int sur40_probe(struct usb_interface *interface,
 			"Unable to register video master device.");
 		goto err_free_buffer;
 	}
+	sur40->v4l2.release = sur40_release;
 
 	/* initialize the lock and subdevice */
 	sur40->queue = sur40_queue;
@@ -831,11 +841,10 @@ static void sur40_disconnect(struct usb_interface *interface)
 	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);
+
+	v4l2_device_put(&sur40->v4l2);
 }
 
 /*
-- 
2.43.0

Re: [PATCH v1 2/2] Input: sur40 - keep device state alive until the video node is released

[Hans Verkuil]

On 20/09/2026 13:39, Nguyen Ngoc Thang wrote:
> sur40_disconnect() frees the sur40_state, which embeds the video_device,
> the v4l2_device and the vb2_queue, while a video node may still be open.
> Closing that file afterwards touches freed memory:
> 
>   BUG: KASAN: slab-use-after-free in vb2_core_queue_release+0x12d/0x150
>   Read of size 4 at addr ffff888066fa8aa8 by task v4l_id/28733
>    vb2_core_queue_release+0x12d/0x150
>    vb2_fop_release+0x16e/0x200
>    v4l2_release+0x22c/0x350
>    __fput+0x418/0xa50
>   Freed by task 16811:
>    kfree+0x1c5/0x650
>    sur40_disconnect+0xaf/0x140
> 
> Give the v4l2_device a release callback that frees the bulk buffer and
> the state, and drop the disconnect path's own reference with
> v4l2_device_put(). The last closer of the node then does the freeing.
> The probe error paths never open the node and keep freeing directly.
> 
> Reported-by: syzbot+eb4706daf505f9c4b547@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=eb4706daf505f9c4b547
> Fixes: e831cd251fb9 ("[media] add raw video stream support for Samsung SUR40")
> Cc: stable@vger.kernel.org
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>

Reviewed-by: Hans Verkuil <hverkuil+cisco@kernel.org>

Regards,

	Hans

> ---
>  drivers/input/touchscreen/sur40.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index 7020bcf9b81a..efd4fe557cef 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
> @@ -648,6 +648,15 @@ static int sur40_input_setup_events(struct input_dev *input_dev)
>  }
>  
>  /* Check candidate USB interface. */
> +/* Runs once the last video node reference is gone. */
> +static void sur40_release(struct v4l2_device *v4l2)
> +{
> +	struct sur40_state *sur40 = container_of(v4l2, struct sur40_state, v4l2);
> +
> +	kfree(sur40->bulk_in_buffer);
> +	kfree(sur40);
> +}
> +
>  static int sur40_probe(struct usb_interface *interface,
>  		       const struct usb_device_id *id)
>  {
> @@ -733,6 +742,7 @@ static int sur40_probe(struct usb_interface *interface,
>  			"Unable to register video master device.");
>  		goto err_free_buffer;
>  	}
> +	sur40->v4l2.release = sur40_release;
>  
>  	/* initialize the lock and subdevice */
>  	sur40->queue = sur40_queue;
> @@ -831,11 +841,10 @@ static void sur40_disconnect(struct usb_interface *interface)
>  	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);
> +
> +	v4l2_device_put(&sur40->v4l2);
>  }
>  
>  /*