Re: [PATCH] pinctrl: cs42l43: Add GPIO controlled mic/spk mute LEDs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] pinctrl: cirrus: cs42l43: unbalanced fwnode put in
  cs42l43_pin_probe() [1]
--

commit 1aa7ff7b2b9b125f7c2e43385c49822080f93214
Author: Maciej Strozek <mstrozek@opensource.cirrus.com>

pinctrl: cs42l43: Add GPIO controlled mic/spk mute LEDs

This commit adds support for driving speaker and microphone mute LEDs via
cs42l43 GPIOs when described by ACPI properties.

> diff --git a/drivers/pinctrl/cirrus/pinctrl-cs42l43.c b/drivers/pinctrl/cirrus/pinctrl-cs42l43.c
> index a1c839b355578..44a9a759e0f65 100644
> --- a/drivers/pinctrl/cirrus/pinctrl-cs42l43.c
> +++ b/drivers/pinctrl/cirrus/pinctrl-cs42l43.c

[ ... ]

> @@ -598,6 +748,12 @@ static int cs42l43_pin_probe(struct platform_device *pdev)
>  		return dev_err_probe(priv->dev, ret,
>  				     "Failed to register gpiochip\n");
>  
> +#if IS_REACHABLE(CONFIG_LEDS_CLASS)
> +	ret = cs42l43_mute_leds_register(priv, dev_fwnode(cs42l43->dev));
> +	if (ret)
> +		return ret;
> +#endif
> +
>  	return 0;
>  }

[Severity: High]
This problem wasn't introduced by this patch, but does assigning fwnode via
device_set_node() in cs42l43_pin_probe() cause an unbalanced
fwnode_handle_put() [1] when the platform device is released?

Earlier in cs42l43_pin_probe(), fwnode is initialized from the parent MFD
device or looked up from the "pinctrl" child node, and then assigned to
priv->dev:

drivers/pinctrl/cirrus/pinctrl-cs42l43.c:cs42l43_pin_probe() {
	...
	if (is_of_node(fwnode)) {
		struct fwnode_handle *child;

		child = fwnode_get_named_child_node(fwnode, "pinctrl");
		if (child) {
			ret = devm_add_action_or_reset(&pdev->dev,
				cs42l43_fwnode_put, child);
			if (ret)
				return ret;

			if (!child->dev)
				child->dev = priv->dev;
			fwnode = child;
		}
	}

	priv->gpio_chip.fwnode = fwnode;

	device_set_node(priv->dev, fwnode);
	...
}

Unlike device_add_software_node(), device_set_node() does not acquire a
reference on fwnode, whereas platform_device_release() unconditionally drops a
reference on non-software nodes when the platform device is freed:

drivers/base/platform.c:platform_device_release() {
	...
	if (!is_software_node(dev_fwnode(dev)))
		fwnode_handle_put(pa->pdev.dev.fwnode);
	...
}

When the "pinctrl" child node is present, its single reference from
fwnode_get_named_child_node() is already released by cs42l43_fwnode_put() via
devres; otherwise fwnode belongs to cs42l43->dev with no extra reference
taken.  When the platform device is later unregistered,
platform_device_release() puts pdev->dev.fwnode an extra time, underflowing
the fwnode reference count and risking a use-after-free.

[1] https://sashiko.dev/#/bug/linux-a633bdf8-b5fe-4770-b14c-d3532525ae12

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110142058.354518-1-mstrozek@opensource.cirrus.com?part=1