[PATCH] phy: sunplus: fix error handling in sp_uphy_init()

[PATCH] phy: sunplus: fix error handling in sp_uphy_init()

[Felix Gu]

Fix the error paths of sp_uphy_init() to undo exactly what each stage
did: return directly if clk_prepare_enable() fails, release only the clock
if reset_control_deassert() fails, and jump to err_reset if
update_disc_vol() fails so the clock and reset are not leaked.

Fixes: 99d9ccd97385 ("phy: usb: Add USB2.0 phy driver for Sunplus SP7021")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/phy/sunplus/phy-sunplus-usb2.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/sunplus/phy-sunplus-usb2.c b/drivers/phy/sunplus/phy-sunplus-usb2.c
index 637a5fbae6d9..0ad4c7160d17 100644
--- a/drivers/phy/sunplus/phy-sunplus-usb2.c
+++ b/drivers/phy/sunplus/phy-sunplus-usb2.c
@@ -116,11 +116,11 @@ static int sp_uphy_init(struct phy *phy)
 
 	ret = clk_prepare_enable(usbphy->phy_clk);
 	if (ret)
-		goto err_clk;
+		return ret;
 
 	ret = reset_control_deassert(usbphy->rstc);
 	if (ret)
-		goto err_reset;
+		goto err_clk;
 
 	/* Default value modification */
 	writel(HIGH_MASK_BITS | 0x4002, usbphy->moon4_regs + UPHY_CONTROL0);
@@ -129,7 +129,7 @@ static int sp_uphy_init(struct phy *phy)
 	/* disconnect voltage */
 	ret = update_disc_vol(usbphy);
 	if (ret < 0)
-		return ret;
+		goto err_reset;
 
 	/* board uphy 0 internal register modification for tid certification */
 	val = readl(usbphy->phy_regs + CONFIG9);

---
base-commit: 415606a7be939835db9b0d6b711887586646346d
change-id: 20260803-sunplus-usb3-ba6c829bc063

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

Re: [PATCH] phy: sunplus: fix error handling in sp_uphy_init()

[Philipp Zabel]

On Mo, 2026-08-03 at 23:15 +0800, Felix Gu wrote:
> Fix the error paths of sp_uphy_init() to undo exactly what each stage
> did: return directly if clk_prepare_enable() fails, release only the clock
> if reset_control_deassert() fails, and jump to err_reset if
> update_disc_vol() fails so the clock and reset are not leaked.
> 
> Fixes: 99d9ccd97385 ("phy: usb: Add USB2.0 phy driver for Sunplus SP7021")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

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

Re: [PATCH] phy: sunplus: fix error handling in sp_uphy_init()

[sashiko-bot]

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

Pre-existing issues:
- [High] Kernel panic due to dereferencing an invalid error pointer from nvmem_cell_get.
- [Medium] Memory leak of the buffer returned by nvmem_cell_read.
- [Medium] Unvalidated array access may lead to an out-of-bounds read.
--

--- Patch [1]: [PATCH] phy: sunplus: fix error handling in sp_uphy_init() ---
commit bbdc3fb921cb2657598b63724660355fd956b455
Author: Felix Gu <ustc.gu@gmail.com>

phy: sunplus: fix error handling in sp_uphy_init()

Fixes the error paths in sp_uphy_init() to correctly undo initialization
steps. It returns directly if clock enable fails, releases the clock if
reset deassert fails, and asserts reset and clock if disconnect voltage
update fails.

> diff --git a/drivers/phy/sunplus/phy-sunplus-usb2.c b/drivers/phy/sunplus/phy-sunplus-usb2.c
> index 637a5fbae6d9a..0ad4c7160d17c 100644
> --- a/drivers/phy/sunplus/phy-sunplus-usb2.c
> +++ b/drivers/phy/sunplus/phy-sunplus-usb2.c
> @@ -129,7 +129,7 @@ static int sp_uphy_init(struct phy *phy)
>  	/* disconnect voltage */
>  	ret = update_disc_vol(usbphy);

[Severity: High]
This isn't a bug introduced by this patch, but looking at update_disc_vol()
called here in drivers/phy/sunplus/phy-sunplus-usb2.c, is there a potential
dereference of an invalid pointer?

If nvmem_cell_get() fails and returns an error pointer other than
-EPROBE_DEFER (such as -ENOENT), or if it returns NULL, the code doesn't
return early. It falls through and passes this invalid pointer directly to
nvmem_cell_read():

	cell = nvmem_cell_get(usbphy->dev, disc_name);
	if (IS_ERR_OR_NULL(cell)) {
		if (PTR_ERR(cell) == -EPROBE_DEFER)
			return -EPROBE_DEFER;
	}

	otp_v = nvmem_cell_read(cell, &otp_l);
	nvmem_cell_put(cell);

Will this cause a panic when nvmem_cell_read() attempts to dereference the
cell pointer?

[Severity: Medium]
This is a pre-existing issue, but does update_disc_vol() leak the buffer
returned by nvmem_cell_read()?

	otp_v = nvmem_cell_read(cell, &otp_l);
	nvmem_cell_put(cell);

	if (!IS_ERR(otp_v)) {
		set = *(otp_v + 1);
		...

The nvmem_cell_read() function returns a dynamically allocated buffer that
the caller is responsible for freeing. It appears otp_v is never passed to
kfree() before the function returns, which would result in a memory leak.

[Severity: Medium]
This is another pre-existing issue, but could the array access in
update_disc_vol() lead to an out-of-bounds read?

	otp_v = nvmem_cell_read(cell, &otp_l);
	nvmem_cell_put(cell);

	if (!IS_ERR(otp_v)) {
		set = *(otp_v + 1);

The size of the read buffer is provided in otp_l. If a device tree
incorrectly provides a 1-byte nvmem cell, unconditionally accessing the
second byte of the buffer at *(otp_v + 1) without checking if otp_l is at
least 2 might lead to reading out-of-bounds memory.

>  	if (ret < 0)
> -		return ret;
> +		goto err_reset;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-sunplus-usb3-v1-1-5a562524c869@gmail.com?part=1

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

Re: [PATCH] phy: sunplus: fix error handling in sp_uphy_init()

[Vinod Koul]

On Mon, 03 Aug 2026 23:15:17 +0800, Felix Gu wrote:
> Fix the error paths of sp_uphy_init() to undo exactly what each stage
> did: return directly if clk_prepare_enable() fails, release only the clock
> if reset_control_deassert() fails, and jump to err_reset if
> update_disc_vol() fails so the clock and reset are not leaked.
> 
> 

Applied, thanks!

[1/1] phy: sunplus: fix error handling in sp_uphy_init()
      commit: 8b2683bc4cc18c581b7cd24f227cbe61abca7f4d

Best regards,
-- 
~Vinod



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