[PATCH 1/4] Input: sur40 - fix input device registration ordering

[PATCH 1/4] Input: sur40 - fix input device registration ordering

[Dmitry Torokhov]

In sur40_probe(), input_register_device() was previously called early before
the V4L2 video device and vb2_queue components were fully initialized. If
userspace opened the input device immediately upon registration, sur40_open()
would trigger and start the sur40_poll() worker thread. This worker thread
invokes sur40_process_video() and accesses the uninitialized vb2_queue
structure, leading to a data race and potential system crash.

Furthermore, if V4L2 or video registration failed after input_register_device()
succeeded, the error path fell through to calling input_free_device() on a
successfully registered device instead of input_unregister_device(), corrupting
input core state.

Move input_register_device() to the very end of sur40_probe(). This ensures
the V4L2 and video queue structures are fully initialized before polling can
start, and naturally resolves the error path bug since input_free_device()
is now only called when input registration has not yet occurred.

To maintain strict LIFO (Last-In, First-Out) teardown ordering, also move
input_unregister_device() to the very beginning of sur40_disconnect(). This
guarantees that the input polling worker thread is stopped before V4L2
video components or control handlers are unregistered.

Reported-by: sashiko-bot@kernel.org
Cc: stable@vger.kernel.org
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/sur40.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index fe63d53d56db..8639ec3ad703 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -725,21 +725,13 @@ static int sur40_probe(struct usb_interface *interface,
 		goto err_free_input;
 	}
 
-	/* register the polled input device */
-	error = input_register_device(input);
-	if (error) {
-		dev_err(&interface->dev,
-			"Unable to register polled input device.");
-		goto err_free_buffer;
-	}
-
 	/* register the video master device */
 	snprintf(sur40->v4l2.name, sizeof(sur40->v4l2.name), "%s", DRIVER_LONG);
 	error = v4l2_device_register(sur40->dev, &sur40->v4l2);
 	if (error) {
 		dev_err(&interface->dev,
 			"Unable to register video master device.");
-		goto err_unreg_v4l2;
+		goto err_free_buffer;
 	}
 
 	/* initialize the lock and subdevice */
@@ -798,6 +790,14 @@ static int sur40_probe(struct usb_interface *interface,
 		goto err_unreg_video;
 	}
 
+	/* register the polled input device */
+	error = input_register_device(input);
+	if (error) {
+		dev_err(&interface->dev,
+			"Unable to register polled input device.");
+		goto err_unreg_video;
+	}
+
 	/* we can register the device now, as it is ready */
 	usb_set_intfdata(interface, sur40);
 	dev_dbg(&interface->dev, "%s is now attached\n", DRIVER_DESC);
@@ -823,11 +823,12 @@ 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);
 
-	input_unregister_device(sur40->input);
 	kfree(sur40->bulk_in_buffer);
 	kfree(sur40);
 
-- 
2.54.0.1136.gdb2ca164c4-goog

[PATCH 2/4] Input: sur40 - fix V4L error path cleanup

[Dmitry Torokhov]

In sur40_probe(), if video_register_device() fails, the error path jumps to
err_unreg_video. This incorrectly attempts to unregister a video device
that was never successfully registered, and fails to free the V4L2 control
handler (v4l2_ctrl_handler_free) that was initialized immediately prior.

Fix this by introducing an err_free_ctrl label to properly free the V4L2
control handler and bypass video_unregister_device() when video device
registration fails.

Reported-by: sashiko-bot@kernel.org
Cc: stable@vger.kernel.org
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/sur40.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 8639ec3ad703..e9089b0c3e2f 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -787,7 +787,7 @@ static int sur40_probe(struct usb_interface *interface,
 	if (error) {
 		dev_err(&interface->dev,
 			"Unable to register video subdevice.");
-		goto err_unreg_video;
+		goto err_free_ctrl;
 	}
 
 	/* register the polled input device */
@@ -806,6 +806,8 @@ static int sur40_probe(struct usb_interface *interface,
 
 err_unreg_video:
 	video_unregister_device(&sur40->vdev);
+err_free_ctrl:
+	v4l2_ctrl_handler_free(&sur40->hdl);
 err_unreg_v4l2:
 	v4l2_device_unregister(&sur40->v4l2);
 err_free_buffer:
-- 
2.54.0.1136.gdb2ca164c4-goog

Re: [PATCH 2/4] Input: sur40 - fix V4L error path cleanup

[sashiko-bot]

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

Pre-existing issues:
- [High] Calling `vb2_wait_for_all_buffers` in `sur40_stop_streaming` causes a system deadlock.
--

commit 1d8742f82651a7f0160c799526e70838d3c9fa29
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

    Input: sur40 - fix V4L error path cleanup
    
    In sur40_probe(), if video_register_device() fails, the error path jumps to
    err_unreg_video. This incorrectly attempts to unregister a video device
    that was never successfully registered, and fails to free the V4L2 control
    handler (v4l2_ctrl_handler_free) that was initialized immediately prior.

> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index 8639ec3ad7038..e9089b0c3e2f4 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but can the current implementation of
sur40_stop_streaming() cause a system deadlock?

Looking at drivers/input/touchscreen/sur40.c:sur40_stop_streaming():

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

	/* Release all active buffers */
	return_all_buffers(sur40, VB2_BUF_STATE_ERROR);

The driver unconditionally calls vb2_wait_for_all_buffers() before forcibly
returning the active buffers. This blocks infinitely until all queued buffers
are returned via vb2_buffer_done().

The driver expects the poller thread (sur40_poll) to process and return these
buffers. Is it possible for the poller thread to be stopped or not running
when this is called?

For instance, if userspace opens only /dev/videoX without ever opening
/dev/input/eventX, the input-bound poller never starts.

Additionally, if the device is abruptly unplugged while streaming is active,
sur40_disconnect() calls input_unregister_device(), which synchronously stops
the poller. When the video file descriptor is subsequently closed,
sur40_stop_streaming() is triggered.

In both of these scenarios, won't vb2_wait_for_all_buffers() wait forever for
buffers to be returned by a thread that is not running? Should the driver
synchronously cancel transfers and immediately return all queued buffers with
VB2_BUF_STATE_ERROR instead of waiting?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616051235.1549517-1-dmitry.torokhov@gmail.com?part=2

[PATCH 3/4] Input: sur40 - factor out and move input device initialization

[Dmitry Torokhov]

The input device allocation, setup, and registration in sur40_probe() is
quite verbose. Factor it out into a helper function sur40_init_input() to
improve readability.

Additionally, call this helper at the very end of sur40_probe() instead of
allocating the input device early. This ensures all video components are fully
initialized before the input device is registered (which starts polling),
and simplifies the early probe error paths since we don't have to carry and
free the input device if probe fails during early V4L2 setup.

Reported-by: sashiko-bot@kernel.org
Cc: stable@vger.kernel.org
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/sur40.c | 91 +++++++++++++++++--------------
 1 file changed, 50 insertions(+), 41 deletions(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index e9089b0c3e2f..1ad68131e3a6 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -647,6 +647,53 @@ static int sur40_input_setup_events(struct input_dev *input_dev)
 	return 0;
 }
 
+static int sur40_init_input(struct sur40_state *sur40)
+{
+	struct input_dev *input;
+	int error;
+
+	input = input_allocate_device();
+	if (!input)
+		return -ENOMEM;
+
+	/* Set up regular input device structure */
+	input->name = DRIVER_LONG;
+	usb_to_input_id(sur40->usbdev, &input->id);
+	usb_make_path(sur40->usbdev, sur40->phys, sizeof(sur40->phys));
+	strlcat(sur40->phys, "/input0", sizeof(sur40->phys));
+	input->phys = sur40->phys;
+	input->dev.parent = sur40->dev;
+
+	input->open = sur40_open;
+	input->close = sur40_close;
+
+	error = sur40_input_setup_events(input);
+	if (error)
+		goto err_free_input;
+
+	input_set_drvdata(input, sur40);
+	error = input_setup_polling(input, sur40_poll);
+	if (error) {
+		dev_err(sur40->dev, "failed to set up polling\n");
+		goto err_free_input;
+	}
+
+	input_set_poll_interval(input, POLL_INTERVAL);
+
+	error = input_register_device(input);
+	if (error) {
+		dev_err(sur40->dev, "Unable to register polled input device.\n");
+		goto err_free_input;
+	}
+
+	sur40->input = input;
+	return 0;
+
+err_free_input:
+	input_free_device(input);
+	return error;
+}
+
 /* Check candidate USB interface. */
 static int sur40_probe(struct usb_interface *interface,
 		       const struct usb_device_id *id)
@@ -655,7 +702,6 @@ static int sur40_probe(struct usb_interface *interface,
 	struct sur40_state *sur40;
 	struct usb_host_interface *iface_desc;
 	struct usb_endpoint_descriptor *endpoint;
-	struct input_dev *input;
 	int error;
 
 	/* Check if we really have the right interface. */
@@ -676,44 +722,13 @@ static int sur40_probe(struct usb_interface *interface,
 	if (!sur40)
 		return -ENOMEM;
 
-	input = input_allocate_device();
-	if (!input) {
-		error = -ENOMEM;
-		goto err_free_dev;
-	}
-
 	/* initialize locks/lists */
 	INIT_LIST_HEAD(&sur40->buf_list);
 	spin_lock_init(&sur40->qlock);
 	mutex_init(&sur40->lock);
 
-	/* Set up regular input device structure */
-	input->name = DRIVER_LONG;
-	usb_to_input_id(usbdev, &input->id);
-	usb_make_path(usbdev, sur40->phys, sizeof(sur40->phys));
-	strlcat(sur40->phys, "/input0", sizeof(sur40->phys));
-	input->phys = sur40->phys;
-	input->dev.parent = &interface->dev;
-
-	input->open = sur40_open;
-	input->close = sur40_close;
-
-	error = sur40_input_setup_events(input);
-	if (error)
-		goto err_free_input;
-
-	input_set_drvdata(input, sur40);
-	error = input_setup_polling(input, sur40_poll);
-	if (error) {
-		dev_err(&interface->dev, "failed to set up polling");
-		goto err_free_input;
-	}
-
-	input_set_poll_interval(input, POLL_INTERVAL);
-
 	sur40->usbdev = usbdev;
 	sur40->dev = &interface->dev;
-	sur40->input = input;
 
 	/* use the bulk-in endpoint tested above */
 	sur40->bulk_in_size = usb_endpoint_maxp(endpoint);
@@ -722,7 +737,7 @@ static int sur40_probe(struct usb_interface *interface,
 	if (!sur40->bulk_in_buffer) {
 		dev_err(&interface->dev, "Unable to allocate input buffer.");
 		error = -ENOMEM;
-		goto err_free_input;
+		goto err_free_dev;
 	}
 
 	/* register the video master device */
@@ -790,13 +805,9 @@ static int sur40_probe(struct usb_interface *interface,
 		goto err_free_ctrl;
 	}
 
-	/* register the polled input device */
-	error = input_register_device(input);
-	if (error) {
-		dev_err(&interface->dev,
-			"Unable to register polled input device.");
+	error = sur40_init_input(sur40);
+	if (error)
 		goto err_unreg_video;
-	}
 
 	/* we can register the device now, as it is ready */
 	usb_set_intfdata(interface, sur40);
@@ -812,8 +823,6 @@ static int sur40_probe(struct usb_interface *interface,
 	v4l2_device_unregister(&sur40->v4l2);
 err_free_buffer:
 	kfree(sur40->bulk_in_buffer);
-err_free_input:
-	input_free_device(input);
 err_free_dev:
 	kfree(sur40);
 
-- 
2.54.0.1136.gdb2ca164c4-goog

Re: [PATCH 3/4] Input: sur40 - factor out and move input device initialization

[sashiko-bot]

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

Pre-existing issues:
- [High] Deadlock in `sur40_stop_streaming` due to waiting for buffers that will never be processed if input polling is not active.
--

commit 7253dffc6b93516e6c96ea7370f123676e435b94
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: sur40 - factor out and move input device initialization
    
The input device allocation, setup, and registration in sur40_probe() is
quite verbose. Factor it out into a helper function sur40_init_input() to
improve readability.
    
Additionally, call this helper at the very end of sur40_probe() instead of
allocating the input device early. This ensures all video components are fully
initialized before the input device is registered (which starts polling),
and simplifies the early probe error paths since we don't have to carry and
free the input device if probe fails during early V4L2 setup.
    
Reported-by: sashiko-bot@kernel.org
Cc: stable@vger.kernel.org
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index e9089b0c3e2f4..1ad68131e3a64 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c

[ ... ]

> @@ -647,6 +647,53 @@ static int sur40_input_setup_events(struct input_dev *input_dev)
>  	return 0;
>  }
>  
> +static int sur40_init_input(struct sur40_state *sur40)
> +{
> +	struct input_dev *input;
> +	int error;
> +

[ ... ]

> +	error = sur40_input_setup_events(input);
> +	if (error)
> +		goto err_free_input;
> +
> +	input_set_drvdata(input, sur40);
> +	error = input_setup_polling(input, sur40_poll);

[Severity: High]
This isn't a bug introduced by this patch, but setting up polling here
highlights an existing deadlock in the driver.

If a user opens the video device, queues buffers, starts streaming, and then
stops streaming while the input device remains closed, the sur40_poll()
callback will never run.

This causes a deadlock when stopping the video stream:

drivers/input/touchscreen/sur40.c:sur40_stop_streaming() {
	...
	vb2_wait_for_all_buffers(vq);
	sur40->sequence = -1;

	/* Release all active buffers */
	return_all_buffers(sur40, VB2_BUF_STATE_ERROR);
}

The vb2_wait_for_all_buffers() function blocks indefinitely waiting for buffers
to be returned, but those buffers are only processed inside sur40_poll(). Since
the input device is closed, the polling thread isn't active, causing the
process to hang.

Should the driver avoid waiting for buffers if polling isn't active, or
should the polling lifecycle be decoupled from the input device open state?

> +	if (error) {
> +		dev_err(sur40->dev, "failed to set up polling\n");
> +		goto err_free_input;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616051235.1549517-1-dmitry.torokhov@gmail.com?part=3

[PATCH 4/4] Input: sur40 - fix V4L2 video device lifetime

[Dmitry Torokhov]

sur40_disconnect() synchronously frees the sur40_state structure (kfree(sur40))
while userspace might still hold an open file descriptor to the V4L2 video
device node. When userspace later accesses or closes the lingering file
descriptor, the V4L2 core invokes file operations (such as vb2_fop_release)
that dereference the already freed sur40 memory, resulting in a use-after-free
vulnerability.

Fix this by implementing a V4L2 release callback (sur40_video_release) in
sur40_video_device to clean up V4L2 components and free the sur40 structure
only when the last video file descriptor is closed.

Additionally, update the sur40_probe() error path to call video_unregister_device()
and return inline if input initialization fails after video device registration
succeeded, allowing the V4L2 release callback to manage cleanup.

Also, call v4l2_device_disconnect() in sur40_disconnect() to safely dissociate
the V4L2 device from the parent USB device during unplug.

Reported-by: sashiko-bot@kernel.org
Cc: stable@vger.kernel.org
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/sur40.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 1ad68131e3a6..2f0efee23d1e 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -806,8 +806,10 @@ static int sur40_probe(struct usb_interface *interface,
 	}
 
 	error = sur40_init_input(sur40);
-	if (error)
-		goto err_unreg_video;
+	if (error) {
+		video_unregister_device(&sur40->vdev);
+		return error;
+	}
 
 	/* we can register the device now, as it is ready */
 	usb_set_intfdata(interface, sur40);
@@ -815,8 +817,6 @@ static int sur40_probe(struct usb_interface *interface,
 
 	return 0;
 
-err_unreg_video:
-	video_unregister_device(&sur40->vdev);
 err_free_ctrl:
 	v4l2_ctrl_handler_free(&sur40->hdl);
 err_unreg_v4l2:
@@ -835,13 +835,8 @@ 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);
+	v4l2_device_disconnect(&sur40->v4l2);
 
 	usb_set_intfdata(interface, NULL);
 	dev_dbg(&interface->dev, "%s is now disconnected\n", DRIVER_DESC);
@@ -1176,11 +1171,21 @@ static const struct v4l2_ioctl_ops sur40_video_ioctl_ops = {
 	.vidioc_streamoff	= vb2_ioctl_streamoff,
 };
 
+static void sur40_video_release(struct video_device *vdev)
+{
+	struct sur40_state *sur40 = video_get_drvdata(vdev);
+
+	v4l2_device_unregister(&sur40->v4l2);
+	v4l2_ctrl_handler_free(&sur40->hdl);
+	kfree(sur40->bulk_in_buffer);
+	kfree(sur40);
+}
+
 static const struct video_device sur40_video_device = {
 	.name = DRIVER_LONG,
 	.fops = &sur40_video_fops,
 	.ioctl_ops = &sur40_video_ioctl_ops,
-	.release = video_device_release_empty,
+	.release = sur40_video_release,
 	.device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_TOUCH |
 		       V4L2_CAP_READWRITE | V4L2_CAP_STREAMING,
 };
-- 
2.54.0.1136.gdb2ca164c4-goog

Re: [PATCH 4/4] Input: sur40 - fix V4L2 video device lifetime

[sashiko-bot]

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

New issues:
- [Critical] Calling `v4l2_device_disconnect()` after `video_unregister_device()` causes a deterministic use-after-free on device unplug.
- [Critical] Double-free and use-after-free of the `sur40` structure and V4L2 components if `video_register_device()` fails during probe.

Pre-existing issues:
- [High] `sur40_stop_streaming()` hangs forever waiting for buffers to be returned by a stopped polling thread.
- [High] Lingering input device file descriptor allows Use-After-Free access to sur40->phys via EVIOCGPHYS.
--

commit c1c922112f8294f71a1841f179b4f4b61138851b
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: sur40 - fix V4L2 video device lifetime

This commit introduces a V4L2 release callback to clean up components and
free the sur40 structure only when the last video file descriptor is closed.
It updates the error path and disconnect sequence to handle this new
lifecycle.

> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index 1ad68131e3a64..2f0efee23d1ec 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
> @@ -806,8 +806,10 @@ static int sur40_probe(struct usb_interface *interface,
>  	}
>  
>  	error = sur40_init_input(sur40);

[Severity: High]
This is a pre-existing issue, but could there be a use-after-free when
userspace queries EVIOCGPHYS?

Inside sur40_init_input(), the input device's phys path is pointed directly
to the sur40->phys array:

sur40_init_input()
    input->phys = sur40->phys;
    
If the video device is closed, the new release callback frees the sur40
structure. If a userspace process still holds the input device open, a
subsequent EVIOCGPHYS ioctl will read from the freed sur40 memory.

> -	if (error)
> -		goto err_unreg_video;
> +	if (error) {
> +		video_unregister_device(&sur40->vdev);
> +		return error;
> +	}
>  
>  	/* we can register the device now, as it is ready */
>  	usb_set_intfdata(interface, sur40);
> @@ -815,8 +817,6 @@ static int sur40_probe(struct usb_interface *interface,
>  
>  	return 0;
>  
> -err_unreg_video:
> -	video_unregister_device(&sur40->vdev);
>  err_free_ctrl:
>  	v4l2_ctrl_handler_free(&sur40->hdl);
>  err_unreg_v4l2:

[Severity: Critical]
Does this error path cause a double-free if video_register_device() fails
earlier in sur40_probe()?

If video_register_device() fails internally during device_register(), the
V4L2 core handles cleanup by dropping the reference count, which
synchronously invokes the new sur40_video_release() callback. That
callback frees the control handler, buffer, and the sur40 structure.

Jumping to err_free_ctrl on failure will then call v4l2_ctrl_handler_free()
and kfree() again on the already-freed pointers.

> @@ -835,13 +835,8 @@ static void sur40_disconnect(struct usb_interface *interface)
>  	struct sur40_state *sur40 = usb_get_intfdata(interface);
>  
>  	input_unregister_device(sur40->input);

[Severity: High]
This is a pre-existing issue, but does this sequence cause a deadlock when
sur40_stop_streaming() is eventually called?

Calling input_unregister_device() synchronously stops the input polling
thread (sur40_poll). Later, when the video device is closed and
sur40_stop_streaming() is triggered, it invokes vb2_wait_for_all_buffers().
This waits indefinitely for buffers that can only be returned by the
now-dead polling thread.

> -
> -	v4l2_ctrl_handler_free(&sur40->hdl);
>  	video_unregister_device(&sur40->vdev);
> -	v4l2_device_unregister(&sur40->v4l2);
> -
> -	kfree(sur40->bulk_in_buffer);
> -	kfree(sur40);
> +	v4l2_device_disconnect(&sur40->v4l2);

[Severity: Critical]
Will this dereference the newly freed sur40 memory?

If there are no open file descriptors, video_unregister_device() drops the
final reference to the video device. This synchronously triggers the
new sur40_video_release() callback, which executes kfree(sur40).

Immediately after, v4l2_device_disconnect(&sur40->v4l2) is called, which
would access the freed memory. Should the order of these two calls be
swapped?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616051235.1549517-1-dmitry.torokhov@gmail.com?part=4

Re: [PATCH 1/4] Input: sur40 - fix input device registration ordering

[sashiko-bot]

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

Pre-existing issues:
- [High] A malicious USB device can cause the input polling thread to hang indefinitely in an infinite loop.
- [High] Device unplug while streaming causes a deadlock in `sur40_stop_streaming()`.
--

commit e70bc2bfbadc0d9c5dd284e6e5e9d8bace3e06fd
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: sur40 - fix input device registration ordering

This patch changes the device registration ordering in sur40_probe() to
ensure V4L2 components are initialized before the input device is registered,
preventing a race condition. It also updates sur40_disconnect() to
unregister the input device first, stopping the polling worker thread
before cleaning up V4L2 structures.

> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index fe63d53d56db0..8639ec3ad7038 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
[ ... ]
> @@ -823,11 +823,12 @@ 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 a pre-existing deadlock
when the device is unplugged while streaming.

If the device is physically disconnected, sur40_disconnect() calls
input_unregister_device() which synchronously stops the polling worker thread.
When the application then closes the video file descriptor,
sur40_stop_streaming() is invoked:

drivers/input/touchscreen/sur40.c:sur40_stop_streaming() {
    ...
    vb2_wait_for_all_buffers(vq);
    sur40->sequence = -1;

    /* Release all active buffers */
    return_all_buffers(sur40, VB2_BUF_STATE_ERROR);
    ...
}

Because vb2_wait_for_all_buffers() blocks waiting for the now-dead polling
worker to return queued buffers, and the manual buffer release
return_all_buffers() is executed after the wait, will this code deadlock the
system call?

>  	v4l2_device_unregister(&sur40->v4l2);
>  
> -	input_unregister_device(sur40->input);
>  	kfree(sur40->bulk_in_buffer);

[Severity: High]
This isn't a bug introduced by this patch, but is a pre-existing Denial of
Service vulnerability where a malicious USB device can cause the input
polling thread to hang indefinitely.

In sur40_poll(), the driver reads bulk messages and calculates packet_blobs:

drivers/input/touchscreen/sur40.c:sur40_poll() {
    ...
    packet_blobs = result / sizeof(struct sur40_blob);
    // ...
    for (i = 0; i < packet_blobs; i++) {
        need_blobs--;
    ...
}

If a malicious USB device sends exactly 16 bytes (the size of struct
sur40_header) with count > 0, result will be 0, leading to packet_blobs = 0.
The inner loop is skipped, leaving need_blobs unchanged.

Because need_blobs never decrements, could the outer do { ... } while
(need_blobs > 0) loop spin indefinitely, hanging the worker thread and
blocking cancel_delayed_work_sync() during module removal and device
cleanup?

>  	kfree(sur40);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616051235.1549517-1-dmitry.torokhov@gmail.com?part=1