Re: [PATCH v5 2/2] Input: synaptics-rmi4 - reject a PDT that grows between scans

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] input: synaptics-rmi4: multiple resource leaks in rmi_driver_probe error paths
--

commit 81cc67e5a61f0cfdb2ce1738f5778c94caef2b1c
Author: Wei Jie Law <98lawweijie@gmail.com>

    Input: synaptics-rmi4 - reject a PDT that grows between scans

This commit introduces a validation check in rmi_create_function() to prevent
out-of-bounds writes if a device's interrupt count grows between Page
Description Table scans. It fails the probe by returning -EINVAL before
allocating memory for the invalid function.

> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index 443ba41063524..b2ed5991b1970 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -898,6 +898,25 @@ static int rmi_create_function(struct rmi_device *rmi_dev,
>  	rmi_dbg(RMI_DEBUG_CORE, dev, "Initializing F%02X.\n",
>  			pdt->function_number);
>  
> +	/*
> +	 * irq_mask[] was sized from the interrupt count collected by the
> +	 * earlier rmi_count_irqs() scan of the PDT, and irq[] holds
> +	 * RMI_FN_MAX_IRQS entries.  Nothing guarantees that this scan sees
> +	 * the same table -- the PDT is read back from the device every
> +	 * time -- so a device that grows its interrupt counts between the
> +	 * two scans would push the set_bit() calls below past the end of
> +	 * the flexible array.  Refuse the function instead, before anything
> +	 * is allocated for it.
> +	 */
> +	if (pdt->interrupt_source_count > RMI_FN_MAX_IRQS ||
> +	    *current_irq_count + pdt->interrupt_source_count > data->irq_count) {
> +		dev_err(dev,
> +			"F%02X: interrupt count changed between PDT scans (pos %u + %u > %d)\n",
> +			pdt->function_number, *current_irq_count,
> +			pdt->interrupt_source_count, data->irq_count);
> +		return -EINVAL;

[Severity: Medium]
Returning -EINVAL here aborts the device probe and exposes regressions where
the IRQ domain, function devices, and sysfs nodes are not properly freed in
the error handling paths of rmi_driver_probe() and its helpers.

First, does this error path permanently leak the IRQ domain? The domain is
allocated without devm in rmi_probe_interrupts():

drivers/input/rmi4/rmi_driver.c:rmi_probe_interrupts() {
	...
	data->irqdomain = irq_domain_create_linear(fwnode, irq_count,
						   &irq_domain_simple_ops,
						   data);
	...
	data->irq_memory = devm_kcalloc(dev, size, 4, GFP_KERNEL);
	if (!data->irq_memory) {
		dev_err(dev, "Failed to allocate memory for irq masks.\n");
		return -ENOMEM;
	}
	...
}

If devm_kcalloc() fails, it returns without calling irq_domain_remove().
Similarly, if rmi_create_function() returns -EINVAL (the new error path
added by this patch), the teardown block in rmi_driver_probe() fails to remove
the domain:

drivers/input/rmi4/rmi_driver.c:rmi_driver_probe() {
	...
err_destroy_functions:
	rmi_free_function_list(rmi_dev);
err:
	return retval;
}

Second, does jumping directly to the err label in rmi_driver_probe() leak
the RMI function devices?

drivers/input/rmi4/rmi_driver.c:rmi_driver_probe() {
	...
	retval = rmi_f34_create_sysfs(rmi_dev);
	if (retval)
		goto err;
	...
}

If rmi_f34_create_sysfs() fails, the jump to the err label skips the
err_destroy_functions label and rmi_free_function_list(rmi_dev).

Finally, does the err_destroy_functions teardown block leave orphaned sysfs
nodes?

drivers/input/rmi4/rmi_driver.c:rmi_driver_probe() {
	...
	retval = rmi_irq_init(rmi_dev);
	if (retval < 0)
		goto err_destroy_functions;
	...
err_destroy_functions:
	rmi_free_function_list(rmi_dev);
err:
	...
}

If rmi_irq_init() or input_register_device() fails, the jump to
err_destroy_functions does not call rmi_f34_remove_sysfs(rmi_dev),
leaking the sysfs entries created earlier.

> +	}
> +
>  	fn = rmi_alloc_function(rmi_dev, pdt->function_number);
>  	if (!fn) {
>  		dev_err(dev, "Failed to allocate memory for F%02X\n",

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011083730.223083-1-98lawweijie@gmail.com?part=2