[PATCH] usb: typec: tcpm: fix use-after-free of the kthread worker on port unregister

[PATCH] usb: typec: tcpm: fix use-after-free of the kthread worker on port unregister

[Igor Paunovic]

tcpm_unregister_port() destroys the port's kthread worker first and
calls tcpm_reset_port() afterwards.  Since the Discover Identity retry
mechanism was added, tcpm_reset_port() calls
mod_vdm_discovery_cancel_delayed_work(), which does
kthread_cancel_work_sync(&port->vdm_discovery_work).  That dereferences
work->worker, which still points at the worker that
kthread_destroy_worker() has already freed:

  tcpm_unregister_port()
    kthread_destroy_worker(port->wq)      -> kfree(worker)
    ...
    tcpm_reset_port()
      mod_vdm_discovery_cancel_delayed_work()
        kthread_cancel_work_sync(&port->vdm_discovery_work)
          __kthread_cancel_work_sync()
            raw_spin_lock_irqsave(&worker->lock, ...)   <- freed memory

KASAN report on 7.3-rc1 when unbinding a fusb302 port (RK3588):

  BUG: KASAN: slab-use-after-free in _raw_spin_lock_irqsave+0x10c/0x210
  Write of size 4 at addr ffff00010122ef04 by task bash/8349
  Call trace:
   _raw_spin_lock_irqsave+0x10c/0x210
   __kthread_cancel_work_sync+0x60/0x408
   kthread_cancel_work_sync+0x20/0x48
   tcpm_reset_port+0x18c/0xb80 [tcpm]
   tcpm_unregister_port+0x104/0x2f8 [tcpm]
   fusb302_remove+0xc8/0x200 [fusb302]
   i2c_device_remove+0x7c/0x288
   ...
  Allocated by task 112:
   kthread_create_worker_on_node+0x14c/0x2c8
   tcpm_register_port+0x288/0x3918 [tcpm]
   fusb302_probe+0x604/0xc88 [fusb302]
  Freed by task 8349:
   kfree+0x260/0x558
   kthread_destroy_worker+0xa0/0x130
   tcpm_unregister_port+0x74/0x2f8 [tcpm]
   fusb302_remove+0xc8/0x200 [fusb302]

With CONFIG_PROVE_LOCKING the same unbind shows up as
"DEBUG_LOCKS_WARN_ON(lock->magic != lock)" in __lock_acquire, followed
by an oops in the unbinding task, which then exits with interrupts
disabled and the following shutdown hangs.

The work itself cannot be pending at that point: kthread_destroy_worker()
has flushed the worker and the discovery timer is cancelled right before
the cancel call.  So just remember that the worker is gone and skip the
cancel in that case.

Tested on an Orange Pi 5 Plus (RK3588, fusb302) with KASAN: unbinding
the port reports the use-after-free above without this patch and
nothing with it; the port binds again fine afterwards in both cases.

Fixes: 205dc9cb39f5 ("usb: typec: tcpm: implement retry mechanism for Discover Identity VDMs")
Signed-off-by: Igor Paunovic <royalnet026@gmail.com>
Assisted-by: LLM
---
 drivers/usb/typec/tcpm/tcpm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index a8cd1959c426f..e47d674c2ae00 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -1756,7 +1756,8 @@ static void mod_enable_frs_delayed_work(struct tcpm_port *port, unsigned int del
 static void mod_vdm_discovery_cancel_delayed_work(struct tcpm_port *port)
 {
 	hrtimer_cancel(&port->vdm_discovery_timer);
-	kthread_cancel_work_sync(&port->vdm_discovery_work);
+	if (port->wq)
+		kthread_cancel_work_sync(&port->vdm_discovery_work);
 }
 
 static void mod_vdm_discovery_delayed_work(struct tcpm_port *port, unsigned int delay_ms)
@@ -8961,6 +8962,7 @@ void tcpm_unregister_port(struct tcpm_port *port)
 
 	port->registered = false;
 	kthread_destroy_worker(port->wq);
+	port->wq = NULL;
 
 	hrtimer_cancel(&port->vdm_discovery_timer);
 	hrtimer_cancel(&port->enable_frs_timer);
-- 
2.43.0