Skip to content

Commit eba2936

Browse files
Jimmy Hugregkh
authored andcommitted
usb: gadget: uvc: fix NULL pointer dereference during unbind race
Commit b81ac43 ("usb: gadget: uvc: allow for application to cleanly shutdown") introduced two stages of synchronization waits totaling 1500ms in uvc_function_unbind() to prevent several types of kernel panics. However, this timing-based approach is insufficient during power management (PM) transitions. When the PM subsystem starts freezing user space processes, the wait_event_interruptible_timeout() is aborted early, which allows the unbind thread to proceed and nullify the gadget pointer (cdev->gadget = NULL): [ 814.123447][ T947] configfs-gadget.g1 gadget.0: uvc: uvc_function_unbind() [ 814.178583][ T3173] PM: suspend entry (deep) [ 814.192487][ T3173] Freezing user space processes [ 814.197668][ T947] configfs-gadget.g1 gadget.0: uvc: uvc_function_unbind no clean disconnect, wait for release When the PM subsystem resumes or aborts the suspend and tasks are restarted, the V4L2 release path is executed and attempts to access the already nullified gadget pointer, triggering a kernel panic: [ 814.292597][ C0] PM: pm_system_irq_wakeup: 479 triggered dhdpcie_host_wake [ 814.386727][ T3173] Restarting tasks ... [ 814.403522][ T4558] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000030 [ 814.404021][ T4558] pc : usb_gadget_deactivate+0x14/0xf4 [ 814.404031][ T4558] lr : usb_function_deactivate+0x54/0x94 [ 814.404078][ T4558] Call trace: [ 814.404080][ T4558] usb_gadget_deactivate+0x14/0xf4 [ 814.404083][ T4558] usb_function_deactivate+0x54/0x94 [ 814.404087][ T4558] uvc_function_disconnect+0x1c/0x5c [ 814.404092][ T4558] uvc_v4l2_release+0x44/0xac [ 814.404095][ T4558] v4l2_release+0xcc/0x130 Address the race condition and NULL pointer dereference by: 1. State Synchronization (flag + mutex) Introduce a 'func_unbound' flag in struct uvc_device. This allows uvc_function_disconnect() to safely skip accessing the nullified cdev->gadget pointer. As suggested by Alan Stern, this flag is protected by a new mutex (uvc->lock) to ensure proper memory ordering and prevent instruction reordering or speculative loads. This mutex is also used to protect 'func_connected' for consistent state management. 2. Explicit Synchronization (completion) Use a completion to synchronize uvc_function_unbind() with the uvc_vdev_release() callback. This prevents Use-After-Free (UAF) by ensuring struct uvc_device is freed after all video device resources are released. Fixes: b81ac43 ("usb: gadget: uvc: allow for application to cleanly shutdown") Cc: stable <stable@kernel.org> Suggested-by: Alan Stern <stern@rowland.harvard.edu> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Jimmy Hu <hhhuuu@google.com> Link: https://patch.msgid.link/20260320065427.1374555-1-hhhuuu@google.com Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent dd36014 commit eba2936

3 files changed

Lines changed: 43 additions & 4 deletions

File tree

drivers/usb/gadget/function/f_uvc.c

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,12 @@ uvc_function_disconnect(struct uvc_device *uvc)
413413
{
414414
int ret;
415415

416+
guard(mutex)(&uvc->lock);
417+
if (uvc->func_unbound) {
418+
dev_dbg(&uvc->vdev.dev, "skipping function deactivate (unbound)\n");
419+
return;
420+
}
421+
416422
if ((ret = usb_function_deactivate(&uvc->func)) < 0)
417423
uvcg_info(&uvc->func, "UVC disconnect failed with %d\n", ret);
418424
}
@@ -431,6 +437,15 @@ static ssize_t function_name_show(struct device *dev,
431437

432438
static DEVICE_ATTR_RO(function_name);
433439

440+
static void uvc_vdev_release(struct video_device *vdev)
441+
{
442+
struct uvc_device *uvc = video_get_drvdata(vdev);
443+
444+
/* Signal uvc_function_unbind() that the video device has been released */
445+
if (uvc->vdev_release_done)
446+
complete(uvc->vdev_release_done);
447+
}
448+
434449
static int
435450
uvc_register_video(struct uvc_device *uvc)
436451
{
@@ -443,7 +458,7 @@ uvc_register_video(struct uvc_device *uvc)
443458
uvc->vdev.v4l2_dev->dev = &cdev->gadget->dev;
444459
uvc->vdev.fops = &uvc_v4l2_fops;
445460
uvc->vdev.ioctl_ops = &uvc_v4l2_ioctl_ops;
446-
uvc->vdev.release = video_device_release_empty;
461+
uvc->vdev.release = uvc_vdev_release;
447462
uvc->vdev.vfl_dir = VFL_DIR_TX;
448463
uvc->vdev.lock = &uvc->video.mutex;
449464
uvc->vdev.device_caps = V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_STREAMING;
@@ -659,6 +674,8 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
659674
int ret = -EINVAL;
660675

661676
uvcg_info(f, "%s()\n", __func__);
677+
scoped_guard(mutex, &uvc->lock)
678+
uvc->func_unbound = false;
662679

663680
opts = fi_to_f_uvc_opts(f->fi);
664681
/* Sanity check the streaming endpoint module parameters. */
@@ -988,12 +1005,19 @@ static void uvc_free(struct usb_function *f)
9881005
static void uvc_function_unbind(struct usb_configuration *c,
9891006
struct usb_function *f)
9901007
{
1008+
DECLARE_COMPLETION_ONSTACK(vdev_release_done);
9911009
struct usb_composite_dev *cdev = c->cdev;
9921010
struct uvc_device *uvc = to_uvc(f);
9931011
struct uvc_video *video = &uvc->video;
9941012
long wait_ret = 1;
1013+
bool connected;
9951014

9961015
uvcg_info(f, "%s()\n", __func__);
1016+
scoped_guard(mutex, &uvc->lock) {
1017+
uvc->func_unbound = true;
1018+
uvc->vdev_release_done = &vdev_release_done;
1019+
connected = uvc->func_connected;
1020+
}
9971021

9981022
kthread_cancel_work_sync(&video->hw_submit);
9991023

@@ -1006,7 +1030,7 @@ static void uvc_function_unbind(struct usb_configuration *c,
10061030
* though the video device removal uevent. Allow some time for the
10071031
* application to close out before things get deleted.
10081032
*/
1009-
if (uvc->func_connected) {
1033+
if (connected) {
10101034
uvcg_dbg(f, "waiting for clean disconnect\n");
10111035
wait_ret = wait_event_interruptible_timeout(uvc->func_connected_queue,
10121036
uvc->func_connected == false, msecs_to_jiffies(500));
@@ -1017,7 +1041,10 @@ static void uvc_function_unbind(struct usb_configuration *c,
10171041
video_unregister_device(&uvc->vdev);
10181042
v4l2_device_unregister(&uvc->v4l2_dev);
10191043

1020-
if (uvc->func_connected) {
1044+
scoped_guard(mutex, &uvc->lock)
1045+
connected = uvc->func_connected;
1046+
1047+
if (connected) {
10211048
/*
10221049
* Wait for the release to occur to ensure there are no longer any
10231050
* pending operations that may cause panics when resources are cleaned
@@ -1029,6 +1056,10 @@ static void uvc_function_unbind(struct usb_configuration *c,
10291056
uvcg_dbg(f, "done waiting for release with ret: %ld\n", wait_ret);
10301057
}
10311058

1059+
/* Wait for the video device to be released */
1060+
wait_for_completion(&vdev_release_done);
1061+
uvc->vdev_release_done = NULL;
1062+
10321063
usb_ep_free_request(cdev->gadget->ep0, uvc->control_req);
10331064
kfree(uvc->control_buf);
10341065

@@ -1047,6 +1078,8 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi)
10471078
return ERR_PTR(-ENOMEM);
10481079

10491080
mutex_init(&uvc->video.mutex);
1081+
mutex_init(&uvc->lock);
1082+
uvc->func_unbound = true;
10501083
uvc->state = UVC_STATE_DISCONNECTED;
10511084
init_waitqueue_head(&uvc->func_connected_queue);
10521085
opts = fi_to_f_uvc_opts(fi);

drivers/usb/gadget/function/uvc.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,9 @@ struct uvc_device {
155155
enum uvc_state state;
156156
struct usb_function func;
157157
struct uvc_video video;
158+
struct completion *vdev_release_done;
159+
struct mutex lock; /* protects func_unbound and func_connected */
160+
bool func_unbound;
158161
bool func_connected;
159162
wait_queue_head_t func_connected_queue;
160163

drivers/usb/gadget/function/uvc_v4l2.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,8 @@ uvc_v4l2_subscribe_event(struct v4l2_fh *fh,
574574
if (sub->type < UVC_EVENT_FIRST || sub->type > UVC_EVENT_LAST)
575575
return -EINVAL;
576576

577+
guard(mutex)(&uvc->lock);
578+
577579
if (sub->type == UVC_EVENT_SETUP && uvc->func_connected)
578580
return -EBUSY;
579581

@@ -595,7 +597,8 @@ static void uvc_v4l2_disable(struct uvc_device *uvc)
595597
uvc_function_disconnect(uvc);
596598
uvcg_video_disable(&uvc->video);
597599
uvcg_free_buffers(&uvc->video.queue);
598-
uvc->func_connected = false;
600+
scoped_guard(mutex, &uvc->lock)
601+
uvc->func_connected = false;
599602
wake_up_interruptible(&uvc->func_connected_queue);
600603
}
601604

0 commit comments

Comments
 (0)