* Re: [PATCH] phy: fsl-imx8mq-usb: fix typec switch leak on probe error path
2026-06-02 14:12 [PATCH] phy: fsl-imx8mq-usb: fix typec switch leak on probe error path Felix Gu
@ 2026-06-02 14:24 ` sashiko-bot
2026-06-02 20:28 ` Frank Li
2026-06-03 3:09 ` Xu Yang
2 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-06-02 14:24 UTC (permalink / raw)
To: Felix Gu; +Cc: neil.armstrong, linux-phy, vkoul, Frank.Li, olteanv, imx
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Potential NULL pointer dereference if the Type-C switch's `.set()` callback is invoked concurrently during probe.
- [High] Potential Use-After-Free if a typec port driver holds a reference to the switch and calls `.set()` after the PHY driver unbinds.
--
commit 546b07c54e0077d325cf3fd20b1dd95d1c123a9d
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
> index b05d80e849a14..8af5a4f856987 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> @@ -173,8 +173,10 @@ static struct typec_switch_dev *tca_blk_get_typec_switch(struct platform_device
> return sw;
> }
>
> -static void tca_blk_put_typec_switch(struct typec_switch_dev *sw)
> +static void tca_blk_put_typec_switch(void *data)
> {
> + struct typec_switch_dev *sw = data;
> +
> typec_switch_unregister(sw);
> }
[Severity: High]
This is a pre-existing issue, but could this cleanup sequence lead to a
use-after-free during unbind?
When the PHY driver unbinds or fails probe late, the devres framework will
automatically free the imx_phy and tca structures since they are allocated
with devm_kzalloc. While typec_switch_unregister() drops the initial
reference to the switch, it doesn't force consumers to drop their references.
If a consumer typec port driver still holds a reference to the switch and
calls its .set callback after the PHY driver unbinds:
tca_blk_typec_switch_set()
struct imx8mq_usb_phy *imx_phy = typec_switch_get_drvdata(sw);
struct tca_blk *tca = imx_phy->tca;
won't this callback access the freed imx_phy and tca memory?
[ ... ]
> @@ -266,17 +269,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 isn't a bug introduced by this patch, but does registering the switch
here expose it to consumers before imx_phy->tca is assigned?
Once registered, the switch becomes globally discoverable via fwnode matching.
If a consumer discovers it and immediately invokes the .set callback
(tca_blk_typec_switch_set), that callback expects imx_phy->tca to be
fully initialized:
tca_blk_typec_switch_set()
struct tca_blk *tca = imx_phy->tca;
int ret;
if (tca->orientation == orientation)
Since the assignment of imx_phy->tca happens later in the probe sequence
after this function returns, wouldn't this concurrent access dereference 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);
> + if (ret)
> + return ERR_PTR(ret);
>
> - tca_blk_put_typec_switch(tca->sw);
> + return tca;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260602-imx8mq-typec-v1-1-3debe9bc77d9@gmail.com?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] phy: fsl-imx8mq-usb: fix typec switch leak on probe error path
2026-06-02 14:12 [PATCH] phy: fsl-imx8mq-usb: fix typec switch leak on probe error path Felix Gu
2026-06-02 14:24 ` sashiko-bot
@ 2026-06-02 20:28 ` Frank Li
2026-06-03 3:09 ` Xu Yang
2 siblings, 0 replies; 5+ messages in thread
From: Frank Li @ 2026-06-02 20:28 UTC (permalink / raw)
To: Felix Gu
Cc: Vinod Koul, Neil Armstrong, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Xu Yang, Jun Li, linux-phy, imx, linux-arm-kernel,
linux-kernel
On Tue, Jun 02, 2026 at 10:12:41PM +0800, Felix Gu wrote:
> 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. The .remove callback and
> imx95_usb_phy_put_tca() are no longer needed.
>
> Fixes: b58f0f86fd61 ("phy: fsl-imx8mq-usb: add tca function driver for imx95")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.com>
> drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 27 ++++++++-------------------
> 1 file changed, 8 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index b05d80e849a1..8af5a4f85698 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> @@ -173,8 +173,10 @@ static struct typec_switch_dev *tca_blk_get_typec_switch(struct platform_device
> return sw;
> }
>
> -static void tca_blk_put_typec_switch(struct typec_switch_dev *sw)
> +static void tca_blk_put_typec_switch(void *data)
> {
> + struct typec_switch_dev *sw = data;
> +
> typec_switch_unregister(sw);
> }
>
> @@ -248,6 +250,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 +269,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);
>
> - 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;
> }
>
> static u32 phy_tx_vref_tune_from_property(u32 percent)
> @@ -739,16 +736,8 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> return PTR_ERR_OR_ZERO(phy_provider);
> }
>
> -static void imx8mq_usb_phy_remove(struct platform_device *pdev)
> -{
> - struct imx8mq_usb_phy *imx_phy = platform_get_drvdata(pdev);
> -
> - imx95_usb_phy_put_tca(imx_phy);
> -}
> -
> static struct platform_driver imx8mq_usb_phy_driver = {
> .probe = imx8mq_usb_phy_probe,
> - .remove = imx8mq_usb_phy_remove,
> .driver = {
> .name = "imx8mq-usb-phy",
> .of_match_table = imx8mq_usb_phy_of_match,
>
> ---
> base-commit: 08484c504b55a98bd100527fbe10a3caf55ff3ff
> change-id: 20260602-imx8mq-typec-20a6f1d1900f
>
> Best regards,
> --
> Felix Gu <ustc.gu@gmail.com>
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] phy: fsl-imx8mq-usb: fix typec switch leak on probe error path
2026-06-02 14:12 [PATCH] phy: fsl-imx8mq-usb: fix typec switch leak on probe error path Felix Gu
2026-06-02 14:24 ` sashiko-bot
2026-06-02 20:28 ` Frank Li
@ 2026-06-03 3:09 ` Xu Yang
2026-06-03 5:45 ` Xu Yang
2 siblings, 1 reply; 5+ messages in thread
From: Xu Yang @ 2026-06-03 3:09 UTC (permalink / raw)
To: Felix Gu
Cc: Vinod Koul, Neil Armstrong, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Xu Yang, Jun Li,
linux-phy, imx, linux-arm-kernel, linux-kernel
On Tue, Jun 02, 2026 at 10:12:41PM +0800, Felix Gu wrote:
> 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. The .remove callback and
> imx95_usb_phy_put_tca() are no longer needed.
>
> Fixes: b58f0f86fd61 ("phy: fsl-imx8mq-usb: add tca function driver for imx95")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---
> drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 27 ++++++++-------------------
> 1 file changed, 8 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index b05d80e849a1..8af5a4f85698 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> @@ -173,8 +173,10 @@ static struct typec_switch_dev *tca_blk_get_typec_switch(struct platform_device
> return sw;
> }
>
> -static void tca_blk_put_typec_switch(struct typec_switch_dev *sw)
> +static void tca_blk_put_typec_switch(void *data)
> {
> + struct typec_switch_dev *sw = data;
> +
> typec_switch_unregister(sw);
> }
Nit: You can simply call typec_switch_unregister(data)
Reviewed-by: Xu Yang <xu.yang_2@nxp.com>
>
> @@ -248,6 +250,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 +269,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);
>
> - 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;
> }
>
> static u32 phy_tx_vref_tune_from_property(u32 percent)
> @@ -739,16 +736,8 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> return PTR_ERR_OR_ZERO(phy_provider);
> }
>
> -static void imx8mq_usb_phy_remove(struct platform_device *pdev)
> -{
> - struct imx8mq_usb_phy *imx_phy = platform_get_drvdata(pdev);
> -
> - imx95_usb_phy_put_tca(imx_phy);
> -}
> -
> static struct platform_driver imx8mq_usb_phy_driver = {
> .probe = imx8mq_usb_phy_probe,
> - .remove = imx8mq_usb_phy_remove,
> .driver = {
> .name = "imx8mq-usb-phy",
> .of_match_table = imx8mq_usb_phy_of_match,
>
> ---
> base-commit: 08484c504b55a98bd100527fbe10a3caf55ff3ff
> change-id: 20260602-imx8mq-typec-20a6f1d1900f
>
> Best regards,
> --
> Felix Gu <ustc.gu@gmail.com>
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] phy: fsl-imx8mq-usb: fix typec switch leak on probe error path
2026-06-03 3:09 ` Xu Yang
@ 2026-06-03 5:45 ` Xu Yang
0 siblings, 0 replies; 5+ messages in thread
From: Xu Yang @ 2026-06-03 5:45 UTC (permalink / raw)
To: Felix Gu
Cc: Vinod Koul, Neil Armstrong, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Xu Yang, Jun Li,
linux-phy, imx, linux-arm-kernel, linux-kernel
On Wed, Jun 03, 2026 at 11:09:32AM +0800, Xu Yang wrote:
> On Tue, Jun 02, 2026 at 10:12:41PM +0800, Felix Gu wrote:
> > 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. The .remove callback and
> > imx95_usb_phy_put_tca() are no longer needed.
> >
> > Fixes: b58f0f86fd61 ("phy: fsl-imx8mq-usb: add tca function driver for imx95")
> > Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> > ---
> > drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 27 ++++++++-------------------
> > 1 file changed, 8 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > index b05d80e849a1..8af5a4f85698 100644
> > --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > @@ -173,8 +173,10 @@ static struct typec_switch_dev *tca_blk_get_typec_switch(struct platform_device
> > return sw;
> > }
> >
> > -static void tca_blk_put_typec_switch(struct typec_switch_dev *sw)
> > +static void tca_blk_put_typec_switch(void *data)
> > {
> > + struct typec_switch_dev *sw = data;
> > +
> > typec_switch_unregister(sw);
> > }
>
> Nit: You can simply call typec_switch_unregister(data)
>
> Reviewed-by: Xu Yang <xu.yang_2@nxp.com>
I have a similar patch in my patchset. I have added your patch to my patchset
because it has been reviewed and my other patches depend on this one. Otherwise,
rebase is needed for others.
https://lore.kernel.org/linux-phy/20260603-imx8mp-usb-phy-improvement-v3-0-7afb8f89abc6@nxp.com/T/#t
Thanks,
Xu Yang
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 5+ messages in thread