Re: [PATCH 11/19] usb: typec: tcpm: don't wait for the VDM discovery work

[Heikki Krogerus]

On Tue, Oct 27, 2026 at 03:57:51PM +0100, Greg Kroah-Hartman wrote:
> From: Josef Bacik <josef@xxxxxxxxxxxxxx>
> 
> mod_vdm_discovery_cancel_delayed_work() calls kthread_cancel_work_sync()
> with port->lock held, and tcpm_vdm_discovery_work() starts by taking
> that lock.  This is fine as long as the caller runs on the kthread
> worker of the port, like the work.  But tcpm_reset_port() is also
> reached from tcpm_tcpc_reset(), which tcpci_irq() calls from its
> interrupt thread.  If the work is running, it waits for the lock while
> the holder of the lock waits for the work.
> 
> Use a flag instead, protected by port->lock, which is set when the work
> is scheduled and cleared when it is cancelled.  The work returns at once
> if the flag is not set.
> 
> tcpm_unregister_port() calls tcpm_reset_port() after it has destroyed
> the worker.  Commit 98d00deb6ae4 ("usb: typec: tcpm: fix use-after-free
> of the kthread worker on port unregister") put a test of port->wq in
> front of the cancel for that case.  Nothing is cancelled any more, so
> the test goes.
> 
> Fixes: 205dc9cb39f5 ("usb: typec: tcpm: implement retry mechanism for Discover Identity VDMs")
> Cc: stable <stable@xxxxxxxxxx>
> Cc: Badhri Jagan Sridharan <badhri@xxxxxxxxxx>
> Cc: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>

> ---
>  drivers/usb/typec/tcpm/tcpm.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index d721baef5210..600e820a7b93 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -542,6 +542,7 @@ struct tcpm_port {
>  	struct kthread_work enable_frs;
>  	struct hrtimer vdm_discovery_timer;
>  	struct kthread_work vdm_discovery_work;
> +	bool vdm_discovery_armed;
>  	bool state_machine_running;
>  	/* Set to true when VDM State Machine has following actions. */
>  	bool vdm_sm_running;
> @@ -1768,15 +1769,20 @@ static void mod_enable_frs_delayed_work(struct tcpm_port *port, unsigned int del
>  	}
>  }
>  
> +/*
> + * Called with port->lock held, which the work takes, and not only from the
> + * worker: the work cannot be waited for.  It does nothing unless it is armed.
> + */
>  static void mod_vdm_discovery_cancel_delayed_work(struct tcpm_port *port)
>  {
>  	hrtimer_cancel(&port->vdm_discovery_timer);
> -	if (port->wq)
> -		kthread_cancel_work_sync(&port->vdm_discovery_work);
> +	port->vdm_discovery_armed = false;
>  }
>  
>  static void mod_vdm_discovery_delayed_work(struct tcpm_port *port, unsigned int delay_ms)
>  {
> +	port->vdm_discovery_armed = true;
> +
>  	if (delay_ms) {
>  		hrtimer_start(&port->vdm_discovery_timer, ms_to_ktime(delay_ms), HRTIMER_MODE_REL);
>  	} else {
> @@ -7295,6 +7301,10 @@ static void tcpm_vdm_discovery_work(struct kthread_work *work)
>  
>  	mutex_lock(&port->lock);
>  
> +	if (!port->vdm_discovery_armed)
> +		goto unlock;
> +	port->vdm_discovery_armed = false;
> +
>  	tcpm_log_force(port, "%s state [%s]", __func__,
>  		       vdm_discovery_state_strings[port->vdm_discovery_state]);
>  
> 
> -- 
> 2.56.0

-- 
heikki