[PATCH v9 08/10] misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()

[AngeloGioacchino Del Regno]

On 6/8/26 13:09, sashiko-bot at kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Unsafe downcast of `dev->parent` to `struct spmi_device` causing out-of-bounds read and kernel panic
> --
> 
> commit eb0fe105cd38bff6269804925b90232e1c422522
> Author: AngeloGioacchino Del Regno <angelogioacchino.delregno at collabora.com>
> 
> misc: qcom-coincell: Migrate to devm_spmi_subdevice_alloc_and_add()
> 
> This commit updates the qcom-coincell driver to use a dedicated SPMI
> sub-device instead of fetching the regmap from its parent. It creates the
> sub-device using the parent device and initializes a specific regmap to
> handle the base address offsets automatically.
> 
>> diff --git a/drivers/misc/qcom-coincell.c b/drivers/misc/qcom-coincell.c
>> index 3c57f74291476..b0904f441300c 100644
>> --- a/drivers/misc/qcom-coincell.c
>> +++ b/drivers/misc/qcom-coincell.c
>> @@ -71,37 +71,55 @@ static int qcom_coincell_probe(struct platform_device *pdev)
> [ ... ]
>> -	rc = of_property_read_u32(node, "reg", &chgr.base_addr);
>> +	rc = device_property_read_u32(dev, "reg", &qcom_coincell_regmap_config.reg_base);
>>   	if (rc)
>>   		return rc;
>>   
>> +	sparent = to_spmi_device(dev->parent);
>> +	sub_sdev = devm_spmi_subdevice_alloc_and_add(dev, sparent);
> 
> [Severity: High]
> Could this lead to a panic if the parent device is not actually an SPMI
> device?
> 
> If this driver is bound to a device whose parent is a regular platform
> device (for example, via a malformed device tree or a manual sysfs bind),
> dev->parent will not point to a valid spmi_device.
> 

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, so this is fine.

> Because to_spmi_device() is a container_of cast, it will blindly cast the
> parent device. When passed to devm_spmi_subdevice_alloc_and_add(), the
> underlying spmi_subdevice_alloc_and_add() function reads sparent->ctrl:
> 
> drivers/spmi/spmi.c:spmi_subdevice_alloc_and_add() {
>      ...
>      sdev->ctrl = sparent->ctrl;
>      ...
> }
> 
> Since the parent is actually a platform_device, this ends up reading the
> memory offset corresponding to platform_dma_mask instead of a valid ctrl
> pointer. Later regmap initialization will dereference this invalid pointer,
> causing an out-of-bounds read.
> 
> The previous dev_get_regmap() approach avoided this by safely searching the
> parent's devres list without relying on a structural cast. Is there a way to
> verify the parent device type before casting?
> 
>> +	if (IS_ERR(sub_sdev))
>> +		return PTR_ERR(sub_sdev);
>