[PATCH 0/2] phy: zynqmp: fix clock and runtime PM error paths in xpsgtr driver

[PATCH 0/2] phy: zynqmp: fix clock and runtime PM error paths in xpsgtr driver

[Radhey Shyam Pandey]

This series fixes two pre-existing resource handling bugs in the Zynq
UltraScale+ MPSoC Gigabit Transceiver (XPSGTR) PHY driver. They were
reported during review of the SERDES scrambler series [1] and were
intentionally deferred to this follow-up series.

Patch 1:
Propagate clk_prepare_enable() failures from xpsgtr_phy_init() and 
disable the reference clock on initialization error paths.

Patch 2:
Move saved_regs allocation before runtime PM resume to avoid 
leaking the runtime PM usage count on probe failures.

[1]: https://lore.kernel.org/all/20260627155229.2791113-1-radhey.shyam.pandey@amd.com

Radhey Shyam Pandey (2):
  phy: zynqmp: fix clock error handling in xpsgtr_phy_init()
  phy: zynqmp: fix runtime PM leak on probe allocation failure

 drivers/phy/xilinx/phy-zynqmp.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)


base-commit: 0718283ab28bc3907e10b61a6b4be6fefa1cbb2f
-- 
2.43.0


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

[PATCH 1/2] phy: zynqmp: fix clock error handling in xpsgtr_phy_init()

[Radhey Shyam Pandey]

Propagate clk_prepare_enable() failures to the caller instead of
returning success, and disable the reference clock on initialization
error paths to avoid leaking clock references when phy_exit() is not
called.

Fixes: 25d700833513 ("phy: xilinx: phy-zynqmp: dynamic clock support for power-save")
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
 drivers/phy/xilinx/phy-zynqmp.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
index fe6b4925d166..c8230f2bda62 100644
--- a/drivers/phy/xilinx/phy-zynqmp.c
+++ b/drivers/phy/xilinx/phy-zynqmp.c
@@ -658,12 +658,13 @@ static int xpsgtr_phy_init(struct phy *phy)
 {
 	struct xpsgtr_phy *gtr_phy = phy_get_drvdata(phy);
 	struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
-	int ret = 0;
+	int ret;
 
 	mutex_lock(&gtr_dev->gtr_mutex);
 
 	/* Configure and enable the clock when peripheral phy_init call */
-	if (clk_prepare_enable(gtr_dev->clk[gtr_phy->refclk]))
+	ret = clk_prepare_enable(gtr_dev->clk[gtr_phy->refclk]);
+	if (ret)
 		goto out;
 
 	/* Skip initialization if not required. */
@@ -673,7 +674,7 @@ static int xpsgtr_phy_init(struct phy *phy)
 	if (gtr_dev->tx_term_fix) {
 		ret = xpsgtr_phy_tx_term_fix(gtr_phy);
 		if (ret < 0)
-			goto out;
+			goto out_disable_clk;
 
 		gtr_dev->tx_term_fix = false;
 	}
@@ -687,7 +688,7 @@ static int xpsgtr_phy_init(struct phy *phy)
 	 */
 	ret = xpsgtr_configure_pll(gtr_phy);
 	if (ret)
-		goto out;
+		goto out_disable_clk;
 
 	xpsgtr_lane_set_protocol(gtr_phy);
 
@@ -705,6 +706,10 @@ static int xpsgtr_phy_init(struct phy *phy)
 		break;
 	}
 
+	goto out;
+
+out_disable_clk:
+	clk_disable_unprepare(gtr_dev->clk[gtr_phy->refclk]);
 out:
 	mutex_unlock(&gtr_dev->gtr_mutex);
 	return ret;
-- 
2.43.0


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

[PATCH 2/2] phy: zynqmp: fix runtime PM leak on probe allocation failure

[Radhey Shyam Pandey]

Allocate saved_regs before pm_runtime_resume_and_get() so a
devm_kmalloc() failure does not leave an unreleased runtime PM usage
counter.

Fixes: 5af9b304bc60 ("phy: xilinx: phy-zynqmp: Fix SGMII linkup failure on resume")
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
 drivers/phy/xilinx/phy-zynqmp.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
index c8230f2bda62..2138f5399821 100644
--- a/drivers/phy/xilinx/phy-zynqmp.c
+++ b/drivers/phy/xilinx/phy-zynqmp.c
@@ -1044,6 +1044,12 @@ static int xpsgtr_probe(struct platform_device *pdev)
 		return PTR_ERR(provider);
 	}
 
+	gtr_dev->saved_regs = devm_kmalloc(gtr_dev->dev,
+					   sizeof(save_reg_address),
+					   GFP_KERNEL);
+	if (!gtr_dev->saved_regs)
+		return -ENOMEM;
+
 	pm_runtime_set_active(gtr_dev->dev);
 	pm_runtime_enable(gtr_dev->dev);
 
@@ -1053,12 +1059,6 @@ static int xpsgtr_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	gtr_dev->saved_regs = devm_kmalloc(gtr_dev->dev,
-					   sizeof(save_reg_address),
-					   GFP_KERNEL);
-	if (!gtr_dev->saved_regs)
-		return -ENOMEM;
-
 	return 0;
 }
 
-- 
2.43.0


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

Re: [PATCH 2/2] phy: zynqmp: fix runtime PM leak on probe allocation failure

[sashiko-bot]

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

Pre-existing issues:
- [High] Premature registration of the PHY provider exposes consumers to a Use-After-Free (UAF) if a subsequent probe initialization step fails.
--

--- Patch [2]: [PATCH 2/2] phy: zynqmp: fix runtime PM leak on probe allocation failure ---
commit e07d2a03d56ecad62f1f38421ad1054872a5ffb6
Author: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
    
phy: zynqmp: fix runtime PM leak on probe allocation failure
    
This commit moves the devm_kmalloc() allocation for saved_regs before calling
pm_runtime_resume_and_get(). This ensures that an allocation failure does not
leave an unreleased runtime PM usage counter.
    
> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> index c8230f2bda629..2138f5399821a 100644
> --- a/drivers/phy/xilinx/phy-zynqmp.c
> +++ b/drivers/phy/xilinx/phy-zynqmp.c
> @@ -1044,6 +1044,12 @@ static int xpsgtr_probe(struct platform_device *pdev)
>  		return PTR_ERR(provider);
>  	}
>  
> +	gtr_dev->saved_regs = devm_kmalloc(gtr_dev->dev,
> +					   sizeof(save_reg_address),
> +					   GFP_KERNEL);
> +	if (!gtr_dev->saved_regs)
> +		return -ENOMEM;
> +

[Severity: High]
This is a pre-existing issue, but does failing the probe here expose a
use-after-free race condition?

Just above this block in xpsgtr_probe(), the driver registers the PHY provider
using devm_of_phy_provider_register(). If this new devm_kmalloc() call (or the
subsequent pm_runtime_resume_and_get() call) fails, the probe function will
return an error. Devres will then automatically free the driver data, including
gtr_dev.

If a consumer device is probing concurrently and obtains a reference to the PHY
before this error path executes, could that consumer later attempt to use the
freed gtr_dev memory when calling phy_init()? Would it be safer to move the
devm_of_phy_provider_register() call to the very end of xpsgtr_probe() after
all initialization and allocations have succeeded?

>  	pm_runtime_set_active(gtr_dev->dev);
>  	pm_runtime_enable(gtr_dev->dev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720153832.1130006-1-radhey.shyam.pandey@amd.com?part=2

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