[PATCH] usb: gadget: f_tcm: Cancel delayed set_alt work on teardown

[PATCH] usb: gadget: f_tcm: Cancel delayed set_alt work on teardown

[Cen Zhang]

tcm_set_alt() defers BOT/UAS endpoint setup to a system workqueue and
returns USB_GADGET_DELAYED_STATUS. The queued work keeps the function
state alive only by a raw struct f_uas pointer, but configfs unlink can
unbind and free that function before the work runs.

The buggy scenario involves two paths, with each column showing the order
within that path:

USB control request path:           configfs unlink path:
  1. SET_CONFIGURATION or             1. Userspace removes the function
     SET_INTERFACE reaches               symlink while the gadget is bound.
     tcm_set_alt().
  2. tcm_set_alt() queues delayed     2. config_usb_cfg_unlink() forces
     set_alt work and returns            gadget unregister.
     USB_GADGET_DELAYED_STATUS.
  3. tcm_delayed_set_alt() later      3. tcm_unbind() releases descriptors
     dereferences struct f_uas.          and tcm_free() frees struct f_uas.

Validation reproduced this kernel report:
BUG: KASAN: slab-use-after-free in tcm_delayed_set_alt+0x6c/0xef0

Call Trace:
 <TASK>
 dump_stack_lvl+0x66/0xa0
 print_report+0xce/0x630
 ? tcm_delayed_set_alt+0x6c/0xef0
 ? srso_alias_return_thunk+0x5/0xfbef5
 ? __virt_addr_valid+0x188/0x320
 ? tcm_delayed_set_alt+0x6c/0xef0
 kasan_report+0xe0/0x110
 ? tcm_delayed_set_alt+0x6c/0xef0
 tcm_delayed_set_alt+0x6c/0xef0
 ? __pfx_tcm_delayed_set_alt+0x10/0x10
 ? process_one_work+0x4cb/0xb90
 ? rcu_is_watching+0x20/0x50
 ? tcm_delayed_set_alt+0x9/0xef0
 process_one_work+0x4d7/0xb90
 ? __pfx_process_one_work+0x10/0x10
 ? srso_alias_return_thunk+0x5/0xfbef5
 ? __list_add_valid_or_report+0x37/0xf0
 ? __pfx_tcm_delayed_set_alt+0x10/0x10
 ? srso_alias_return_thunk+0x5/0xfbef5
 worker_thread+0x2d8/0x570
 ? __pfx_worker_thread+0x10/0x10
 kthread+0x1ad/0x1f0
 ? __pfx_kthread+0x10/0x10
 ret_from_fork+0x3c9/0x540
 ? __pfx_ret_from_fork+0x10/0x10
 ? srso_alias_return_thunk+0x5/0xfbef5
 ? __switch_to+0x2e9/0x730
 ? __pfx_kthread+0x10/0x10
 ret_from_fork_asm+0x1a/0x30
 </TASK>

Allocated by task 544:
 kasan_save_stack+0x33/0x60
 kasan_save_track+0x14/0x30
 __kasan_kmalloc+0x8f/0xa0
 tcm_alloc+0x68/0x180
 usb_get_function+0x36/0x60
 config_usb_cfg_link+0x125/0x1b0
 configfs_symlink+0x322/0x890
 vfs_symlink+0xc2/0x270
 filename_symlinkat+0x295/0x2f0
 __x64_sys_symlinkat+0x62/0x90
 do_syscall_64+0x115/0x6a0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Freed by task 661:
 kasan_save_stack+0x33/0x60
 kasan_save_track+0x14/0x30
 kasan_save_free_info+0x3b/0x60
 __kasan_slab_free+0x43/0x70
 kfree+0x2f9/0x530
 config_usb_cfg_unlink+0x173/0x1e0
 configfs_unlink+0x1fa/0x340
 vfs_unlink+0x15c/0x510
 filename_unlinkat+0x2ba/0x450
 __x64_sys_unlinkat+0x63/0x90
 do_syscall_64+0x115/0x6a0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Make the delayed set_alt work item owned by struct f_uas instead of an
anonymous heap allocation. This gives teardown paths a stable handle for
the exact work item: disable cancels pending work without sleeping, and
unbind/free synchronously cancel it before descriptors or struct f_uas are
released.

Fixes: c52661d60f63 ("usb-gadget: Initial merge of target module for UASP + BOT")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 drivers/usb/gadget/function/f_tcm.c | 32 ++++++++++-------------------
 drivers/usb/gadget/function/tcm.h   |  2 ++
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
index 34d9f49e9987..4096f94586fe 100644
--- a/drivers/usb/gadget/function/f_tcm.c
+++ b/drivers/usb/gadget/function/f_tcm.c
@@ -2363,20 +2363,10 @@ static int tcm_bind(struct usb_configuration *c, struct usb_function *f)
 	return -ENOTSUPP;
 }
 
-struct guas_setup_wq {
-	struct work_struct work;
-	struct f_uas *fu;
-	unsigned int alt;
-};
-
 static void tcm_delayed_set_alt(struct work_struct *wq)
 {
-	struct guas_setup_wq *work = container_of(wq, struct guas_setup_wq,
-			work);
-	struct f_uas *fu = work->fu;
-	int alt = work->alt;
-
-	kfree(work);
+	struct f_uas *fu = container_of(wq, struct f_uas, delayed_set_alt);
+	int alt = fu->delayed_alt;
 
 	if (fu->flags & USBG_IS_BOT)
 		bot_cleanup_old_alt(fu);
@@ -2413,15 +2403,8 @@ static int tcm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 		return -EOPNOTSUPP;
 
 	if ((alt == USB_G_ALT_INT_BBB) || (alt == USB_G_ALT_INT_UAS)) {
-		struct guas_setup_wq *work;
-
-		work = kmalloc_obj(*work, GFP_ATOMIC);
-		if (!work)
-			return -ENOMEM;
-		INIT_WORK(&work->work, tcm_delayed_set_alt);
-		work->fu = fu;
-		work->alt = alt;
-		schedule_work(&work->work);
+		fu->delayed_alt = alt;
+		schedule_work(&fu->delayed_set_alt);
 		return USB_GADGET_DELAYED_STATUS;
 	}
 	return -EOPNOTSUPP;
@@ -2431,6 +2414,8 @@ static void tcm_disable(struct usb_function *f)
 {
 	struct f_uas *fu = to_f_uas(f);
 
+	cancel_work(&fu->delayed_set_alt);
+
 	if (fu->flags & USBG_IS_UAS)
 		uasp_cleanup_old_alt(fu);
 	else if (fu->flags & USBG_IS_BOT)
@@ -2583,11 +2568,15 @@ static void tcm_free(struct usb_function *f)
 {
 	struct f_uas *tcm = to_f_uas(f);
 
+	cancel_work_sync(&tcm->delayed_set_alt);
 	kfree(tcm);
 }
 
 static void tcm_unbind(struct usb_configuration *c, struct usb_function *f)
 {
+	struct f_uas *tcm = to_f_uas(f);
+
+	cancel_work_sync(&tcm->delayed_set_alt);
 	usb_free_all_descriptors(f);
 }
 
@@ -2621,6 +2610,7 @@ static struct usb_function *tcm_alloc(struct usb_function_instance *fi)
 	fu->function.free_func = tcm_free;
 	fu->tpg = tpg_instances[i].tpg;
 
+	INIT_WORK(&fu->delayed_set_alt, tcm_delayed_set_alt);
 	hash_init(fu->stream_hash);
 	mutex_unlock(&tpg_instances_lock);
 
diff --git a/drivers/usb/gadget/function/tcm.h b/drivers/usb/gadget/function/tcm.h
index 009974d81d66..b07bf4aa7d88 100644
--- a/drivers/usb/gadget/function/tcm.h
+++ b/drivers/usb/gadget/function/tcm.h
@@ -123,6 +123,8 @@ struct f_uas {
 	struct usbg_tpg		*tpg;
 	struct usb_function	function;
 	u16			iface;
+	struct work_struct	delayed_set_alt;
+	unsigned int		delayed_alt;
 
 	u32			flags;
 #define USBG_ENABLED		(1 << 0)
-- 
2.43.0

Re: [PATCH] usb: gadget: f_tcm: Cancel delayed set_alt work on teardown

[Thinh Nguyen]

On Tue, Jun 23, 2026, Cen Zhang wrote:
> tcm_set_alt() defers BOT/UAS endpoint setup to a system workqueue and
> returns USB_GADGET_DELAYED_STATUS. The queued work keeps the function
> state alive only by a raw struct f_uas pointer, but configfs unlink can
> unbind and free that function before the work runs.
> 
> The buggy scenario involves two paths, with each column showing the order
> within that path:
> 
> USB control request path:           configfs unlink path:
>   1. SET_CONFIGURATION or             1. Userspace removes the function
>      SET_INTERFACE reaches               symlink while the gadget is bound.
>      tcm_set_alt().
>   2. tcm_set_alt() queues delayed     2. config_usb_cfg_unlink() forces
>      set_alt work and returns            gadget unregister.
>      USB_GADGET_DELAYED_STATUS.
>   3. tcm_delayed_set_alt() later      3. tcm_unbind() releases descriptors
>      dereferences struct f_uas.          and tcm_free() frees struct f_uas.
> 
> Validation reproduced this kernel report:
> BUG: KASAN: slab-use-after-free in tcm_delayed_set_alt+0x6c/0xef0
> 
> Call Trace:
>  <TASK>
>  dump_stack_lvl+0x66/0xa0
>  print_report+0xce/0x630
>  ? tcm_delayed_set_alt+0x6c/0xef0
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? __virt_addr_valid+0x188/0x320
>  ? tcm_delayed_set_alt+0x6c/0xef0
>  kasan_report+0xe0/0x110
>  ? tcm_delayed_set_alt+0x6c/0xef0
>  tcm_delayed_set_alt+0x6c/0xef0
>  ? __pfx_tcm_delayed_set_alt+0x10/0x10
>  ? process_one_work+0x4cb/0xb90
>  ? rcu_is_watching+0x20/0x50
>  ? tcm_delayed_set_alt+0x9/0xef0
>  process_one_work+0x4d7/0xb90
>  ? __pfx_process_one_work+0x10/0x10
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? __list_add_valid_or_report+0x37/0xf0
>  ? __pfx_tcm_delayed_set_alt+0x10/0x10
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  worker_thread+0x2d8/0x570
>  ? __pfx_worker_thread+0x10/0x10
>  kthread+0x1ad/0x1f0
>  ? __pfx_kthread+0x10/0x10
>  ret_from_fork+0x3c9/0x540
>  ? __pfx_ret_from_fork+0x10/0x10
>  ? srso_alias_return_thunk+0x5/0xfbef5
>  ? __switch_to+0x2e9/0x730
>  ? __pfx_kthread+0x10/0x10
>  ret_from_fork_asm+0x1a/0x30
>  </TASK>
> 
> Allocated by task 544:
>  kasan_save_stack+0x33/0x60
>  kasan_save_track+0x14/0x30
>  __kasan_kmalloc+0x8f/0xa0
>  tcm_alloc+0x68/0x180
>  usb_get_function+0x36/0x60
>  config_usb_cfg_link+0x125/0x1b0
>  configfs_symlink+0x322/0x890
>  vfs_symlink+0xc2/0x270
>  filename_symlinkat+0x295/0x2f0
>  __x64_sys_symlinkat+0x62/0x90
>  do_syscall_64+0x115/0x6a0
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> Freed by task 661:
>  kasan_save_stack+0x33/0x60
>  kasan_save_track+0x14/0x30
>  kasan_save_free_info+0x3b/0x60
>  __kasan_slab_free+0x43/0x70
>  kfree+0x2f9/0x530
>  config_usb_cfg_unlink+0x173/0x1e0
>  configfs_unlink+0x1fa/0x340
>  vfs_unlink+0x15c/0x510
>  filename_unlinkat+0x2ba/0x450
>  __x64_sys_unlinkat+0x63/0x90
>  do_syscall_64+0x115/0x6a0
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> Make the delayed set_alt work item owned by struct f_uas instead of an
> anonymous heap allocation. This gives teardown paths a stable handle for
> the exact work item: disable cancels pending work without sleeping, and
> unbind/free synchronously cancel it before descriptors or struct f_uas are
> released.
> 
> Fixes: c52661d60f63 ("usb-gadget: Initial merge of target module for UASP + BOT")

Cc stable for these kinds of fixes.

> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> ---
>  drivers/usb/gadget/function/f_tcm.c | 32 ++++++++++-------------------
>  drivers/usb/gadget/function/tcm.h   |  2 ++
>  2 files changed, 13 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> index 34d9f49e9987..4096f94586fe 100644
> --- a/drivers/usb/gadget/function/f_tcm.c
> +++ b/drivers/usb/gadget/function/f_tcm.c
> @@ -2363,20 +2363,10 @@ static int tcm_bind(struct usb_configuration *c, struct usb_function *f)
>  	return -ENOTSUPP;
>  }
>  
> -struct guas_setup_wq {
> -	struct work_struct work;
> -	struct f_uas *fu;
> -	unsigned int alt;
> -};
> -
>  static void tcm_delayed_set_alt(struct work_struct *wq)
>  {
> -	struct guas_setup_wq *work = container_of(wq, struct guas_setup_wq,
> -			work);
> -	struct f_uas *fu = work->fu;
> -	int alt = work->alt;
> -
> -	kfree(work);
> +	struct f_uas *fu = container_of(wq, struct f_uas, delayed_set_alt);
> +	int alt = fu->delayed_alt;
>  
>  	if (fu->flags & USBG_IS_BOT)
>  		bot_cleanup_old_alt(fu);
> @@ -2413,15 +2403,8 @@ static int tcm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
>  		return -EOPNOTSUPP;
>  
>  	if ((alt == USB_G_ALT_INT_BBB) || (alt == USB_G_ALT_INT_UAS)) {
> -		struct guas_setup_wq *work;
> -
> -		work = kmalloc_obj(*work, GFP_ATOMIC);
> -		if (!work)
> -			return -ENOMEM;
> -		INIT_WORK(&work->work, tcm_delayed_set_alt);
> -		work->fu = fu;
> -		work->alt = alt;
> -		schedule_work(&work->work);
> +		fu->delayed_alt = alt;
> +		schedule_work(&fu->delayed_set_alt);
>  		return USB_GADGET_DELAYED_STATUS;
>  	}
>  	return -EOPNOTSUPP;
> @@ -2431,6 +2414,8 @@ static void tcm_disable(struct usb_function *f)
>  {
>  	struct f_uas *fu = to_f_uas(f);
>  
> +	cancel_work(&fu->delayed_set_alt);
> +

Couldn't a race still happen here? I don't think you can just use
cancel_work_sync in tcm_disable here.

BR,
Thinh

>  	if (fu->flags & USBG_IS_UAS)
>  		uasp_cleanup_old_alt(fu);
>  	else if (fu->flags & USBG_IS_BOT)
> @@ -2583,11 +2568,15 @@ static void tcm_free(struct usb_function *f)
>  {
>  	struct f_uas *tcm = to_f_uas(f);
>  
> +	cancel_work_sync(&tcm->delayed_set_alt);
>  	kfree(tcm);
>  }
>  
>  static void tcm_unbind(struct usb_configuration *c, struct usb_function *f)
>  {
> +	struct f_uas *tcm = to_f_uas(f);
> +
> +	cancel_work_sync(&tcm->delayed_set_alt);
>  	usb_free_all_descriptors(f);
>  }
>  
> @@ -2621,6 +2610,7 @@ static struct usb_function *tcm_alloc(struct usb_function_instance *fi)
>  	fu->function.free_func = tcm_free;
>  	fu->tpg = tpg_instances[i].tpg;
>  
> +	INIT_WORK(&fu->delayed_set_alt, tcm_delayed_set_alt);
>  	hash_init(fu->stream_hash);
>  	mutex_unlock(&tpg_instances_lock);
>  
> diff --git a/drivers/usb/gadget/function/tcm.h b/drivers/usb/gadget/function/tcm.h
> index 009974d81d66..b07bf4aa7d88 100644
> --- a/drivers/usb/gadget/function/tcm.h
> +++ b/drivers/usb/gadget/function/tcm.h
> @@ -123,6 +123,8 @@ struct f_uas {
>  	struct usbg_tpg		*tpg;
>  	struct usb_function	function;
>  	u16			iface;
> +	struct work_struct	delayed_set_alt;
> +	unsigned int		delayed_alt;
>  
>  	u32			flags;
>  #define USBG_ENABLED		(1 << 0)
> -- 
> 2.43.0

Re: [PATCH] usb: gadget: f_tcm: Cancel delayed set_alt work on teardown

[Cen Zhang]

Hi Thinh,

Thinh Nguyen <Thinh.Nguyen@synopsys.com> 于2026年6月27日周六 08:08写道：
>
> Couldn't a race still happen here? I don't think you can just use
> cancel_work_sync in tcm_disable here.
>

You're right. tcm_disable() can be called while the composite device lock is
held, so using cancel_work_sync() there is not safe.

I reworked this in v2 by using a small delayed-set-alt state machine:
tcm_disable() now only uses a non-sleeping cancellation path. If the work is
still queued, it is cancelled and cleaned up immediately. If it is already
running, it is marked cancelled and the worker owns the cleanup before
returning. cancel_work_sync() is only used from unbind/free paths where
sleeping is safe.

I also added Cc: stable@vger.kernel.org as suggested.

Thanks,
Cen Zhang