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

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 b=
y the Type-C switch's `.set()` callback before it is actually assigned in t=
he probe path.
- [High] Lifetime mismatch between the devres-allocated provider data and t=
he 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 <[email protected]>

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/fre=
escale/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 =3D TYPEC_ORIENTATION_NORMAL;
>  	tca->sw =3D tca_blk_get_typec_switch(pdev, imx_phy);

[Severity: High]
This is a pre-existing issue, but does registering the switch here expose i=
t=20
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 callbac=
k:

tca_blk_typec_switch_set()
   struct tca_blk *tca =3D imx_phy->tca;
   ...
   if (tca->orientation =3D=3D 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?

> =20
> -	return tca;
> -}
> -
> -static void imx95_usb_phy_put_tca(struct imx8mq_usb_phy *imx_phy)
> -{
> -	struct tca_blk *tca =3D imx_phy->tca;
> -
> -	if (!tca)
> -		return;
> +	ret =3D 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=
=20
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=20
   provider's reference, but the consumer's reference keeps the sw_dev=20
   alive.

3. devres then frees the provider data (imx_phy) which was allocated with=20
   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);
> =20
> -	tca_blk_put_typec_switch(tca->sw);
> +	return tca;
>  }

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260731-imx8mp-usb=
[email protected]?part=3D1