Re: [PATCH v3 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 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A concurrent consumer can invoke the Type-C switch `.set` callback before `imx_phy->tca` is fully initialized, causing a NULL pointer dereference.
--

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

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

This patch fixes a typec switch leak on the probe error path by using
devm_add_action_or_reset. This allows cleaning up the switch on both
probe failure and driver removal, eliminating the need for a separate
remove callback.

> 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
[ ... ]
> @@ -248,6 +248,7 @@ static struct tca_blk *imx95_usb_phy_get_tca(struct platform_device *pdev,
>  	struct device *dev = &pdev->dev;
>  	struct resource *res;
>  	struct tca_blk *tca;
> +	int ret;
>  
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>  	if (!res)
> @@ -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 is it possible for a concurrent consumer
to invoke the Type-C switch .set callback before imx_phy->tca is fully
initialized?

In tca_blk_get_typec_switch(), we call typec_switch_register() which makes
the switch visible to the system. Once registered, a consumer could
immediately resolve it and invoke its .set callback, which is
tca_blk_typec_switch_set().

If that happens, tca_blk_typec_switch_set() does:

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

However, imx_phy->tca is assigned using the return value of this function
later in imx8mq_usb_phy_probe():

imx8mq_usb_phy_probe() {
    ...
    imx_phy->tca = imx95_usb_phy_get_tca(pdev, imx_phy);
    ...
}

Could this race condition result in a NULL pointer dereference if the
callback fires before probe finishes assigning imx_phy->tca?

> -	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);
> +	if (ret)
> +		return ERR_PTR(ret);
>  
> -	tca_blk_put_typec_switch(tca->sw);
> +	return tca;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-imx8mp-usb-phy-improvement-v3-0-7afb8f89abc6@nxp.com?part=1

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