Re: [PATCH v4 5/6] phy: qcom: qmp-pcie: Refactor common multiphy handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] devres: UAF of parent-allocated qmp in child's devres action
--

commit 959cd8885f7d94827203cfb4fa89bce224f75fb7
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

phy: qcom: qmp-pcie: Refactor common multiphy handling

Refactors the multiphy handling to reuse port resource arrays and attach
power domains to individual PHY devices instead of the parent device. It
also validates the clock-output-names property count.

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie-multiphy.c
[ ... ]
> @@ -398,45 +386,39 @@ static const struct phy_ops qmp_pcie_phy_ops = {
>  static void qmp_pcie_pd_detach(void *data)
>  {
>  	struct qmp_pcie *qmp = data;
> -	const struct qmp_phy_cfg *cfg = qmp->cfg;
>  	int i;
>  
> -	for (i = 0; i < cfg->num_pds; i++) {
> +	for (i = 0; i < qmp->cfg->num_pds; i++) {
>  		if (!IS_ERR_OR_NULL(qmp->pd_devs[i]))
>  			dev_pm_domain_detach(qmp->pd_devs[i], true);
>  	}
>  }
>  
> -static int qmp_pcie_pd_init(struct qmp_pcie *qmp)
> +static int qmp_pcie_pd_init(struct qmp_pcie *qmp, struct phy *phy)
>  {
>  	const struct qmp_phy_cfg *cfg = qmp->cfg;
> -	struct device *dev = qmp->dev;
> +	struct device *pd_dev = &phy->dev;
>  	int i, ret;
>  
>  	if (!cfg->num_pds)
>  		return 0;
>  
> -	qmp->pd_devs = devm_kcalloc(dev, cfg->num_pds, sizeof(*qmp->pd_devs),
> +	qmp->pd_devs = devm_kcalloc(pd_dev, cfg->num_pds, sizeof(*qmp->pd_devs),
>  				    GFP_KERNEL);
>  	if (!qmp->pd_devs)
>  		return -ENOMEM;
>  
> +	ret = devm_add_action_or_reset(pd_dev, qmp_pcie_pd_detach, qmp);

[Severity: High]
Can this code introduce a use-after-free regression during device unbind?

The qmp structure is allocated via devm_kcalloc() and tied to the parent
platform device's devres (qmp->dev) in qmp_pcie_multiphy_probe().
However, the cleanup action qmp_pcie_pd_detach() is now added to the child
PHY device's devres (pd_dev = &phy->dev).

If the parent device is unbound while a consumer holds a reference to the
PHY (e.g., via phy_get()), the parent's devres executes first and frees
the qmp array.

When the consumer later releases the PHY, the child PHY device's devres
executes, calling qmp_pcie_pd_detach() on the already freed qmp pointer.
This would lead to a use-after-free when accessing qmp->cfg->num_pds.

> +	if (ret)
> +		return ret;
> +
>  	for (i = 0; i < cfg->num_pds; i++) {
> -		qmp->pd_devs[i] = dev_pm_domain_attach_by_name(dev,
> -							       cfg->pd_names[i]);
> -		if (IS_ERR_OR_NULL(qmp->pd_devs[i])) {
> -			ret = PTR_ERR(qmp->pd_devs[i]) ? : -ENODATA;
> -			goto err_detach;
> -		}
> +		qmp->pd_devs[i] = dev_pm_domain_attach_by_name(pd_dev, cfg->pd_names[i]);
> +		if (IS_ERR_OR_NULL(qmp->pd_devs[i]))
> +			return PTR_ERR_OR_ZERO(qmp->pd_devs[i]) ? : -ENODATA;
>  	}
>  
> -	return devm_add_action_or_reset(dev, qmp_pcie_pd_detach, qmp);
> -
> -err_detach:
> -	while (--i >= 0)
> -		dev_pm_domain_detach(qmp->pd_devs[i], false);
> -
> -	return ret;
> +	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-nord-v4-0-ed05c6e09240@oss.qualcomm.com?part=5

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