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

[sashiko-bot@kernel.org]

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