Re: [PATCH v2 01/22] usb: typec: Add alternate mode state notifiers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Synchronous notifier execution inside deeply nested TCPM locks causes recursive and AB-BA deadlocks.
--

commit 24aff87629a22c99f4d8e24783e51c35bd321aa1
Author: Sven Peter <sven@kernel.org>

usb: typec: Add alternate mode state notifiers

This commit adds a blocking notifier chain to USB Type-C alternate modes,
triggering synchronously when an alternate mode is activated or deactivated.
It allows drivers (like Apple Silicon USB4/Thunderbolt) to react to cable
details and state changes.

> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 54c7810b563be..5808a3ef0d6cd 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c

[ ... ]

> @@ -312,6 +315,10 @@ void typec_altmode_update_active(struct typec_altmode *adev, bool active)
>  	}
>  
>  	adev->active = active;
> +	blocking_notifier_call_chain(&alt->state_notifier,
> +				     active ? TYPEC_ALTMODE_ENTERED :
> +					      TYPEC_ALTMODE_EXITED,
> +				     adev);

[Severity: High]
Does this synchronous notifier chain execution risk recursive deadlocks or
AB-BA lock inversions with the TCPM driver?

Looking at drivers/usb/typec/tcpm/tcpm.c, tcpm_pd_svdm() calls
typec_altmode_update_active() while already holding port->lock:

tcpm_pd_svdm() {
    mutex_lock(&port->lock);
    ...
    typec_altmode_update_active(pdev, true);
}

Because blocking_notifier_call_chain() executes synchronously, any subscriber
callback will run with TCPM's port->lock held.

If a subscriber callback attempts to interact with the alternate mode
(for example, by calling typec_altmode_enter()), the TCPM altmode operations
such as tcpm_altmode_enter() will attempt to acquire port->lock again,
leading to a direct recursive deadlock. 

Additionally, if subscribers acquire their own hardware locks in the callback,
it could create an AB-BA inversion against port->lock.

Could this notification be deferred (similar to the adev_action deferral
mechanism TCPM uses elsewhere) or executed outside of the port->lock context
to prevent these locking inversions?

>  	snprintf(dir, sizeof(dir), "mode%d", adev->mode);
>  	sysfs_notify(&adev->dev.kobj, dir, "active");
>  	sysfs_notify(&adev->dev.kobj, NULL, "active");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-b4-apple-soc-tbt-v2-0-1f80085f93fb@kernel.org?part=1