Re: [PATCH] Input: sur40 - fix use-after-free in sur40_open()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Premature free of the `sur40` struct in `sur40_disconnect()` leading to a Use-After-Free when V4L2 file handles are closed.
- [Medium] Missing v4l2_ctrl_handler_free() in the video_register_device() failure path.
--

--- Patch [1]: [PATCH] Input: sur40 - fix use-after-free in sur40_open() ---
commit e0be966f67bc10328835ca2aa6f5d5b458a14ed3
Author: Deepanshu Kartikey <kartikey406@gmail.com>

Input: sur40 - fix use-after-free in sur40_open()

If any of the registration steps after input_register_device() fail, the error
path frees sur40 via input_free_device() + kfree() without ever calling
input_unregister_device(). This patch splits the unwind so that once
input_register_device() has succeeded, the failure path properly calls
input_unregister_device() before freeing sur40 and its buffer.

Closes: https://syzkaller.appspot.com/bug?extid=683f6c071b5abfc266c1

> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index 77ec2c94b91fd..4b3a4d5386e4c 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
> @@ -808,6 +808,10 @@ static int sur40_probe(struct usb_interface *interface,
>  	video_unregister_device(&sur40->vdev);
>  err_unreg_v4l2:
>  	v4l2_device_unregister(&sur40->v4l2);
> +	input_unregister_device(input);
> +	kfree(sur40->bulk_in_buffer);
> +	kfree(sur40);

[Severity: Medium]
This is a pre-existing issue, but if video_register_device() fails, the code
jumps to err_unreg_video and falls through to err_unreg_v4l2 here.

Unlike the failure path for v4l2_ctrl_handler_setup(), does this path skip
calling v4l2_ctrl_handler_free(&sur40->hdl) before freeing the sur40 structure?

Could this lead to a memory leak of the dynamically allocated structures within
the V4L2 control handler?

> +	return error;
>  err_free_buffer:
>  	kfree(sur40->bulk_in_buffer);
>  err_free_input:

[Severity: High]
This isn't a bug introduced by this patch, but looking at sur40_disconnect():

drivers/input/touchscreen/sur40.c:sur40_disconnect() {
	...
	video_unregister_device(&sur40->vdev);
	v4l2_device_unregister(&sur40->v4l2);

	input_unregister_device(sur40->input);
	kfree(sur40->bulk_in_buffer);
	kfree(sur40);
	...
}

Does freeing the sur40 context immediately without waiting for V4L2 file handles
to be fully released destroy the still-referenced vdev?

The sur40_state structure embeds a struct video_device using the
.release = video_device_release_empty dummy callback, which opts out of V4L2
internal refcounting.

Since video_unregister_device() does not block until open file handles are
closed, if an unprivileged userspace process holds an open file descriptor to
the V4L2 device when it is disconnected, the sur40 memory is freed while
actively referenced.

When the application later closes the fd, the V4L2 core calls vb2_fop_release(),
which dereferences vdev->queue (located inside the freed sur40 struct). Can this
trigger a use-after-free kernel panic?

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