Re: [PATCH v9 02/10] spmi: Print error status with %pe format

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The OF device node assigned to the newly allocated SPMI device does not have its reference count incremented, creating a potential use-after-free condition when the iterator macro automatically drops the reference.
--

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

spmi: Print error status with %pe format

This commit updates error messages in the SPMI driver to use the %pe format
specifier for printing error statuses instead of printing integer error codes.

> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
> index e93fc2c957590..95db19a0a44ec 100644
> --- a/drivers/spmi/spmi.c
> +++ b/drivers/spmi/spmi.c
[ ... ]
> @@ -522,7 +522,7 @@ static void of_spmi_register_devices(struct spmi_controller *ctrl)
>  		err = spmi_device_add(sdev);
>  		if (err) {
>  			dev_err(&sdev->dev,
> -				"failure adding device. status %d\n", err);
> +				"failure adding device. status %pe\n", ERR_PTR(err));

[Severity: High]
This is a pre-existing issue, but I noticed a potential use-after-free
condition with the OF node reference counting.

In drivers/spmi/spmi.c:of_spmi_register_devices(), the
for_each_available_child_of_node() loop iterates through child nodes,
automatically calling of_node_put() on the previous node when advancing.

Inside the loop, the node is assigned to the SPMI device:

	device_set_node(&sdev->dev, of_fwnode_handle(node));

Does this assignment need to invoke of_node_get() to increment the node's
reference count? Without it, when the loop advances, the node's reference
is dropped, leaving sdev->dev.of_node as a dangling pointer.

If the device tree node is dynamically managed (e.g., via DT overlays),
unloading the overlay will free the node, causing a use-after-free whenever
the active SPMI device later accesses its of_node.

Additionally, drivers/spmi/spmi.c:spmi_dev_release() lacks an of_node_put()
to balance the required reference:

static void spmi_dev_release(struct device *dev)
{
	struct spmi_device *sdev = to_spmi_device(dev);

	kfree(sdev);
}

Should spmi_dev_release() be updated to safely release the reference once
it is correctly obtained?

>  			spmi_device_put(sdev);
>  		}
>  	}

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

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