[PATCH] usb: gadget: f_tcm: detach the gadget in usbg_drop_tpg() to fix a use-after-free

[PATCH] usb: gadget: f_tcm: detach the gadget in usbg_drop_tpg() to fix a use-after-free

[Hui Peng]

A target portal group can be removed with rmdir() on its configfs
directory while it is still enabled and still attached to a USB gadget
function. usbg_drop_tpg() frees the struct usbg_tpg but never calls
usbg_detach(), and it leaves opts->can_attach set, so the function
instance keeps a dangling tpg pointer and a subsequent bind to a UDC
happily attaches to freed memory.

Enumerating the gadget then dereferences the freed tpg from the UAS/BOT
completion path, in softirq context:

 ==================================================================
 BUG: KASAN: slab-use-after-free in usbg_submit_command.isra.0+0xbe7/0xd50
 Read of size 8 at addr ffff888107493478 by task swapper/3/0

 CPU: 3 UID: 0 PID: 0 Comm: swapper/3 Tainted: G    B D    7.3.0-rc3-g5dd1818b15d9 #1 PREEMPT(lazy)
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
 Call Trace:
  <IRQ>
  dump_stack_lvl+0x70/0xa0
  print_report+0x153/0x4c6
  kasan_report+0xf1/0x120
  usbg_submit_command.isra.0+0xbe7/0xd50
  uasp_cmd_complete+0x83/0xf0
  dummy_timer+0x1337/0x2d60
  __hrtimer_run_queues+0x2a3/0x640
  hrtimer_run_softirq+0x1b1/0x3d0
  handle_softirqs+0x188/0x4e0
  __irq_exit_rcu+0x62/0x150
  sysvec_apic_timer_interrupt+0x6b/0x80
  </IRQ>

 Allocated by task 1:
  __kmalloc_cache_noprof+0x16a/0x380
  usbg_make_tpg+0x243/0x580
  target_fabric_make_tpg+0xb1/0x7f0
  configfs_mkdir+0x4e9/0xe10
  vfs_mkdir+0x2ed/0x790
  __x64_sys_mkdir+0x6f/0xa0

 Freed by task 1:
  kfree+0x159/0x420
  config_item_cleanup+0x148/0x1e0
  config_item_put+0x90/0xb0
  configfs_rmdir+0x816/0xa50
 ==================================================================

Detach the gadget in usbg_drop_tpg() if the tpg is still connected, and
clear opts->can_attach for every function instance bound to this tpg so
that a later usbg_attach() cannot pick up the freed object.

The forward declarations of usbg_attach() and usbg_detach() are moved
above usbg_drop_tpg() since it now calls usbg_detach().

Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
No Fixes: tag: I could not identify a single commit that introduced
the problem with confidence, so I have left it out rather than guess.

Reproduced on Linux 7.3.0-rc3 (5dd1818b15d9) with KASAN under QEMU using
dummy_hcd: create a tcm function and a target tpgt_1, write 1 to its
"enable" attribute, link the function into a gadget config, rmdir tpgt_1
without disabling it first, then bind the gadget to dummy_udc.0. With
this patch applied the same sequence produces no KASAN splat.

 drivers/usb/gadget/function/f_tcm.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

--- a/drivers/usb/gadget/function/f_tcm.c
+++ b/drivers/usb/gadget/function/f_tcm.c
@@ -1756,6 +1756,9 @@ unlock_inst:
 
 static int tcm_usbg_drop_nexus(struct usbg_tpg *);
 
+static int usbg_attach(struct usbg_tpg *);
+static void usbg_detach(struct usbg_tpg *);
+
 static void usbg_drop_tpg(struct se_portal_group *se_tpg)
 {
 	struct usbg_tpg *tpg = container_of(se_tpg,
@@ -1763,6 +1766,11 @@ static void usbg_drop_tpg(struct se_port
 	unsigned i;
 	struct f_tcm_opts *opts;
 
+	if (tpg->gadget_connect) {
+		usbg_detach(tpg);
+		tpg->gadget_connect = false;
+	}
+
 	tcm_usbg_drop_nexus(tpg);
 	core_tpg_deregister(se_tpg);
 	destroy_workqueue(tpg->workqueue);
@@ -1776,6 +1784,7 @@ static void usbg_drop_tpg(struct se_port
 		opts = container_of(tpg_instances[i].func_inst,
 			struct f_tcm_opts, func_inst);
 		mutex_lock(&opts->dep_lock);
+		opts->can_attach = false;
 		if (opts->has_dep)
 			module_put(opts->dependent);
 		else
@@ -1832,9 +1841,6 @@ static struct configfs_attribute *usbg_w
 	NULL,
 };
 
-static int usbg_attach(struct usbg_tpg *);
-static void usbg_detach(struct usbg_tpg *);
-
 static int usbg_enable_tpg(struct se_portal_group *se_tpg, bool enable)
 {
 	struct usbg_tpg  *tpg = container_of(se_tpg, struct usbg_tpg, se_tpg);
-- 
2.43.0

[PATCH v2] usb: gadget: f_tcm: detach the gadget in usbg_drop_tpg() to fix a use-after-free

[Hui Peng]

A target portal group can be removed with rmdir() on its configfs
directory while it is still enabled and still attached to a USB gadget
function. usbg_drop_tpg() frees the struct usbg_tpg but never calls
usbg_detach(), and it leaves opts->can_attach set, so the function
instance keeps a dangling tpg pointer and a subsequent bind to a UDC
happily attaches to freed memory.

Enumerating the gadget then dereferences the freed tpg from the UAS/BOT
completion path, in softirq context:

 ==================================================================
 BUG: KASAN: slab-use-after-free in usbg_submit_command.isra.0+0xbe7/0xd50
 Read of size 8 at addr ffff888107493478 by task swapper/3/0

 CPU: 3 UID: 0 PID: 0 Comm: swapper/3 Tainted: G    B D    7.3.0-rc3-g5dd1818b15d9 #1 PREEMPT(lazy)
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
 Call Trace:
  <IRQ>
  dump_stack_lvl+0x70/0xa0
  print_report+0x153/0x4c6
  kasan_report+0xf1/0x120
  usbg_submit_command.isra.0+0xbe7/0xd50
  uasp_cmd_complete+0x83/0xf0
  dummy_timer+0x1337/0x2d60
  __hrtimer_run_queues+0x2a3/0x640
  hrtimer_run_softirq+0x1b1/0x3d0
  handle_softirqs+0x188/0x4e0
  __irq_exit_rcu+0x62/0x150
  sysvec_apic_timer_interrupt+0x6b/0x80
  </IRQ>

 Allocated by task 1:
  __kmalloc_cache_noprof+0x16a/0x380
  usbg_make_tpg+0x243/0x580
  target_fabric_make_tpg+0xb1/0x7f0
  configfs_mkdir+0x4e9/0xe10
  vfs_mkdir+0x2ed/0x790
  __x64_sys_mkdir+0x6f/0xa0

 Freed by task 1:
  kfree+0x159/0x420
  config_item_cleanup+0x148/0x1e0
  config_item_put+0x90/0xb0
  configfs_rmdir+0x816/0xa50
 ==================================================================

Detach the gadget in usbg_drop_tpg() if the tpg is still connected, and
clear opts->can_attach for every function instance bound to this tpg so
that a later usbg_attach() cannot pick up the freed object.

The forward declarations of usbg_attach() and usbg_detach() are moved
above usbg_drop_tpg() since it now calls usbg_detach().

Fixes: dc8c46a5ae77 ("usb: gadget: f_tcm: convert to new function interface with backward compatibility")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
v2: Add the Fixes: tag that v1 said it could not determine.

dc8c46a5ae77 introduced opts->can_attach, the !opts->can_attach gate in
tcm_bind(), usbg_attach()/usbg_detach(), the tpg_instances[] table and
fu->tpg, and the version of usbg_drop_tpg() that tears all of that down
without ever detaching - i.e. every line this patch touches. Note
git blame points at 08a1cb0f65fd ("usb: gadget: tcm: factor out
f_tcm") for the top of usbg_drop_tpg(), but that commit is a pure file
split out of legacy/tcm_usb_gadget.c, so it is not the tag.

There is a defensible alternative: 4bb8548df632 ("usb: gadget: f_tcm:
add configfs support") added the callbacks that set can_attach for a
pure-configfs gadget, which is what my reproducer uses. I went with
dc8c46a5ae77 because the legacy gadget already set can_attach
unconditionally, so the hole was reachable without 4bb8548df632 - and
in any case both landed in v4.5-rc1, five patches apart in the same
series, so the backport target is identical. Happy to switch if you
prefer the other.

For completeness, the underlying "free the tpg without detaching"
shape is as old as c52661d60f63 ("usb-gadget: Initial merge of target
module for UASP + BOT", v3.5-rc1), but f_tcm.c does not exist before
v4.5-rc1 and opts->can_attach - which this patch has to clear - did
not either, so dc8c46a5ae77 is the oldest commit this patch is
actually applicable to.

Two things worth a reviewer's attention: tpg->gadget_connect is read
here without the target-core configfs locking that usbg_enable_tpg()
writes it under, and the opts->can_attach = false lands inside the
if (i < TPG_INSTANCES) arm added by e877b729c649 ("usb: gadget: f_tcm:
out of bound access in usbg_drop_tpg"). If you would rather refuse the
rmdir than auto-detach, note that fabric_drop_tpg returns void, so it
cannot be failed from here.

Reproduced on Linux 7.3.0-rc3 (5dd1818b15d9) with KASAN under QEMU using
dummy_hcd: create a tcm function and a target tpgt_1, write 1 to its
"enable" attribute, link the function into a gadget config, rmdir tpgt_1
without disabling it first, then bind the gadget to dummy_udc.0. With
this patch applied the same sequence produces no KASAN splat.

 drivers/usb/gadget/function/f_tcm.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

--- a/drivers/usb/gadget/function/f_tcm.c
+++ b/drivers/usb/gadget/function/f_tcm.c
@@ -1756,6 +1756,9 @@ unlock_inst:
 
 static int tcm_usbg_drop_nexus(struct usbg_tpg *);
 
+static int usbg_attach(struct usbg_tpg *);
+static void usbg_detach(struct usbg_tpg *);
+
 static void usbg_drop_tpg(struct se_portal_group *se_tpg)
 {
 	struct usbg_tpg *tpg = container_of(se_tpg,
@@ -1763,6 +1766,11 @@ static void usbg_drop_tpg(struct se_port
 	unsigned i;
 	struct f_tcm_opts *opts;
 
+	if (tpg->gadget_connect) {
+		usbg_detach(tpg);
+		tpg->gadget_connect = false;
+	}
+
 	tcm_usbg_drop_nexus(tpg);
 	core_tpg_deregister(se_tpg);
 	destroy_workqueue(tpg->workqueue);
@@ -1776,6 +1784,7 @@ static void usbg_drop_tpg(struct se_port
 		opts = container_of(tpg_instances[i].func_inst,
 			struct f_tcm_opts, func_inst);
 		mutex_lock(&opts->dep_lock);
+		opts->can_attach = false;
 		if (opts->has_dep)
 			module_put(opts->dependent);
 		else
@@ -1832,9 +1841,6 @@ static struct configfs_attribute *usbg_w
 	NULL,
 };
 
-static int usbg_attach(struct usbg_tpg *);
-static void usbg_detach(struct usbg_tpg *);
-
 static int usbg_enable_tpg(struct se_portal_group *se_tpg, bool enable)
 {
 	struct usbg_tpg  *tpg = container_of(se_tpg, struct usbg_tpg, se_tpg);
-- 
2.43.0