[PATCH] usb: typec: ucsi: yoga-c630: unregister EC notifier before UCSI teardown

[PATCH] usb: typec: ucsi: yoga-c630: unregister EC notifier before UCSI teardown

[Fan Wu]

Both yoga_c630_ucsi_remove() and the probe error path call
ucsi_unregister() while the EC notifier is still registered.
ucsi_unregister() frees the connector array, so an EC event (USB, HPD or
UCSI) arriving before yoga_c630_ec_unregister_notify() makes
yoga_c630_ucsi_notify() run on freed memory: typec_set_orientation() on the
freed port and ucsi_connector_change() queuing work in the freed connector
array. USB and HPD events do not depend on the UCSI notification-enable
state, so the disable command ucsi_unregister() sends to the EC does not
close this window.

Fix this by unregistering the EC notifier before ucsi_unregister() in both
paths, restoring the teardown ordering from before the error and remove
paths were reworked. blocking_notifier_chain_unregister() waits for a
running callback, so once it returns the EC interrupt thread can no longer
reach the UCSI instance and ucsi_unregister() can tear the connectors down
undisturbed.

The callback is also the only external source that requeues the connector
work, so the cancel_work_sync() calls in ucsi_unregister() can no longer
race with a new connector-change event.

This issue was found by an in-house static analysis tool.

Fixes: 168c3896f32e ("usb: typec: ucsi: yoga-c630: fix error and remove paths")
Cc: stable@vger.kernel.org
Co-developed-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/usb/typec/ucsi/ucsi_yoga_c630.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
index 1be18d1..59738dc 100644
--- a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
+++ b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
@@ -296,7 +296,9 @@ static int yoga_c630_ucsi_probe(struct auxiliary_device *adev,
 	return 0;
 
 err_ucsi_unregister:
+	yoga_c630_ec_unregister_notify(uec->ec, &uec->nb);
 	ucsi_unregister(uec->ucsi);
+	goto err_destroy;
 
 err_unregister:
 	yoga_c630_ec_unregister_notify(uec->ec, &uec->nb);
@@ -311,8 +313,8 @@ static void yoga_c630_ucsi_remove(struct auxiliary_device *adev)
 {
 	struct yoga_c630_ucsi *uec = auxiliary_get_drvdata(adev);
 
-	ucsi_unregister(uec->ucsi);
 	yoga_c630_ec_unregister_notify(uec->ec, &uec->nb);
+	ucsi_unregister(uec->ucsi);
 	ucsi_destroy(uec->ucsi);
 }
 

Re: [PATCH] usb: typec: ucsi: yoga-c630: unregister EC notifier before UCSI teardown

[Dmitry Baryshkov]

On Wed, Sep 23, 2026 at 07:48:28AM +0000, Fan Wu wrote:
> Both yoga_c630_ucsi_remove() and the probe error path call
> ucsi_unregister() while the EC notifier is still registered.
> ucsi_unregister() frees the connector array, so an EC event (USB, HPD or
> UCSI) arriving before yoga_c630_ec_unregister_notify() makes
> yoga_c630_ucsi_notify() run on freed memory: typec_set_orientation() on the
> freed port and ucsi_connector_change() queuing work in the freed connector
> array. USB and HPD events do not depend on the UCSI notification-enable
> state, so the disable command ucsi_unregister() sends to the EC does not
> close this window.
> 
> Fix this by unregistering the EC notifier before ucsi_unregister() in both
> paths, restoring the teardown ordering from before the error and remove
> paths were reworked. blocking_notifier_chain_unregister() waits for a
> running callback, so once it returns the EC interrupt thread can no longer
> reach the UCSI instance and ucsi_unregister() can tear the connectors down
> undisturbed.
> 
> The callback is also the only external source that requeues the connector
> work, so the cancel_work_sync() calls in ucsi_unregister() can no longer
> race with a new connector-change event.
> 
> This issue was found by an in-house static analysis tool.
> 
> Fixes: 168c3896f32e ("usb: typec: ucsi: yoga-c630: fix error and remove paths")
> Cc: stable@vger.kernel.org
> Co-developed-by: Song Li <songl@zju.edu.cn>
> Signed-off-by: Song Li <songl@zju.edu.cn>
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
> ---
>  drivers/usb/typec/ucsi/ucsi_yoga_c630.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

Re: [PATCH] usb: typec: ucsi: yoga-c630: unregister EC notifier before UCSI teardown

[Heikki Krogerus]

On Wed, Sep 23, 2026 at 07:48:28AM +0000, Fan Wu wrote:
> Both yoga_c630_ucsi_remove() and the probe error path call
> ucsi_unregister() while the EC notifier is still registered.
> ucsi_unregister() frees the connector array, so an EC event (USB, HPD or
> UCSI) arriving before yoga_c630_ec_unregister_notify() makes
> yoga_c630_ucsi_notify() run on freed memory: typec_set_orientation() on the
> freed port and ucsi_connector_change() queuing work in the freed connector
> array. USB and HPD events do not depend on the UCSI notification-enable
> state, so the disable command ucsi_unregister() sends to the EC does not
> close this window.
> 
> Fix this by unregistering the EC notifier before ucsi_unregister() in both
> paths, restoring the teardown ordering from before the error and remove
> paths were reworked. blocking_notifier_chain_unregister() waits for a
> running callback, so once it returns the EC interrupt thread can no longer
> reach the UCSI instance and ucsi_unregister() can tear the connectors down
> undisturbed.
> 
> The callback is also the only external source that requeues the connector
> work, so the cancel_work_sync() calls in ucsi_unregister() can no longer
> race with a new connector-change event.
> 
> This issue was found by an in-house static analysis tool.
> 
> Fixes: 168c3896f32e ("usb: typec: ucsi: yoga-c630: fix error and remove paths")
> Cc: stable@vger.kernel.org
> Co-developed-by: Song Li <songl@zju.edu.cn>
> Signed-off-by: Song Li <songl@zju.edu.cn>
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>

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

> ---
>  drivers/usb/typec/ucsi/ucsi_yoga_c630.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
> index 1be18d1..59738dc 100644
> --- a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
> +++ b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
> @@ -296,7 +296,9 @@ static int yoga_c630_ucsi_probe(struct auxiliary_device *adev,
>  	return 0;
>  
>  err_ucsi_unregister:
> +	yoga_c630_ec_unregister_notify(uec->ec, &uec->nb);
>  	ucsi_unregister(uec->ucsi);
> +	goto err_destroy;
>  
>  err_unregister:
>  	yoga_c630_ec_unregister_notify(uec->ec, &uec->nb);
> @@ -311,8 +313,8 @@ static void yoga_c630_ucsi_remove(struct auxiliary_device *adev)
>  {
>  	struct yoga_c630_ucsi *uec = auxiliary_get_drvdata(adev);
>  
> -	ucsi_unregister(uec->ucsi);
>  	yoga_c630_ec_unregister_notify(uec->ec, &uec->nb);
> +	ucsi_unregister(uec->ucsi);
>  	ucsi_destroy(uec->ucsi);
>  }
>  

-- 
heikki