[PATCH] usb: gadget: f_uvc: fix NULL pointer dereference during unbind race

[PATCH] usb: gadget: f_uvc: fix NULL pointer dereference during unbind race

[Jimmy Hu]

Commit b81ac4395bbe ("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

The fix introduces a 'func_unbinding' flag in struct uvc_device to protect
critical sections:
1. In uvc_function_disconnect(), it prevents accessing the nullified
   cdev->gadget pointer.
2. In uvc_v4l2_release(), it ensures uvcg_free_buffers() is skipped
   if unbind is already in progress, avoiding races with concurrent
   bind operations or use-after-free on the video queue memory.

Fixes: b81ac4395bbe ("usb: gadget: uvc: allow for application to cleanly shutdown")
Cc: <stable@vger.kernel.org>
Signed-off-by: Jimmy Hu <hhhuuu@google.com>
---
 drivers/usb/gadget/function/f_uvc.c    | 7 +++++++
 drivers/usb/gadget/function/uvc.h      | 1 +
 drivers/usb/gadget/function/uvc_v4l2.c | 6 ++++++
 3 files changed, 14 insertions(+)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index a96476507d2f..f8c609ad1a43 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -413,6 +413,11 @@ uvc_function_disconnect(struct uvc_device *uvc)
 {
 	int ret;
 
+	if (uvc->func_unbinding) {
+		pr_info("uvc: unbinding, skipping function deactivate\n");
+		return;
+	}
+
 	if ((ret = usb_function_deactivate(&uvc->func)) < 0)
 		uvcg_info(&uvc->func, "UVC disconnect failed with %d\n", ret);
 }
@@ -659,6 +664,7 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	int ret = -EINVAL;
 
 	uvcg_info(f, "%s()\n", __func__);
+	uvc->func_unbinding = false;
 
 	opts = fi_to_f_uvc_opts(f->fi);
 	/* Sanity check the streaming endpoint module parameters. */
@@ -994,6 +1000,7 @@ static void uvc_function_unbind(struct usb_configuration *c,
 	long wait_ret = 1;
 
 	uvcg_info(f, "%s()\n", __func__);
+	uvc->func_unbinding = true;
 
 	kthread_cancel_work_sync(&video->hw_submit);
 
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index 676419a04976..7ca56ff737a4 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -155,6 +155,7 @@ struct uvc_device {
 	enum uvc_state state;
 	struct usb_function func;
 	struct uvc_video video;
+	bool func_unbinding;
 	bool func_connected;
 	wait_queue_head_t func_connected_queue;
 
diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c
index fd4b998ccd16..a8a15b584648 100644
--- a/drivers/usb/gadget/function/uvc_v4l2.c
+++ b/drivers/usb/gadget/function/uvc_v4l2.c
@@ -594,7 +594,13 @@ static void uvc_v4l2_disable(struct uvc_device *uvc)
 {
 	uvc_function_disconnect(uvc);
 	uvcg_video_disable(&uvc->video);
+	if (uvc->func_unbinding) {
+		pr_info("uvc: unbinding, skipping buffer cleanup\n");
+		goto skip_buffer_cleanup;
+	}
 	uvcg_free_buffers(&uvc->video.queue);
+
+skip_buffer_cleanup:
 	uvc->func_connected = false;
 	wake_up_interruptible(&uvc->func_connected_queue);
 }

base-commit: 24d479d26b25bce5faea3ddd9fa8f3a6c3129ea7
-- 
2.53.0.371.g1d285c8824-goog

Re: [PATCH] usb: gadget: f_uvc: fix NULL pointer dereference during unbind race

[Alan Stern]

On Tue, Feb 24, 2026 at 04:39:55PM +0800, Jimmy Hu wrote:
> Commit b81ac4395bbe ("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
> 
> The fix introduces a 'func_unbinding' flag in struct uvc_device to protect
> critical sections:
> 1. In uvc_function_disconnect(), it prevents accessing the nullified
>    cdev->gadget pointer.
> 2. In uvc_v4l2_release(), it ensures uvcg_free_buffers() is skipped
>    if unbind is already in progress, avoiding races with concurrent
>    bind operations or use-after-free on the video queue memory.

Sorry if the answer to this question is obvious to anybody familiar with 
the driver...

The patch adds a flag that can be accessed by two different tasks 
(disconnect and release).  Is there any synchronization to prevent these 
tasks from racing and accessing the new flag concurrently?

Alan Stern

Re: [PATCH] usb: gadget: f_uvc: fix NULL pointer dereference during unbind race

[Jimmy Hu (xWF)]

On Tue, Feb 24, 2026 at 11:47 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Tue, Feb 24, 2026 at 04:39:55PM +0800, Jimmy Hu wrote:
> > Commit b81ac4395bbe ("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
> >
> > The fix introduces a 'func_unbinding' flag in struct uvc_device to protect
> > critical sections:
> > 1. In uvc_function_disconnect(), it prevents accessing the nullified
> >    cdev->gadget pointer.
> > 2. In uvc_v4l2_release(), it ensures uvcg_free_buffers() is skipped
> >    if unbind is already in progress, avoiding races with concurrent
> >    bind operations or use-after-free on the video queue memory.
>
> Sorry if the answer to this question is obvious to anybody familiar with
> the driver...
>
> The patch adds a flag that can be accessed by two different tasks
> (disconnect and release).  Is there any synchronization to prevent these
> tasks from racing and accessing the new flag concurrently?
>
> Alan Stern

Hi Alan,

Thanks for pointing that out. You're right, the boolean flag lacks
proper synchronization for concurrent access.
I will submit a V2 patch using atomic bit operations to ensure memory
visibility and atomicity across tasks. The changes will include:
* Replacing 'bool func_unbinding' with 'unsigned long flags' in struct
uvc_device.
* Using clear_bit() in uvc_function_bind() to reset the state.
* Using set_bit() in uvc_function_unbind() to mark the unbinding phase.
* Using test_bit() in uvc_function_disconnect() and uvc_v4l2_disable()
for safe checks.

Does this approach look reasonable to you?

Best regards,
Jimmy

Re: [PATCH] usb: gadget: f_uvc: fix NULL pointer dereference during unbind race

[Alan Stern]

On Wed, Feb 25, 2026 at 10:31:54AM +0800, Jimmy Hu (xWF) wrote:
> On Tue, Feb 24, 2026 at 11:47 PM Alan Stern <stern@rowland.harvard.edu> wrote:
> >
> > On Tue, Feb 24, 2026 at 04:39:55PM +0800, Jimmy Hu wrote:
> > > Commit b81ac4395bbe ("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
> > >
> > > The fix introduces a 'func_unbinding' flag in struct uvc_device to protect
> > > critical sections:
> > > 1. In uvc_function_disconnect(), it prevents accessing the nullified
> > >    cdev->gadget pointer.
> > > 2. In uvc_v4l2_release(), it ensures uvcg_free_buffers() is skipped
> > >    if unbind is already in progress, avoiding races with concurrent
> > >    bind operations or use-after-free on the video queue memory.
> >
> > Sorry if the answer to this question is obvious to anybody familiar with
> > the driver...
> >
> > The patch adds a flag that can be accessed by two different tasks
> > (disconnect and release).  Is there any synchronization to prevent these
> > tasks from racing and accessing the new flag concurrently?
> >
> > Alan Stern
> 
> Hi Alan,
> 
> Thanks for pointing that out. You're right, the boolean flag lacks
> proper synchronization for concurrent access.
> I will submit a V2 patch using atomic bit operations to ensure memory
> visibility and atomicity across tasks. The changes will include:
> * Replacing 'bool func_unbinding' with 'unsigned long flags' in struct
> uvc_device.
> * Using clear_bit() in uvc_function_bind() to reset the state.
> * Using set_bit() in uvc_function_unbind() to mark the unbinding phase.
> * Using test_bit() in uvc_function_disconnect() and uvc_v4l2_disable()
> for safe checks.
> 
> Does this approach look reasonable to you?

No, because data races are more complicated than just concurrent 
non-atomic accesses.  There's also the issue of ordering.  Here's what I 
mean...

You want to change the bind routine to do something like:

	cdev->gadget = ...
	set_bit(&uvc->flags, 0);	// Driver is bound

and unbind to do something like:

	clear_bit(&uvc->flags, 0);	// Driver is unbound
	cdev->gadget = NULL;

You'll also change disconnect and disable like this:

	if (test_bit(&uvc->flags, 0)) {
		... use cdev->gadget ...
	}

The problem is that modern CPUs can execute instructions out of order 
and can speculate loads.  When the CPU runs the bind routine, there's 
nothing to prevent it from doing the set_bit() before assigning 
cdev->gadget.  Similarly, when the CPU runs the unbind routine, there's 
nothing to prevent it from setting cdev->gadget to NULL before doing the 
clear_bit().

More subtly, when the CPU runs the disconnect or disable routines, 
there's nothing to prevent it from speculatively reading cdev->gadget 
(and getting a NULL result) before doing the test_bit() (and getting 1).

To prevent these problems, you would need to use memory barriers.  For 
example, you could put smb_wmb() between the two statements in bind and 
unbind, and smp_rmb() in disconnect and disable before cdev->gadget 
gets used.  (For a lot more information about these memory barriers and 
other ordering issues, see 
tools/memory-model/Documentation/explanation.txt.)

A simpler approach, if it won't cause any problems, is to ensure 
synchronization by protecting all these different pieces of code with a 
mutex.  Then they would never execute concurrently.

Alan Stern

[PATCH v2] usb: gadget: f_uvc: fix NULL pointer dereference during unbind race

[Jimmy Hu]

Commit b81ac4395bbe ("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

This patch introduces the following safeguards:

1. State Synchronization (flag + mutex)
Introduced 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.

2. Lifecycle Management (kref)
Introduced kref to manage the struct uvc_device lifecycle. This prevents
Use-After-Free (UAF) by ensuring the structure is only freed after the
final reference, including the V4L2 release path, is dropped.

Fixes: b81ac4395bbe ("usb: gadget: uvc: allow for application to cleanly shutdown")
Cc: <stable@vger.kernel.org>
Suggested-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Jimmy Hu <hhhuuu@google.com>
---
v1 -> v2:
- Renamed 'func_unbinding' to 'func_unbound' for clearer state semantics.
- Added a mutex (uvc->lock) to protect the 'func_unbound' flag and ensure
  proper memory ordering, as suggested by Alan Stern.
- Integrated kref to manage the struct uvc_device lifecycle, allowing the 
  removal of redundant buffer cleanup skip logic in uvc_v4l2_disable().
  
v1: https://patchwork.kernel.org/project/linux-usb/patch/20260224083955.1375032-1-hhhuuu@google.com/

 drivers/usb/gadget/function/f_uvc.c    | 27 +++++++++++++++++++++++++-
 drivers/usb/gadget/function/uvc.h      |  4 ++++
 drivers/usb/gadget/function/uvc_v4l2.c |  2 ++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 494fdbc4e85b..485cd91770d5 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -413,8 +413,17 @@ uvc_function_disconnect(struct uvc_device *uvc)
 {
 	int ret;
 
+	mutex_lock(&uvc->lock);
+	if (uvc->func_unbound) {
+		pr_info("uvc: unbound, skipping function deactivate\n");
+		goto unlock;
+	}
+
 	if ((ret = usb_function_deactivate(&uvc->func)) < 0)
 		uvcg_info(&uvc->func, "UVC disconnect failed with %d\n", ret);
+
+unlock:
+	mutex_unlock(&uvc->lock);
 }
 
 /* --------------------------------------------------------------------------
@@ -659,6 +668,9 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	int ret = -EINVAL;
 
 	uvcg_info(f, "%s()\n", __func__);
+	mutex_lock(&uvc->lock);
+	uvc->func_unbound = false;
+	mutex_unlock(&uvc->lock);
 
 	opts = fi_to_f_uvc_opts(f->fi);
 	/* Sanity check the streaming endpoint module parameters. */
@@ -974,6 +986,13 @@ static struct usb_function_instance *uvc_alloc_inst(void)
 	return &opts->func_inst;
 }
 
+void uvc_device_release(struct kref *kref)
+{
+	struct uvc_device *uvc = container_of(kref, struct uvc_device, kref);
+
+	kfree(uvc);
+}
+
 static void uvc_free(struct usb_function *f)
 {
 	struct uvc_device *uvc = to_uvc(f);
@@ -982,7 +1001,7 @@ static void uvc_free(struct usb_function *f)
 	if (!opts->header)
 		config_item_put(&uvc->header->item);
 	--opts->refcnt;
-	kfree(uvc);
+	kref_put(&uvc->kref, uvc_device_release);
 }
 
 static void uvc_function_unbind(struct usb_configuration *c,
@@ -994,6 +1013,9 @@ static void uvc_function_unbind(struct usb_configuration *c,
 	long wait_ret = 1;
 
 	uvcg_info(f, "%s()\n", __func__);
+	mutex_lock(&uvc->lock);
+	uvc->func_unbound = true;
+	mutex_unlock(&uvc->lock);
 
 	kthread_cancel_work_sync(&video->hw_submit);
 
@@ -1046,8 +1068,11 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi)
 	if (uvc == NULL)
 		return ERR_PTR(-ENOMEM);
 
+	kref_init(&uvc->kref);
+	mutex_init(&uvc->lock);
 	mutex_init(&uvc->video.mutex);
 	uvc->state = UVC_STATE_DISCONNECTED;
+	uvc->func_unbound = true;
 	init_waitqueue_head(&uvc->func_connected_queue);
 	opts = fi_to_f_uvc_opts(fi);
 
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index 676419a04976..22b70f25607d 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -155,6 +155,9 @@ struct uvc_device {
 	enum uvc_state state;
 	struct usb_function func;
 	struct uvc_video video;
+	struct kref kref;
+	struct mutex lock;
+	bool func_unbound;
 	bool func_connected;
 	wait_queue_head_t func_connected_queue;
 
@@ -209,5 +212,6 @@ static inline struct uvc_file_handle *file_to_uvc_file_handle(struct file *filp)
 extern void uvc_function_setup_continue(struct uvc_device *uvc, int disable_ep);
 extern void uvc_function_connect(struct uvc_device *uvc);
 extern void uvc_function_disconnect(struct uvc_device *uvc);
+extern void uvc_device_release(struct kref *kref);
 
 #endif /* _UVC_GADGET_H_ */
diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c
index ed48d38498fb..2dae27f05e2a 100644
--- a/drivers/usb/gadget/function/uvc_v4l2.c
+++ b/drivers/usb/gadget/function/uvc_v4l2.c
@@ -671,6 +671,7 @@ uvc_v4l2_open(struct file *file)
 	if (handle == NULL)
 		return -ENOMEM;
 
+	kref_get(&uvc->kref);
 	v4l2_fh_init(&handle->vfh, vdev);
 	v4l2_fh_add(&handle->vfh, file);
 
@@ -695,6 +696,7 @@ uvc_v4l2_release(struct file *file)
 	v4l2_fh_del(&handle->vfh, file);
 	v4l2_fh_exit(&handle->vfh);
 	kfree(handle);
+	kref_put(&uvc->kref, uvc_device_release);
 
 	return 0;
 }

base-commit: 6de23f81a5e08be8fbf5e8d7e9febc72a5b5f27f
-- 
2.53.0.473.g4a7958ca14-goog

Re: [PATCH v2] usb: gadget: f_uvc: fix NULL pointer dereference during unbind race

[Greg Kroah-Hartman]

On Mon, Mar 09, 2026 at 01:31:07PM +0800, Jimmy Hu wrote:
> Commit b81ac4395bbe ("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
> 
> This patch introduces the following safeguards:
> 
> 1. State Synchronization (flag + mutex)
> Introduced 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.
> 
> 2. Lifecycle Management (kref)
> Introduced kref to manage the struct uvc_device lifecycle. This prevents
> Use-After-Free (UAF) by ensuring the structure is only freed after the
> final reference, including the V4L2 release path, is dropped.
> 
> Fixes: b81ac4395bbe ("usb: gadget: uvc: allow for application to cleanly shutdown")
> Cc: <stable@vger.kernel.org>
> Suggested-by: Alan Stern <stern@rowland.harvard.edu>
> Signed-off-by: Jimmy Hu <hhhuuu@google.com>
> ---
> v1 -> v2:
> - Renamed 'func_unbinding' to 'func_unbound' for clearer state semantics.
> - Added a mutex (uvc->lock) to protect the 'func_unbound' flag and ensure
>   proper memory ordering, as suggested by Alan Stern.
> - Integrated kref to manage the struct uvc_device lifecycle, allowing the 
>   removal of redundant buffer cleanup skip logic in uvc_v4l2_disable().
>   
> v1: https://patchwork.kernel.org/project/linux-usb/patch/20260224083955.1375032-1-hhhuuu@google.com/
> 
>  drivers/usb/gadget/function/f_uvc.c    | 27 +++++++++++++++++++++++++-
>  drivers/usb/gadget/function/uvc.h      |  4 ++++
>  drivers/usb/gadget/function/uvc_v4l2.c |  2 ++
>  3 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
> index 494fdbc4e85b..485cd91770d5 100644
> --- a/drivers/usb/gadget/function/f_uvc.c
> +++ b/drivers/usb/gadget/function/f_uvc.c
> @@ -413,8 +413,17 @@ uvc_function_disconnect(struct uvc_device *uvc)
>  {
>  	int ret;
>  
> +	mutex_lock(&uvc->lock);
> +	if (uvc->func_unbound) {
> +		pr_info("uvc: unbound, skipping function deactivate\n");

When drivers work properly, they are quiet.  Also, why are you not using
uvcg_info() here, this pr_info() gives no device specific information so
you know what device this is happening to.



> +		goto unlock;
> +	}
> +
>  	if ((ret = usb_function_deactivate(&uvc->func)) < 0)
>  		uvcg_info(&uvc->func, "UVC disconnect failed with %d\n", ret);
> +
> +unlock:
> +	mutex_unlock(&uvc->lock);
>  }
>  
>  /* --------------------------------------------------------------------------
> @@ -659,6 +668,9 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
>  	int ret = -EINVAL;
>  
>  	uvcg_info(f, "%s()\n", __func__);
> +	mutex_lock(&uvc->lock);
> +	uvc->func_unbound = false;
> +	mutex_unlock(&uvc->lock);
>  
>  	opts = fi_to_f_uvc_opts(f->fi);
>  	/* Sanity check the streaming endpoint module parameters. */
> @@ -974,6 +986,13 @@ static struct usb_function_instance *uvc_alloc_inst(void)
>  	return &opts->func_inst;
>  }
>  
> +void uvc_device_release(struct kref *kref)
> +{
> +	struct uvc_device *uvc = container_of(kref, struct uvc_device, kref);
> +
> +	kfree(uvc);
> +}
> +
>  static void uvc_free(struct usb_function *f)
>  {
>  	struct uvc_device *uvc = to_uvc(f);
> @@ -982,7 +1001,7 @@ static void uvc_free(struct usb_function *f)
>  	if (!opts->header)
>  		config_item_put(&uvc->header->item);
>  	--opts->refcnt;
> -	kfree(uvc);
> +	kref_put(&uvc->kref, uvc_device_release);
>  }
>  
>  static void uvc_function_unbind(struct usb_configuration *c,
> @@ -994,6 +1013,9 @@ static void uvc_function_unbind(struct usb_configuration *c,
>  	long wait_ret = 1;
>  
>  	uvcg_info(f, "%s()\n", __func__);
> +	mutex_lock(&uvc->lock);
> +	uvc->func_unbound = true;
> +	mutex_unlock(&uvc->lock);
>  
>  	kthread_cancel_work_sync(&video->hw_submit);
>  
> @@ -1046,8 +1068,11 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi)
>  	if (uvc == NULL)
>  		return ERR_PTR(-ENOMEM);
>  
> +	kref_init(&uvc->kref);
> +	mutex_init(&uvc->lock);
>  	mutex_init(&uvc->video.mutex);
>  	uvc->state = UVC_STATE_DISCONNECTED;
> +	uvc->func_unbound = true;
>  	init_waitqueue_head(&uvc->func_connected_queue);
>  	opts = fi_to_f_uvc_opts(fi);
>  
> diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
> index 676419a04976..22b70f25607d 100644
> --- a/drivers/usb/gadget/function/uvc.h
> +++ b/drivers/usb/gadget/function/uvc.h
> @@ -155,6 +155,9 @@ struct uvc_device {
>  	enum uvc_state state;
>  	struct usb_function func;
>  	struct uvc_video video;
> +	struct kref kref;

But there is already a device reference count in the vdev structure,
right?  Are you now having 2 reference counts for the same device?
That's going to cause a lot of problems.

> +	struct mutex lock;

Please document what this lock is locking.

thanks,

greg k-h

Re: [PATCH v2] usb: gadget: f_uvc: fix NULL pointer dereference during unbind race

[Jimmy Hu (xWF)]

On Wed, Mar 11, 2026 at 8:46 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Mar 09, 2026 at 01:31:07PM +0800, Jimmy Hu wrote:
> > Commit b81ac4395bbe ("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
> >
> > This patch introduces the following safeguards:
> >
> > 1. State Synchronization (flag + mutex)
> > Introduced 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.
> >
> > 2. Lifecycle Management (kref)
> > Introduced kref to manage the struct uvc_device lifecycle. This prevents
> > Use-After-Free (UAF) by ensuring the structure is only freed after the
> > final reference, including the V4L2 release path, is dropped.
> >
> > Fixes: b81ac4395bbe ("usb: gadget: uvc: allow for application to cleanly shutdown")
> > Cc: <stable@vger.kernel.org>
> > Suggested-by: Alan Stern <stern@rowland.harvard.edu>
> > Signed-off-by: Jimmy Hu <hhhuuu@google.com>
> > ---
> > v1 -> v2:
> > - Renamed 'func_unbinding' to 'func_unbound' for clearer state semantics.
> > - Added a mutex (uvc->lock) to protect the 'func_unbound' flag and ensure
> >   proper memory ordering, as suggested by Alan Stern.
> > - Integrated kref to manage the struct uvc_device lifecycle, allowing the
> >   removal of redundant buffer cleanup skip logic in uvc_v4l2_disable().
> >
> > v1: https://patchwork.kernel.org/project/linux-usb/patch/20260224083955.1375032-1-hhhuuu@google.com/
> >
> >  drivers/usb/gadget/function/f_uvc.c    | 27 +++++++++++++++++++++++++-
> >  drivers/usb/gadget/function/uvc.h      |  4 ++++
> >  drivers/usb/gadget/function/uvc_v4l2.c |  2 ++
> >  3 files changed, 32 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
> > index 494fdbc4e85b..485cd91770d5 100644
> > --- a/drivers/usb/gadget/function/f_uvc.c
> > +++ b/drivers/usb/gadget/function/f_uvc.c
> > @@ -413,8 +413,17 @@ uvc_function_disconnect(struct uvc_device *uvc)
> >  {
> >       int ret;
> >
> > +     mutex_lock(&uvc->lock);
> > +     if (uvc->func_unbound) {
> > +             pr_info("uvc: unbound, skipping function deactivate\n");
>
> When drivers work properly, they are quiet.  Also, why are you not using
> uvcg_info() here, this pr_info() gives no device specific information so
> you know what device this is happening to.
>
>
>
> > +             goto unlock;
> > +     }
> > +
> >       if ((ret = usb_function_deactivate(&uvc->func)) < 0)
> >               uvcg_info(&uvc->func, "UVC disconnect failed with %d\n", ret);
> > +
> > +unlock:
> > +     mutex_unlock(&uvc->lock);
> >  }
> >
> >  /* --------------------------------------------------------------------------
> > @@ -659,6 +668,9 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
> >       int ret = -EINVAL;
> >
> >       uvcg_info(f, "%s()\n", __func__);
> > +     mutex_lock(&uvc->lock);
> > +     uvc->func_unbound = false;
> > +     mutex_unlock(&uvc->lock);
> >
> >       opts = fi_to_f_uvc_opts(f->fi);
> >       /* Sanity check the streaming endpoint module parameters. */
> > @@ -974,6 +986,13 @@ static struct usb_function_instance *uvc_alloc_inst(void)
> >       return &opts->func_inst;
> >  }
> >
> > +void uvc_device_release(struct kref *kref)
> > +{
> > +     struct uvc_device *uvc = container_of(kref, struct uvc_device, kref);
> > +
> > +     kfree(uvc);
> > +}
> > +
> >  static void uvc_free(struct usb_function *f)
> >  {
> >       struct uvc_device *uvc = to_uvc(f);
> > @@ -982,7 +1001,7 @@ static void uvc_free(struct usb_function *f)
> >       if (!opts->header)
> >               config_item_put(&uvc->header->item);
> >       --opts->refcnt;
> > -     kfree(uvc);
> > +     kref_put(&uvc->kref, uvc_device_release);
> >  }
> >
> >  static void uvc_function_unbind(struct usb_configuration *c,
> > @@ -994,6 +1013,9 @@ static void uvc_function_unbind(struct usb_configuration *c,
> >       long wait_ret = 1;
> >
> >       uvcg_info(f, "%s()\n", __func__);
> > +     mutex_lock(&uvc->lock);
> > +     uvc->func_unbound = true;
> > +     mutex_unlock(&uvc->lock);
> >
> >       kthread_cancel_work_sync(&video->hw_submit);
> >
> > @@ -1046,8 +1068,11 @@ static struct usb_function *uvc_alloc(struct usb_function_instance *fi)
> >       if (uvc == NULL)
> >               return ERR_PTR(-ENOMEM);
> >
> > +     kref_init(&uvc->kref);
> > +     mutex_init(&uvc->lock);
> >       mutex_init(&uvc->video.mutex);
> >       uvc->state = UVC_STATE_DISCONNECTED;
> > +     uvc->func_unbound = true;
> >       init_waitqueue_head(&uvc->func_connected_queue);
> >       opts = fi_to_f_uvc_opts(fi);
> >
> > diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
> > index 676419a04976..22b70f25607d 100644
> > --- a/drivers/usb/gadget/function/uvc.h
> > +++ b/drivers/usb/gadget/function/uvc.h
> > @@ -155,6 +155,9 @@ struct uvc_device {
> >       enum uvc_state state;
> >       struct usb_function func;
> >       struct uvc_video video;
> > +     struct kref kref;
>
> But there is already a device reference count in the vdev structure,
> right?  Are you now having 2 reference counts for the same device?
> That's going to cause a lot of problems.
>
> > +     struct mutex lock;
>
> Please document what this lock is locking.
>
> thanks,
>
> greg k-h

Hi Greg, Alan,

I have just sent out v3 of this patch, which addresses the feedback regarding
redundant reference counting and log noise. Specifically, I've replaced
kref with a completion, switched to pr_debug() with device-specific context
to avoid NULL dereferences, and added the requested mutex documentation.

Link to v3: https://lore.kernel.org/all/20260319084640.478695-1-hhhuuu@google.com/

Thanks!
Jimmy