Re: [PATCH v8 1/6] phy: fsl-imx8mq-usb: fix typec switch leak on probe error path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Pre-existing race condition where `imx_phy->tca` is dereferenced by the Type-C switch's `.set()` callback before it is actually assigned in the probe path.
- [High] Lifetime mismatch between the devres-allocated provider data and the registered typec switch callback, leading to Use-After-Free.
--

--- Patch [1]: [PATCH v8 1/6] phy: fsl-imx8mq-usb: fix typec switch leak on probe error path ---
commit c464d7a13f05d9153fb1007a61d8f4290d1a196f
Author: Felix Gu <ustc.gu@gmail.com>

phy: fsl-imx8mq-usb: fix typec switch leak on probe error path

This commit fixes a typec switch leak when probe fails after
imx95_usb_phy_get_tca() succeeds. It uses devm_add_action_or_reset()
so the switch is cleaned up on both probe failure and driver removal.

> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index b05d80e849a14..9a33c06d6fc39 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c

[ ... ]

> @@ -266,17 +267,11 @@ static struct tca_blk *imx95_usb_phy_get_tca(struct platform_device *pdev,
>  	tca->orientation = TYPEC_ORIENTATION_NORMAL;
>  	tca->sw = tca_blk_get_typec_switch(pdev, imx_phy);

[Severity: High]
This is a pre-existing issue, but does registering the switch here expose it 
to consumers before we are fully initialized?

By registering the switch in tca_blk_get_typec_switch(), the device becomes
immediately discoverable. If a consumer acquires it and invokes the callback:

tca_blk_typec_switch_set()
   struct tca_blk *tca = imx_phy->tca;
   ...
   if (tca->orientation == orientation)

Since imx_phy->tca is not assigned until imx8mq_usb_phy_probe() finishes
handling the return from imx95_usb_phy_get_tca(), won't this cause a NULL
pointer dereference if the callback is invoked during this window?

>  
> -	return tca;
> -}
> -
> -static void imx95_usb_phy_put_tca(struct imx8mq_usb_phy *imx_phy)
> -{
> -	struct tca_blk *tca = imx_phy->tca;
> -
> -	if (!tca)
> -		return;
> +	ret = devm_add_action_or_reset(&pdev->dev, tca_blk_put_typec_switch, tca->sw);

[Severity: High]
This is also a pre-existing issue, but does using devres to unregister the 
switch introduce a potential use-after-free?

If a consumer driver (like TCPM) obtains a reference to the switch, and the
PHY driver is subsequently unbound or probe fails:

1. devres calls tca_blk_put_typec_switch() to unregister the switch.

2. typec_switch_unregister() removes the device from sysfs and drops the 
   provider's reference, but the consumer's reference keeps the sw_dev 
   alive.

3. devres then frees the provider data (imx_phy) which was allocated with 
   devm_kzalloc().

If the consumer later calls typec_switch_set(), the callback will access
imx_phy->tca, which has already been freed. Could this lead to memory
corruption?

> +	if (ret)
> +		return ERR_PTR(ret);
>  
> -	tca_blk_put_typec_switch(tca->sw);
> +	return tca;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-imx8mp-usb-phy-improvement-v8-0-2ec8d6b3854d@nxp.com?part=1

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy