Re: [PATCH v11 04/12] spmi: Remove unneeded goto in spmi_device_add() error path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Double free / reference count underflow of `device_node` in the error path of `spmi_device_add()`.
--

--- Patch [4]: [PATCH v11 04/12] spmi: Remove unneeded goto in spmi_device_add() error path ---
commit 541a42516f5700529a8e8c89c7f975174abfce66
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

spmi: Remove unneeded goto in spmi_device_add() error path

This commit cleans up the error handling path in spmi_device_add() by removing
an unnecessary goto statement. It also updates the function to explicitly
return 0 on success to improve readability.

> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
> index 2cf5fd5e1ba90..14ec9de58467a 100644
> --- a/drivers/spmi/spmi.c
> +++ b/drivers/spmi/spmi.c
> @@ -72,13 +72,11 @@ int spmi_device_add(struct spmi_device *sdev)
>  	err = device_add(&sdev->dev);
>  	if (err < 0) {
>  		dev_err(&sdev->dev, "Can't add device, status %pe\n", ERR_PTR(err));
> -		goto err_device_add;
> +		return err;

[Severity: High]
Does returning an error here cause a reference count underflow of the
device_node in the caller?

When this function fails, it returns back to of_spmi_register_devices()
which handles the error:

drivers/spmi/spmi.c:of_spmi_register_devices() {
	...
	for_each_available_child_of_node(ctrl->dev.of_node, node) {
		...
		device_set_node(&sdev->dev, of_fwnode_handle(of_node_get(node)));
		...
		err = spmi_device_add(sdev);
		if (err) {
			dev_err(&sdev->dev, ...);
			of_node_put(node);
			spmi_device_put(sdev);
		}
	}
}

The loop iterator for_each_available_child_of_node() holds one reference to
node.

Inside the loop, of_node_get() takes a second reference before
device_set_node() stores it.

When spmi_device_add() fails, the error path explicitly drops one reference
with of_node_put(node) and then calls spmi_device_put(sdev).

spmi_device_put(sdev) eventually calls spmi_dev_release(), which calls
of_node_put(dev->of_node), dropping the second reference.

When the iterator advances, it automatically drops a third reference. Since
only two references were taken, does this lead to a use-after-free of the
device_node struct?

>  	}
>  
>  	dev_dbg(&sdev->dev, "device registered\n");
> -
> -err_device_add:
> -	return err;
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(spmi_device_add);

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

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