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

[AngeloGioacchino Del Regno]

On 6/8/26 12:54, sashiko-bot at kernel.org wrote:
> 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 at 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?
> 

Yes - and that's ok, because this device cannot exist outside of SPMI and will
never be registered on its own without a SPMI parent.

Overcautious coding bloats drivers for no reason.

> 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);
>