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
prev parent reply other threads:[~2026-06-27 0:09 UTC|newest]
Thread overview: 2+ 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]
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.