All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
To: Igor Paunovic <royalnet026@gmail.com>
Cc: Badhri Jagan Sridharan <badhri@google.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	RD Babiera <rdbabiera@google.com>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] usb: typec: tcpm: fix use-after-free of the kthread worker on port unregister
Date: Mon, 14 Sep 2026 13:43:33 +0200	[thread overview]
Message-ID: <aqfd5Xm99P4BaTSp@black.igk.intel.com> (raw)
In-Reply-To: <20260907183041.8253-1-royalnet026@gmail.com>

On Mon, Sep 07, 2026 at 08:30:41PM +0200, Igor Paunovic wrote:
> 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

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  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

-- 
heikki

  reply	other threads:[~2026-09-14 11:43 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-07 18:30 [PATCH] usb: typec: tcpm: fix use-after-free of the kthread worker on port unregister Igor Paunovic
2026-09-14 11:43 ` Heikki Krogerus [this message]
2026-09-21 21:42   ` RD Babiera

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=aqfd5Xm99P4BaTSp@black.igk.intel.com \
    --to=heikki.krogerus@linux.intel.com \
    --cc=badhri@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=rdbabiera@google.com \
    --cc=royalnet026@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.