Re: [PATCH v9 05/10] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The driver blindly casts `dev->parent` to a `struct spmi_device *` without validating the parent device type, leading to a wild pointer dereference on malformed device trees.
--

commit b6b479ab20e3f2d7a7247657ca4bb0a5a70fc75a
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add()

This commit migrates the SDAM driver to use devm_spmi_subdevice_alloc_and_add().
It registers a new SPMI sub-device for SDAM and initializes a regmap with its
specific base address retrieved from the devicetree.

> diff --git a/drivers/nvmem/qcom-spmi-sdam.c b/drivers/nvmem/qcom-spmi-sdam.c
> index 4f1cca6eab71e..4974105dd9632 100644
> --- a/drivers/nvmem/qcom-spmi-sdam.c
> +++ b/drivers/nvmem/qcom-spmi-sdam.c
[ ... ]
> @@ -110,8 +110,17 @@ static int sdam_write(void *priv, unsigned int offset, void *val,
>  
>  static int sdam_probe(struct platform_device *pdev)
>  {
[ ... ]
>  	struct sdam_chip *sdam;
>  	struct nvmem_device *nvmem;
> +	struct spmi_device *sparent;
> +	struct spmi_subdevice *sub_sdev;
> +	struct device *dev = &pdev->dev;
>  	unsigned int val;
>  	int rc;
>  
> @@ -119,19 +128,23 @@ static int sdam_probe(struct platform_device *pdev)
>  	if (!sdam)
>  		return -ENOMEM;
>  
> -	sdam->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> -	if (!sdam->regmap) {
> -		dev_err(&pdev->dev, "Failed to get regmap handle\n");
> -		return -ENXIO;
> -	}
> +	sparent = to_spmi_device(dev->parent);
> +	sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);

[Severity: High]
Does this code assume that the parent device is always a valid SPMI device?

If the device tree is malformed and this node is placed under a simple-bus
instead of an SPMI PMIC, the parent device will not be an spmi_device.
Because to_spmi_device() uses container_of(), this will result in a bogus
pointer.

When that bogus pointer is passed into devm_spmi_subdevice_alloc_and_add(),
does it cause a wild pointer dereference when accessing sparent->ctrl or
sparent->usid?

Prior to this patch, dev_get_regmap(pdev->dev.parent, NULL) would gracefully
return NULL for malformed hierarchies, but this change appears to introduce
a potential kernel panic.

> +	if (IS_ERR(sub_sdev))
> +		return PTR_ERR(sub_sdev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608100949.36309-1-angelogioacchino.delregno@collabora.com?part=5

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