Re: [PATCH v11 01/12] spmi: Fix potential use-after-free by grabbing of_node reference

[sashiko-bot@kernel.org]

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

--- Patch [1]: [PATCH v11 01/12] spmi: Fix potential use-after-free by grabbing of_node reference ---
commit e75c47aa8c1f2f720d2fbd1e5f16a0d288d88bdb
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

spmi: Fix potential use-after-free by grabbing of_node reference

This commit increases the of_node refcount when assigning a node to a
newly created SPMI child device in of_spmi_register_devices(). It also
releases the reference in spmi_dev_release() to avoid a use-after-free
when an overlay is dynamically unloaded.

Fixes: bc32bbd04011 ("spmi: Set fwnode for spmi devices")
Cc: stable@vger.kernel.org
Reported-by: Sashiko Bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260608100949.36309-1-angelogioacchino.delregno@collabora.com?part=2

> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
> index 57b7c0cb42407..293e1adddcd71 100644
> --- a/drivers/spmi/spmi.c
> +++ b/drivers/spmi/spmi.c
> @@ -24,6 +24,9 @@ static void spmi_dev_release(struct device *dev)
>  {
>  	struct spmi_device *sdev = to_spmi_device(dev);
>  
> +	if (IS_ENABLED(CONFIG_OF))
> +		of_node_put(dev->of_node);
> +
>  	kfree(sdev);
>  }
>  

[ ... ]

> @@ -517,13 +520,14 @@ static void of_spmi_register_devices(struct spmi_controller *ctrl)
>  		if (!sdev)
>  			continue;
>  
> -		device_set_node(&sdev->dev, of_fwnode_handle(node));
> +		device_set_node(&sdev->dev, of_fwnode_handle(of_node_get(node)));
>  		sdev->usid = (u8)reg[0];
>  
>  		err = spmi_device_add(sdev);
>  		if (err) {
>  			dev_err(&sdev->dev,
>  				"failure adding device. status %d\n", err);
>  			of_node_put(node);
>  			spmi_device_put(sdev);

[Severity: High]
Does this error path trigger a double-free of the device node reference?

Since the patch modifies spmi_dev_release() to unconditionally drop the
node reference during device destruction, spmi_device_put(sdev) will now
call of_node_put() as well.

With the explicit of_node_put(node) remaining right before 
spmi_device_put(sdev), it appears the reference will be dropped twice when
spmi_device_add() fails:

drivers/spmi/spmi.c:of_spmi_register_devices() {
    ...
		if (err) {
			dev_err(&sdev->dev,
				"failure adding device. status %d\n", err);
			of_node_put(node);          <-- First drop
			spmi_device_put(sdev);      <-- Second drop (via spmi_dev_release)
		}
}

Could this lead to an underflow when the loop iterator macro automatically
drops the reference a third time at the end of the iteration?

>  		}
>  	}

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

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