Re: [PATCH v6 6/8] mmc: sdhci-cadence: refactor driver structure for V6 controller support

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Time-of-check to time-of-use (TOCTOU) buffer overflow during double-pass PHY parameter parsing.
--

commit 63b7c6883f34223180b95652ed2ee59e64bc1fac
Author: Tanmay Kathpalia <tanmay.kathpalia@altera.com>

mmc: sdhci-cadence: refactor driver structure for V6 controller support

This commit refactors the sdhci-cadence driver in preparation for V6
controller support. It separates PHY parameter handling into a dedicated
sdhci_cdns4_phy structure and introduces a new probe function. Explicit
driver data is now required for compatible entries.

> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c

[ ... ]

> @@ -539,6 +544,24 @@ static void sdhci_cdns_mmc_hw_reset(struct mmc_host *mmc)
>  	usleep_range(300, 1000);
>  }
>  
> +static int sdhci_cdns4_phy_probe(struct platform_device *pdev, struct sdhci_cdns_priv *priv)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct sdhci_cdns4_phy *phy;
> +	unsigned int nr_phy_params;
> +
> +	nr_phy_params = sdhci_cdns4_phy_param_count(dev->of_node);
> +	phy = devm_kzalloc(dev, struct_size(phy, phy_params, nr_phy_params), GFP_KERNEL);
> +	if (!phy)
> +		return -ENOMEM;
> +
> +	phy->nr_phy_params = nr_phy_params;
> +	sdhci_cdns4_phy_param_parse(dev->of_node, phy);

[Severity: Critical]
This is a pre-existing issue, but does this allocation and double-pass
parsing sequence open a time-of-check to time-of-use window?

If a device tree overlay is applied concurrently during driver probe
(between the count and the parse calls), sdhci_cdns4_phy_param_parse()
could find more properties than were initially allocated.

Looking at sdhci_cdns4_phy_param_parse(), the pointer p is incremented
without bounds checking against the allocated size:

static void sdhci_cdns4_phy_param_parse(...)
{
        ...
        ret = of_property_read_u32(np, sdhci_cdns4_phy_cfgs[i].property, &val);
        if (!ret) {
                p->addr = sdhci_cdns4_phy_cfgs[i].addr;
                p->data = val;
                p++;
        }
        ...
}

Could this lead to a heap buffer overflow overwriting adjacent memory if
the property list grows between sdhci_cdns4_phy_param_count() and
sdhci_cdns4_phy_param_parse()?

> +	priv->phy = phy;
> +
> +	return sdhci_cdns4_phy_init(priv);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260829184252.27644-1-tanmay.kathpalia@altera.com?part=6