All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
Cc: olteanv@gmail.com, vkoul@kernel.org, neil.armstrong@linaro.org,
	linux-phy@lists.infradead.org
Subject: Re: [PATCH v10 04/11] spmi: Remove unneeded goto in spmi_device_add() error path
Date: Tue, 07 Jul 2026 08:48:23 +0000	[thread overview]
Message-ID: <20260707084824.6B80C1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260707083730.33977-5-angelogioacchino.delregno@collabora.com>

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@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

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

  reply	other threads:[~2026-07-07  8:48 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-07  8:37 [PATCH v10 00/11] SPMI: Implement sub-devices and migrate drivers AngeloGioacchino Del Regno
2026-07-07  8:37 ` AngeloGioacchino Del Regno
2026-07-07  8:37 ` [PATCH v10 01/11] spmi: Fix potential use-after-free by grabbing of_node reference AngeloGioacchino Del Regno
2026-07-07  8:37   ` AngeloGioacchino Del Regno
2026-07-07  8:53   ` sashiko-bot
2026-07-07  9:07   ` Andy Shevchenko
2026-07-07  9:07     ` Andy Shevchenko
2026-07-07  8:37 ` [PATCH v10 02/11] spmi: Remove redundant dev_name() print in spmi_device_add() AngeloGioacchino Del Regno
2026-07-07  8:37   ` AngeloGioacchino Del Regno
2026-07-07  8:37 ` [PATCH v10 03/11] spmi: Print error status with %pe format AngeloGioacchino Del Regno
2026-07-07  8:37   ` AngeloGioacchino Del Regno
2026-07-07  8:37 ` [PATCH v10 04/11] spmi: Remove unneeded goto in spmi_device_add() error path AngeloGioacchino Del Regno
2026-07-07  8:37   ` AngeloGioacchino Del Regno
2026-07-07  8:48   ` sashiko-bot [this message]
2026-07-07  8:37 ` [PATCH v10 05/11] spmi: Implement spmi_subdevice_alloc_and_add() and devm variant AngeloGioacchino Del Regno
2026-07-07  8:37   ` AngeloGioacchino Del Regno
2026-07-07  8:48   ` sashiko-bot
2026-07-07  8:37 ` [PATCH v10 06/11] nvmem: qcom-spmi-sdam: Migrate to devm_spmi_subdevice_alloc_and_add() AngeloGioacchino Del Regno
2026-07-07  8:37   ` AngeloGioacchino Del Regno
2026-07-07  8:50   ` sashiko-bot
2026-07-07  8:37 ` [PATCH v10 07/11] power: reset: qcom-pon: " AngeloGioacchino Del Regno
2026-07-07  8:37   ` AngeloGioacchino Del Regno
2026-07-07  8:49   ` sashiko-bot
2026-07-07  8:37 ` [PATCH v10 08/11] phy: qualcomm: eusb2-repeater: " AngeloGioacchino Del Regno
2026-07-07  8:37   ` AngeloGioacchino Del Regno
2026-07-07  8:51   ` sashiko-bot
2026-07-07  8:37 ` [PATCH v10 09/11] misc: qcom-coincell: " AngeloGioacchino Del Regno
2026-07-07  8:37   ` AngeloGioacchino Del Regno
2026-07-07  8:56   ` sashiko-bot
2026-07-07  8:37 ` [PATCH v10 10/11] iio: adc: qcom-spmi-iadc: " AngeloGioacchino Del Regno
2026-07-07  8:37   ` AngeloGioacchino Del Regno
2026-07-07  9:03   ` sashiko-bot
2026-07-07  8:37 ` [PATCH v10 11/11] iio: adc: qcom-spmi-iadc: Remove regmap R/W wrapper functions AngeloGioacchino Del Regno
2026-07-07  8:37   ` AngeloGioacchino Del Regno
2026-07-07  8:55   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260707084824.6B80C1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=linux-phy@lists.infradead.org \
    --cc=neil.armstrong@linaro.org \
    --cc=olteanv@gmail.com \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.