Re: [PATCH v9 08/10] misc: qcom-coincell: 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] 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@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.

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

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

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