Re: [PATCH v2 2/4] mtd: rawnand: qcom: Make "aon" clock optional

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free when boot partition parsing fails
- [High] Unbalanced teardown leads to Use-After-Free when a subsequent child node fails probe
--

--- Patch [2]: [PATCH v2 2/4] mtd: rawnand: qcom: Make "aon" clock optional ---
commit 7ddf091d1f9f9e091fb62bfa7346955975fe9533
Author: Stephan Gerhold <stephan.gerhold@linaro.org>

mtd: rawnand: qcom: Make "aon" clock optional
    
This commit makes the second "aon" clock optional to avoid an error when
it is missing on certain SoCs like MDM9607 and SDX55.

> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index 4b80ce084d9ac..0251dd591d40a 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -2280,7 +2280,7 @@ static int qcom_nandc_probe(struct platform_device *pdev)
>  	if (IS_ERR(nandc->core_clk))
>  		return PTR_ERR(nandc->core_clk);
>  
> -	nandc->aon_clk = devm_clk_get(dev, "aon");
> +	nandc->aon_clk = devm_clk_get_optional(dev, "aon");
>  	if (IS_ERR(nandc->aon_clk))
>  		return PTR_ERR(nandc->aon_clk);
>  

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the probe
path I noticed a potential use-after-free when boot partition parsing fails.

In qcom_nand_host_init_and_register(), if mtd_device_parse_register()
succeeds but the subsequent qcom_nand_host_parse_boot_partitions() fails:

qcom_nand_host_init_and_register()
   mtd_device_parse_register()
   qcom_nand_host_parse_boot_partitions()
   goto err;

err:
   nand_cleanup(chip);
   return ret;

Since mtd_device_unregister() is not called in the error path, and the caller
qcom_probe_nand_devices() frees the host memory via devm_kfree(), does this
leave stale pointers in the MTD subsystem?

[Severity: High]
This is also a pre-existing issue, but there appears to be an unbalanced
teardown leading to a use-after-free if a subsequent child node fails
during probe.

In qcom_probe_nand_devices(), the driver iterates through child nodes:

qcom_probe_nand_devices() {
    for_each_available_child_of_node(...) {
        ...
        ret = qcom_nand_host_init_and_register();
        if (ret)
            continue;
    }
    return ret;
}

If an earlier child is successfully initialized and registered, but a later
child fails, the function eventually returns a non-zero error code. This
causes the main qcom_nandc_probe() to fail, and devres automatically frees
the memory for all children.

Since there is no error handling path to unregister previously successful
children, could this result in the MTD subsystem retaining pointers to the
implicitly freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-qcom-nandc-mdm9607-v2-0-d906f7e8b814@linaro.org?part=2