Re: [PATCH v3 3/4] leds: flash: lm3643: Describe the LED of ACPI boards without LED nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] leds: flash: lm3643: TOCTOU race in swnode registration causes UAF
--

commit a08dd419e737ce9719711220082ac93884d1bc47
Author: Ruslan Koreev <koreev.r@gmail.com>

leds: flash: lm3643: Describe the LED of ACPI boards without LED nodes

This commit adds a DMI table and software node descriptions for Lenovo
ThinkPads that enumerate the LM3643 through ACPI without a _DSD. This
fallback allows the Windows Hello IR flood illuminator to be probed
and registered when the firmware provides no child nodes.

> diff --git a/drivers/leds/flash/leds-lm3643.c b/drivers/leds/flash/leds-lm3643.c
> index cbe66e90d8e1b..18ac75df72e2a 100644
> --- a/drivers/leds/flash/leds-lm3643.c
> +++ b/drivers/leds/flash/leds-lm3643.c

[ ... ]

> +static int lm3643_add_dmi_leds(struct device *dev)
> +{
> +	const struct dmi_system_id *id;
> +	const struct software_node **nodes;
> +	int ret;
> +
> +	if (!has_acpi_companion(dev))
> +		return 0;
> +
> +	id = dmi_first_match(lm3643_dmi_leds);
> +	if (!id)
> +		return 0;
> +
> +	nodes = id->driver_data;
> +	/* The nodes are static: a second chip on the same board cannot reuse them */
> +	if (software_node_fwnode(nodes[0]))
> +		return dev_err_probe(dev, -EBUSY, "LED description already in use\n");
> +
> +	ret = software_node_register_node_group(nodes);

[Severity: High]
Is there a potential race condition here when multiple devices probe
concurrently?

If two threads execute lm3643_add_dmi_leds() simultaneously (for example,
during concurrent sysfs bind or async probe of two LM3643 devices matching
the same DMI table), they both might pass the lockless 
software_node_fwnode(nodes[0]) check before either registers the nodes.

Thread A could then successfully register the group via
software_node_register_node_group(nodes).

Thread B would subsequently attempt to register the same static nodes array,
fail, and drop into the error path within software_node_register_node_group():

drivers/base/swnode.c:software_node_register_node_group() {
    ...
		ret = software_node_register(node_group[i]);
		if (ret) {
			software_node_unregister_node_group(node_group);
			return ret;
		}
    ...
}

Because software_node_unregister_node_group() unconditionally unregisters
the entire array, Thread B's failure teardown will free the nodes that
Thread A successfully registered. 

Could this lead to a use-after-free when Thread A subsequently calls
device_get_child_node_count() and accesses the freed software nodes?

> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to register the LED description\n");
> +
> +	set_secondary_fwnode(dev, software_node_fwnode(nodes[0]));
> +	dev_dbg(dev, "no LED nodes in firmware, using the DMI description\n");
> +
> +	return devm_add_action_or_reset(dev, lm3643_remove_dmi_leds, dev);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011191026.149338-1-rillian.grant@gmail.com?part=3