[PATCH v10 04/11] spmi: Remove unneeded goto in spmi_device_add() error path

[sashiko-bot at kernel.org]

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

Pre-existing issues:
- [High] If `ida_alloc()` fails in `spmi_controller_alloc()`, the error path incorrectly frees ID 0.
--

--- Patch [4]: [PATCH v10 04/11] spmi: Remove unneeded goto in spmi_device_add() error path ---
commit 46c20d4429e15ea3e907f6068ddedd7ed9c940f8
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno at collabora.com>

spmi: Remove unneeded goto in spmi_device_add() error path

This commit simplifies the error handling in spmi_device_add() by removing an
unnecessary goto statement and returning directly on failure. It also explicitly
returns 0 on success to improve code readability.

> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
> index fa3323521f1a2..cae89dd915d4a 100644
> --- a/drivers/spmi/spmi.c
> +++ b/drivers/spmi/spmi.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but does the spmi_controller_alloc() error path
incorrectly free ID 0 if ida_alloc() fails?

Looking at spmi_controller_alloc():

	ctrl = kzalloc_flex(*ctrl, priv, size);
	/* ctrl->nr is implicitly 0 */

	/* ... */

	id = ida_alloc(&ctrl_ida, GFP_KERNEL);
	if (id < 0) {
		dev_err(parent,
			"unable to allocate SPMI controller identifier.\n");
		spmi_controller_put(ctrl);
		return ERR_PTR(id);
	}

If ida_alloc() returns an error code, spmi_controller_put(ctrl) is called
while ctrl->nr is still 0. This drops the reference and triggers the release
function:

drivers/spmi/spmi.c:spmi_ctrl_release() {
	struct spmi_controller *ctrl = to_spmi_controller(dev);

	ida_free(&ctrl_ida, ctrl->nr);
	kfree(ctrl);
}

Could this cause ida_free() to unconditionally free ID 0, potentially
corrupting the global IDA state if another SPMI controller is actively using
that identifier?

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