The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v2] usb: gadget: f_tcm: synchronize delayed set_alt with teardown
@ 2026-06-27 10:41 Cen Zhang
  2026-07-11  1:35 ` Thinh Nguyen
  0 siblings, 1 reply; 2+ messages in thread
From: Cen Zhang @ 2026-06-27 10:41 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiasheng Jiang, Thinh Nguyen,
	Martin K . Petersen, Christophe JAILLET, Kees Cook, Mike Christie,
	Nicholas Bellinger, Felipe Balbi, Sebastian Andrzej Siewior
  Cc: linux-usb, linux-kernel, baijiaju1990, zzzccc427, stable

The f_tcm set_alt() path defers endpoint setup to a work item and
completes the delayed status response from process context. The delayed
work uses f_tcm private state and may complete the setup request after
disconnect or function teardown has already moved on.

Cancel and drain the delayed set_alt work when the function is unbound or
freed. For disable paths, which are reached under the composite device
lock, use a small state machine and a non-sleeping cancellation path
instead of cancel_work_sync(). If the work is already running, mark it
cancelled and let the worker own the cleanup; otherwise tcm_disable() can
cancel the queued work and clean up immediately.

Also serialize the final delayed-status completion with the cancellation
check while holding the composite device lock. This prevents a disconnect
from clearing delayed_status while the worker is about to complete the
control request.

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

Fixes: c52661d60f63 ("usb-gadget: Initial merge of target module for UASP + BOT")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.5
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
v2:
Add Cc: stable@vger.kernel.org.
Replace the posted cancel_work()-only disable path with the
workflow-reviewed delayed-set-alt state machine.
Keep tcm_disable() non-sleeping and reserve cancel_work_sync() for
unbind/free teardown paths.

 drivers/usb/gadget/function/f_tcm.c | 192 ++++++++++++++++++++++++----
 drivers/usb/gadget/function/tcm.h   |  13 ++
 2 files changed, 177 insertions(+), 28 deletions(-)

diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
index 34d9f49e9987..b3fa5a17fd2d 100644
--- a/drivers/usb/gadget/function/f_tcm.c
+++ b/drivers/usb/gadget/function/f_tcm.c
@@ -2363,31 +2363,158 @@ 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_cleanup_old_alt(struct f_uas *fu)
+{
+	if (fu->flags & USBG_IS_UAS)
+		uasp_cleanup_old_alt(fu);
+	else if (fu->flags & USBG_IS_BOT)
+		bot_cleanup_old_alt(fu);
+	fu->flags = 0;
+}
+
+static void tcm_delayed_set_alt_done(struct f_uas *fu)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&fu->delayed_set_alt_lock, flags);
+	fu->delayed_set_alt_state = USBG_DELAYED_SET_ALT_IDLE;
+	fu->delayed_set_alt_cancel = false;
+	spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
+}
+
+static bool tcm_delayed_set_alt_cancelled(struct f_uas *fu)
+{
+	bool cancelled;
+	unsigned long flags;
+
+	spin_lock_irqsave(&fu->delayed_set_alt_lock, flags);
+	cancelled = fu->delayed_set_alt_cancel;
+	spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
+
+	return cancelled;
+}
+
+static bool tcm_complete_delayed_status(struct f_uas *fu)
+{
+	struct usb_composite_dev *cdev = fu->function.config->cdev;
+	struct usb_request *req = cdev->req;
+	unsigned long cdev_flags;
+	bool cancelled;
+	int ret;
+
+	spin_lock_irqsave(&cdev->lock, cdev_flags);
+	spin_lock(&fu->delayed_set_alt_lock);
+	cancelled = fu->delayed_set_alt_cancel;
+	if (!cancelled) {
+		fu->delayed_set_alt_state = USBG_DELAYED_SET_ALT_IDLE;
+		fu->delayed_set_alt_cancel = false;
+	}
+	spin_unlock(&fu->delayed_set_alt_lock);
+
+	if (cancelled) {
+		spin_unlock_irqrestore(&cdev->lock, cdev_flags);
+		return false;
+	}
+
+	if (cdev->delayed_status == 0) {
+		WARN(cdev, "%s: Unexpected call\n", __func__);
+	} else if (--cdev->delayed_status == 0) {
+		req->length = 0;
+		req->context = cdev;
+		ret = usb_ep_queue(cdev->gadget->ep0, req, GFP_ATOMIC);
+		if (ret == 0) {
+			cdev->setup_pending = true;
+		} else {
+			req->status = 0;
+			req->complete(cdev->gadget->ep0, req);
+		}
+	}
+
+	spin_unlock_irqrestore(&cdev->lock, cdev_flags);
+
+	return true;
+}
+
+static bool tcm_cancel_delayed_set_alt(struct f_uas *fu)
+{
+	bool cleanup = false;
+	bool cancel = false;
+	unsigned long flags;
+
+	spin_lock_irqsave(&fu->delayed_set_alt_lock, flags);
+	switch (fu->delayed_set_alt_state) {
+	case USBG_DELAYED_SET_ALT_IDLE:
+		cleanup = true;
+		break;
+	case USBG_DELAYED_SET_ALT_QUEUED:
+	case USBG_DELAYED_SET_ALT_RUNNING:
+		fu->delayed_set_alt_cancel = true;
+		cancel = true;
+		break;
+	}
+	spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
+
+	if (cancel && cancel_work(&fu->delayed_set_alt)) {
+		spin_lock_irqsave(&fu->delayed_set_alt_lock, flags);
+		if (fu->delayed_set_alt_state == USBG_DELAYED_SET_ALT_QUEUED) {
+			fu->delayed_set_alt_state = USBG_DELAYED_SET_ALT_IDLE;
+			fu->delayed_set_alt_cancel = false;
+			cleanup = true;
+		}
+		spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
+	}
+
+	return cleanup;
+}
+
+static void tcm_cancel_delayed_set_alt_sync(struct f_uas *fu)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&fu->delayed_set_alt_lock, flags);
+	if (fu->delayed_set_alt_state != USBG_DELAYED_SET_ALT_IDLE)
+		fu->delayed_set_alt_cancel = true;
+	spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
+
+	cancel_work_sync(&fu->delayed_set_alt);
+
+	spin_lock_irqsave(&fu->delayed_set_alt_lock, flags);
+	fu->delayed_set_alt_state = USBG_DELAYED_SET_ALT_IDLE;
+	fu->delayed_set_alt_cancel = false;
+	spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
+}
 
 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;
+	struct f_uas *fu = container_of(wq, struct f_uas, delayed_set_alt);
+	unsigned long flags;
+	unsigned int alt;
 
-	kfree(work);
+	spin_lock_irqsave(&fu->delayed_set_alt_lock, flags);
+	if (fu->delayed_set_alt_state != USBG_DELAYED_SET_ALT_QUEUED) {
+		spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
+		return;
+	}
+	fu->delayed_set_alt_state = USBG_DELAYED_SET_ALT_RUNNING;
+	alt = fu->delayed_alt;
+	spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
 
-	if (fu->flags & USBG_IS_BOT)
-		bot_cleanup_old_alt(fu);
-	if (fu->flags & USBG_IS_UAS)
-		uasp_cleanup_old_alt(fu);
+	tcm_cleanup_old_alt(fu);
+
+	if (tcm_delayed_set_alt_cancelled(fu))
+		goto out_done;
 
 	if (alt == USB_G_ALT_INT_BBB)
 		bot_set_alt(fu);
 	else if (alt == USB_G_ALT_INT_UAS)
 		uasp_set_alt(fu);
-	usb_composite_setup_continue(fu->function.config->cdev);
+
+	if (tcm_complete_delayed_status(fu))
+		return;
+
+	tcm_cleanup_old_alt(fu);
+out_done:
+	tcm_delayed_set_alt_done(fu);
 }
 
 static int tcm_get_alt(struct usb_function *f, unsigned intf)
@@ -2413,15 +2540,20 @@ 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;
+		unsigned long flags;
 
-		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);
+		spin_lock_irqsave(&fu->delayed_set_alt_lock, flags);
+		if (fu->delayed_set_alt_state != USBG_DELAYED_SET_ALT_IDLE) {
+			spin_unlock_irqrestore(&fu->delayed_set_alt_lock,
+					       flags);
+			return -EBUSY;
+		}
+		fu->delayed_alt = alt;
+		fu->delayed_set_alt_cancel = false;
+		fu->delayed_set_alt_state = USBG_DELAYED_SET_ALT_QUEUED;
+		spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
+
+		schedule_work(&fu->delayed_set_alt);
 		return USB_GADGET_DELAYED_STATUS;
 	}
 	return -EOPNOTSUPP;
@@ -2431,11 +2563,8 @@ static void tcm_disable(struct usb_function *f)
 {
 	struct f_uas *fu = to_f_uas(f);
 
-	if (fu->flags & USBG_IS_UAS)
-		uasp_cleanup_old_alt(fu);
-	else if (fu->flags & USBG_IS_BOT)
-		bot_cleanup_old_alt(fu);
-	fu->flags = 0;
+	if (tcm_cancel_delayed_set_alt(fu))
+		tcm_cleanup_old_alt(fu);
 }
 
 static int tcm_setup(struct usb_function *f,
@@ -2583,11 +2712,16 @@ static void tcm_free(struct usb_function *f)
 {
 	struct f_uas *tcm = to_f_uas(f);
 
+	tcm_cancel_delayed_set_alt_sync(tcm);
 	kfree(tcm);
 }
 
 static void tcm_unbind(struct usb_configuration *c, struct usb_function *f)
 {
+	struct f_uas *fu = to_f_uas(f);
+
+	tcm_cancel_delayed_set_alt_sync(fu);
+	tcm_cleanup_old_alt(fu);
 	usb_free_all_descriptors(f);
 }
 
@@ -2620,6 +2754,8 @@ static struct usb_function *tcm_alloc(struct usb_function_instance *fi)
 	fu->function.disable = tcm_disable;
 	fu->function.free_func = tcm_free;
 	fu->tpg = tpg_instances[i].tpg;
+	INIT_WORK(&fu->delayed_set_alt, tcm_delayed_set_alt);
+	spin_lock_init(&fu->delayed_set_alt_lock);
 
 	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..e1d5a9391612 100644
--- a/drivers/usb/gadget/function/tcm.h
+++ b/drivers/usb/gadget/function/tcm.h
@@ -3,6 +3,7 @@
 #define __TARGET_USB_GADGET_H__
 
 #include <linux/kref.h>
+#include <linux/spinlock.h>
 /* #include <linux/usb/uas.h> */
 #include <linux/hashtable.h>
 #include <linux/usb/composite.h>
@@ -29,6 +30,12 @@ enum {
 
 #define USB_G_DEFAULT_SESSION_TAGS	USBG_NUM_CMDS
 
+enum {
+	USBG_DELAYED_SET_ALT_IDLE = 0,
+	USBG_DELAYED_SET_ALT_QUEUED,
+	USBG_DELAYED_SET_ALT_RUNNING,
+};
+
 struct tcm_usbg_nexus {
 	struct se_session *tvn_se_sess;
 };
@@ -132,6 +139,12 @@ struct f_uas {
 #define USBG_BOT_CMD_PEND	(1 << 4)
 #define USBG_BOT_WEDGED		(1 << 5)
 
+	struct work_struct	delayed_set_alt;
+	spinlock_t		delayed_set_alt_lock; /* protects delayed_set_alt_* */
+	unsigned int		delayed_alt;
+	unsigned int		delayed_set_alt_state;
+	bool			delayed_set_alt_cancel;
+
 	struct usbg_cdb		cmd[USBG_NUM_CMDS];
 	struct usb_ep		*ep_in;
 	struct usb_ep		*ep_out;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] usb: gadget: f_tcm: synchronize delayed set_alt with teardown
  2026-06-27 10:41 [PATCH v2] usb: gadget: f_tcm: synchronize delayed set_alt with teardown Cen Zhang
@ 2026-07-11  1:35 ` Thinh Nguyen
  0 siblings, 0 replies; 2+ messages in thread
From: Thinh Nguyen @ 2026-07-11  1:35 UTC (permalink / raw)
  To: Cen Zhang
  Cc: Greg Kroah-Hartman, Jiasheng Jiang, Thinh Nguyen,
	Martin K  . Petersen, Christophe JAILLET, Kees Cook,
	Mike Christie, Nicholas Bellinger, Felipe Balbi,
	Sebastian Andrzej Siewior, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org, baijiaju1990@gmail.com,
	stable@vger.kernel.org

On Sat, Jun 27, 2026, Cen Zhang wrote:
> The f_tcm set_alt() path defers endpoint setup to a work item and
> completes the delayed status response from process context. The delayed
> work uses f_tcm private state and may complete the setup request after
> disconnect or function teardown has already moved on.
> 
> Cancel and drain the delayed set_alt work when the function is unbound or
> freed. For disable paths, which are reached under the composite device
> lock, use a small state machine and a non-sleeping cancellation path
> instead of cancel_work_sync(). If the work is already running, mark it
> cancelled and let the worker own the cleanup; otherwise tcm_disable() can
> cancel the queued work and clean up immediately.
> 
> Also serialize the final delayed-status completion with the cancellation
> check while holding the composite device lock. This prevents a disconnect
> from clearing delayed_status while the worker is about to complete the
> control request.
> 
> 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
> 
> Fixes: c52661d60f63 ("usb-gadget: Initial merge of target module for UASP + BOT")
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> ---
> v2:
> Add Cc: stable@vger.kernel.org.
> Replace the posted cancel_work()-only disable path with the
> workflow-reviewed delayed-set-alt state machine.
> Keep tcm_disable() non-sleeping and reserve cancel_work_sync() for
> unbind/free teardown paths.
> 
>  drivers/usb/gadget/function/f_tcm.c | 192 ++++++++++++++++++++++++----
>  drivers/usb/gadget/function/tcm.h   |  13 ++
>  2 files changed, 177 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_tcm.c b/drivers/usb/gadget/function/f_tcm.c
> index 34d9f49e9987..b3fa5a17fd2d 100644
> --- a/drivers/usb/gadget/function/f_tcm.c
> +++ b/drivers/usb/gadget/function/f_tcm.c
> @@ -2363,31 +2363,158 @@ 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_cleanup_old_alt(struct f_uas *fu)
> +{
> +	if (fu->flags & USBG_IS_UAS)
> +		uasp_cleanup_old_alt(fu);
> +	else if (fu->flags & USBG_IS_BOT)
> +		bot_cleanup_old_alt(fu);
> +	fu->flags = 0;
> +}
> +
> +static void tcm_delayed_set_alt_done(struct f_uas *fu)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&fu->delayed_set_alt_lock, flags);
> +	fu->delayed_set_alt_state = USBG_DELAYED_SET_ALT_IDLE;
> +	fu->delayed_set_alt_cancel = false;
> +	spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
> +}
> +
> +static bool tcm_delayed_set_alt_cancelled(struct f_uas *fu)
> +{
> +	bool cancelled;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&fu->delayed_set_alt_lock, flags);
> +	cancelled = fu->delayed_set_alt_cancel;
> +	spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
> +
> +	return cancelled;
> +}
> +
> +static bool tcm_complete_delayed_status(struct f_uas *fu)
> +{
> +	struct usb_composite_dev *cdev = fu->function.config->cdev;
> +	struct usb_request *req = cdev->req;
> +	unsigned long cdev_flags;
> +	bool cancelled;
> +	int ret;
> +
> +	spin_lock_irqsave(&cdev->lock, cdev_flags);
> +	spin_lock(&fu->delayed_set_alt_lock);
> +	cancelled = fu->delayed_set_alt_cancel;
> +	if (!cancelled) {
> +		fu->delayed_set_alt_state = USBG_DELAYED_SET_ALT_IDLE;
> +		fu->delayed_set_alt_cancel = false;
> +	}
> +	spin_unlock(&fu->delayed_set_alt_lock);
> +
> +	if (cancelled) {
> +		spin_unlock_irqrestore(&cdev->lock, cdev_flags);
> +		return false;
> +	}
> +
> +	if (cdev->delayed_status == 0) {
> +		WARN(cdev, "%s: Unexpected call\n", __func__);
> +	} else if (--cdev->delayed_status == 0) {
> +		req->length = 0;
> +		req->context = cdev;
> +		ret = usb_ep_queue(cdev->gadget->ep0, req, GFP_ATOMIC);
> +		if (ret == 0) {
> +			cdev->setup_pending = true;
> +		} else {
> +			req->status = 0;
> +			req->complete(cdev->gadget->ep0, req);
> +		}
> +	}
> +
> +	spin_unlock_irqrestore(&cdev->lock, cdev_flags);
> +
> +	return true;
> +}
> +
> +static bool tcm_cancel_delayed_set_alt(struct f_uas *fu)
> +{
> +	bool cleanup = false;
> +	bool cancel = false;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&fu->delayed_set_alt_lock, flags);
> +	switch (fu->delayed_set_alt_state) {
> +	case USBG_DELAYED_SET_ALT_IDLE:
> +		cleanup = true;
> +		break;
> +	case USBG_DELAYED_SET_ALT_QUEUED:
> +	case USBG_DELAYED_SET_ALT_RUNNING:
> +		fu->delayed_set_alt_cancel = true;
> +		cancel = true;
> +		break;
> +	}
> +	spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
> +
> +	if (cancel && cancel_work(&fu->delayed_set_alt)) {
> +		spin_lock_irqsave(&fu->delayed_set_alt_lock, flags);
> +		if (fu->delayed_set_alt_state == USBG_DELAYED_SET_ALT_QUEUED) {
> +			fu->delayed_set_alt_state = USBG_DELAYED_SET_ALT_IDLE;
> +			fu->delayed_set_alt_cancel = false;
> +			cleanup = true;
> +		}
> +		spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
> +	}
> +
> +	return cleanup;
> +}
> +
> +static void tcm_cancel_delayed_set_alt_sync(struct f_uas *fu)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&fu->delayed_set_alt_lock, flags);
> +	if (fu->delayed_set_alt_state != USBG_DELAYED_SET_ALT_IDLE)
> +		fu->delayed_set_alt_cancel = true;
> +	spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
> +
> +	cancel_work_sync(&fu->delayed_set_alt);
> +
> +	spin_lock_irqsave(&fu->delayed_set_alt_lock, flags);
> +	fu->delayed_set_alt_state = USBG_DELAYED_SET_ALT_IDLE;
> +	fu->delayed_set_alt_cancel = false;
> +	spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
> +}
>  
>  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;
> +	struct f_uas *fu = container_of(wq, struct f_uas, delayed_set_alt);
> +	unsigned long flags;
> +	unsigned int alt;
>  
> -	kfree(work);
> +	spin_lock_irqsave(&fu->delayed_set_alt_lock, flags);
> +	if (fu->delayed_set_alt_state != USBG_DELAYED_SET_ALT_QUEUED) {
> +		spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
> +		return;
> +	}
> +	fu->delayed_set_alt_state = USBG_DELAYED_SET_ALT_RUNNING;
> +	alt = fu->delayed_alt;
> +	spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
>  
> -	if (fu->flags & USBG_IS_BOT)
> -		bot_cleanup_old_alt(fu);
> -	if (fu->flags & USBG_IS_UAS)
> -		uasp_cleanup_old_alt(fu);
> +	tcm_cleanup_old_alt(fu);
> +
> +	if (tcm_delayed_set_alt_cancelled(fu))
> +		goto out_done;
>  
>  	if (alt == USB_G_ALT_INT_BBB)
>  		bot_set_alt(fu);
>  	else if (alt == USB_G_ALT_INT_UAS)
>  		uasp_set_alt(fu);
> -	usb_composite_setup_continue(fu->function.config->cdev);
> +
> +	if (tcm_complete_delayed_status(fu))
> +		return;
> +
> +	tcm_cleanup_old_alt(fu);
> +out_done:
> +	tcm_delayed_set_alt_done(fu);
>  }
>  
>  static int tcm_get_alt(struct usb_function *f, unsigned intf)
> @@ -2413,15 +2540,20 @@ 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;
> +		unsigned long flags;
>  
> -		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);
> +		spin_lock_irqsave(&fu->delayed_set_alt_lock, flags);
> +		if (fu->delayed_set_alt_state != USBG_DELAYED_SET_ALT_IDLE) {
> +			spin_unlock_irqrestore(&fu->delayed_set_alt_lock,
> +					       flags);
> +			return -EBUSY;
> +		}
> +		fu->delayed_alt = alt;
> +		fu->delayed_set_alt_cancel = false;
> +		fu->delayed_set_alt_state = USBG_DELAYED_SET_ALT_QUEUED;
> +		spin_unlock_irqrestore(&fu->delayed_set_alt_lock, flags);
> +
> +		schedule_work(&fu->delayed_set_alt);
>  		return USB_GADGET_DELAYED_STATUS;
>  	}
>  	return -EOPNOTSUPP;
> @@ -2431,11 +2563,8 @@ static void tcm_disable(struct usb_function *f)
>  {
>  	struct f_uas *fu = to_f_uas(f);
>  
> -	if (fu->flags & USBG_IS_UAS)
> -		uasp_cleanup_old_alt(fu);
> -	else if (fu->flags & USBG_IS_BOT)
> -		bot_cleanup_old_alt(fu);
> -	fu->flags = 0;
> +	if (tcm_cancel_delayed_set_alt(fu))
> +		tcm_cleanup_old_alt(fu);
>  }
>  
>  static int tcm_setup(struct usb_function *f,
> @@ -2583,11 +2712,16 @@ static void tcm_free(struct usb_function *f)
>  {
>  	struct f_uas *tcm = to_f_uas(f);
>  
> +	tcm_cancel_delayed_set_alt_sync(tcm);
>  	kfree(tcm);
>  }
>  
>  static void tcm_unbind(struct usb_configuration *c, struct usb_function *f)
>  {
> +	struct f_uas *fu = to_f_uas(f);
> +
> +	tcm_cancel_delayed_set_alt_sync(fu);
> +	tcm_cleanup_old_alt(fu);
>  	usb_free_all_descriptors(f);
>  }
>  
> @@ -2620,6 +2754,8 @@ static struct usb_function *tcm_alloc(struct usb_function_instance *fi)
>  	fu->function.disable = tcm_disable;
>  	fu->function.free_func = tcm_free;
>  	fu->tpg = tpg_instances[i].tpg;
> +	INIT_WORK(&fu->delayed_set_alt, tcm_delayed_set_alt);
> +	spin_lock_init(&fu->delayed_set_alt_lock);
>  
>  	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..e1d5a9391612 100644
> --- a/drivers/usb/gadget/function/tcm.h
> +++ b/drivers/usb/gadget/function/tcm.h
> @@ -3,6 +3,7 @@
>  #define __TARGET_USB_GADGET_H__
>  
>  #include <linux/kref.h>
> +#include <linux/spinlock.h>
>  /* #include <linux/usb/uas.h> */
>  #include <linux/hashtable.h>
>  #include <linux/usb/composite.h>
> @@ -29,6 +30,12 @@ enum {
>  
>  #define USB_G_DEFAULT_SESSION_TAGS	USBG_NUM_CMDS
>  
> +enum {
> +	USBG_DELAYED_SET_ALT_IDLE = 0,
> +	USBG_DELAYED_SET_ALT_QUEUED,
> +	USBG_DELAYED_SET_ALT_RUNNING,
> +};
> +
>  struct tcm_usbg_nexus {
>  	struct se_session *tvn_se_sess;
>  };
> @@ -132,6 +139,12 @@ struct f_uas {
>  #define USBG_BOT_CMD_PEND	(1 << 4)
>  #define USBG_BOT_WEDGED		(1 << 5)
>  
> +	struct work_struct	delayed_set_alt;
> +	spinlock_t		delayed_set_alt_lock; /* protects delayed_set_alt_* */
> +	unsigned int		delayed_alt;
> +	unsigned int		delayed_set_alt_state;
> +	bool			delayed_set_alt_cancel;
> +
>  	struct usbg_cdb		cmd[USBG_NUM_CMDS];
>  	struct usb_ep		*ep_in;
>  	struct usb_ep		*ep_out;
> -- 
> 2.43.0
> 

Yikes. This turned out to be more complicated than I hoped...

All these state checks should be done in the composite framework..
perhaps in the future (something like adding
usb_function_queue_set_alt()).

Regardless, thanks for the patch.

Reviewed-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>

Thanks,
Thinh

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-07-11  1:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-27 10:41 [PATCH v2] usb: gadget: f_tcm: synchronize delayed set_alt with teardown Cen Zhang
2026-07-11  1:35 ` Thinh Nguyen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox