mirror of https://lore.kernel.org/lkml/
 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: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-07 18:30 Igor Paunovic
2026-09-14 11:43 ` Heikki Krogerus [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=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®