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@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://lore.kernel.org/all/6a8e44ca.dbb3a75c.7844.002f.GAE@xxxxxxxxxx/
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Pei Xiao <xiaopei01@xxxxxxxxxx>
> ---
>  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);