From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Cen Zhang <zzzccc427@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jiasheng Jiang <jiashengjiangcool@gmail.com>,
Kees Cook <kees@kernel.org>,
Mike Christie <michael.christie@oracle.com>,
"Martin K . Petersen" <martin.petersen@oracle.com>,
Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
Thinh Nguyen <Thinh.Nguyen@synopsys.com>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"baijiaju1990@gmail.com" <baijiaju1990@gmail.com>
Subject: Re: [PATCH] usb: gadget: f_tcm: Cancel delayed set_alt work on teardown
Date: Sat, 27 Jun 2026 00:08:29 +0000 [thread overview]
Message-ID: <aj8S2btHtbyjvbCB@vbox> (raw)
In-Reply-To: <20260623015211.4111938-1-zzzccc427@gmail.com>
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
next prev parent reply other threads:[~2026-06-27 0:09 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-23 1:52 [PATCH] usb: gadget: f_tcm: Cancel delayed set_alt work on teardown Cen Zhang
2026-06-27 0:08 ` Thinh Nguyen [this message]
2026-06-27 10:39 ` Cen Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aj8S2btHtbyjvbCB@vbox \
--to=thinh.nguyen@synopsys.com \
--cc=baijiaju1990@gmail.com \
--cc=christophe.jaillet@wanadoo.fr \
--cc=gregkh@linuxfoundation.org \
--cc=jiashengjiangcool@gmail.com \
--cc=kees@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=michael.christie@oracle.com \
--cc=zzzccc427@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox