Re: [PATCH v14 1/2] HID: cp2112: Add fwnode support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Premature release of firmware node references results in use-after-free vulnerabilities when assigning the I2C adapter and GPIO chip firmware nodes.
--

commit 3dfaf368e855835bf23009205c4b93a2a070aae1
Author: Danny Kaehn <danny.kaehn@plexus.com>

HID: cp2112: Add fwnode support

This commit adds firmware node support to describe the CP2112's I2C and
GPIO interfaces. It differentiates between ACPI and DeviceTree bindings,
mapping the child nodes to the respective adapter and gpio_chip devices.

> diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
> index 803b883ae8750..e960fc988058f 100644
> --- a/drivers/hid/hid-cp2112.c
> +++ b/drivers/hid/hid-cp2112.c
[ ... ]
> @@ -1226,6 +1241,28 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		return ret;
>  	}
>  
> +	cp2112_fwnode = dev_fwnode(&hdev->dev);
> +	if (is_acpi_device_node(cp2112_fwnode)) {
> +		fwnode_for_each_child_node(cp2112_fwnode, child) {
> +			ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> +			if (ret)
> +				continue;
> +
> +			switch (addr) {
> +			case CP2112_I2C_ADR:
> +				device_set_node(&dev->adap.dev, child);
> +				break;
> +			case CP2112_GPIO_ADR:
> +				dev->gc.fwnode = child;
> +				break;
> +			}

[Severity: High]
Does this code leave dangling pointers in the device structures?

The fwnode_for_each_child_node() macro automatically drops the reference to
the child node when advancing to the next node or exiting the loop.

Because device_set_node() and direct pointer assignments do not increment
the reference count, the nodes assigned here could be prematurely freed
resulting in a use-after-free.

> +		}
> +	} else if (is_of_node(cp2112_fwnode)) {
> +		child = fwnode_get_named_child_node(cp2112_fwnode, "i2c");
> +		device_set_node(&dev->adap.dev, child);
> +		fwnode_handle_put(child);

[Severity: High]
Are we releasing the firmware node prematurely here?

fwnode_get_named_child_node() returns a node with an incremented reference
count. However, since device_set_node() does not take its own reference on
the child node, dropping the reference immediately after could leave
dev->adap.dev with a dangling pointer if the node is later removed.

> +	}
> +
>  	ret = hid_parse(hdev);
>  	if (ret) {
>  		hid_err(hdev, "parse failed\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520-cp2112-dt-v14-0-b1b4b6734b6f@plexus.com?part=1