Re: [PATCH v5 1/5] 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] Race condition causing NULL pointer dereference during device probe.
- [High] Use-After-Free of devm-allocated driver data during device unbind.
--

commit d64f34cfc15d26f577e5ff3a17572cf971e65696
Author: Felix Gu <ustc.gu@gmail.com>

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

If probe fails after imx95_usb_phy_get_tca() succeeds, the typec
switch leaks because the only cleanup path was in .remove, which
never runs on probe failure. Use 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
> --- 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 this code introduce a race
condition during probe?

When tca_blk_get_typec_switch() registers the switch, it is immediately
visible to consumers. However, imx_phy->tca has not been assigned yet.
It is only assigned after returning from imx95_usb_phy_get_tca() in
imx8mq_usb_phy_probe().

If a consumer concurrently finds the switch and calls typec_switch_set(),
tca_blk_typec_switch_set() will access imx_phy->tca->orientation,
dereferencing a NULL pointer.

>  
> -	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 a pre-existing issue, but could this devm cleanup lead to a
use-after-free during driver unbind?

The imx_phy and tca structures are allocated via devm_kzalloc(). During
unbind, this devres action will execute tca_blk_put_typec_switch(), which
calls typec_switch_unregister().

While typec_switch_unregister() unlinks the device, if a consumer still
holds a reference to the switch, the object itself will outlive the driver
unbind. Since devres immediately frees imx_phy and tca after the action
returns, any subsequent call to tca_blk_typec_switch_set() by the active
consumer will access freed memory.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-imx8mp-usb-phy-improvement-v5-0-25d616403844@nxp.com?part=1

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