Re: [PATCH v2] Input: synaptics-rmi4 - fix input_dev->name use-after-free on unplug

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] Bypassed cleanup of RMI functions on sysfs creation failure leads to memory leak and Use-After-Free.
- [High] Dangling sysfs group on probe error paths leads to NULL Pointer Dereference (Kernel Oops).
- [High] Leak of non-devm irqdomain on all probe error paths leads to Use-After-Free.
- [High] TOCTOU race in sysfs `name` attribute read leading to Use-After-Free.
--

--- Patch [1]: [PATCH v2] Input: synaptics-rmi4 - fix input_dev->name use-after-free on unplug ---
commit d914ebc7a5687cdc5cb5971b151029cd0e21cbb8
Author: Wei Jie Law <98lawweijie@gmail.com>
Input: synaptics-rmi4 - fix input_dev->name use-after-free on unplug

The commit fixes a use-after-free on input_dev->name when an RMI device is
unplugged. The transport driver's input device is reused, but its name is
assigned to memory owned by the RMI device. This causes the name to be freed
before the input device is unregistered. The patch restores the name to a
static string before dropping the reference.

> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index 5d49a9021c7d0..8696a6aa0fa9f 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -369,6 +369,24 @@ static void rmi_driver_set_input_name(struct rmi_device *rmi_dev,
>  	input->name = name;
>  }
>  
> +/*
> + * Let go of an input device that belongs to the transport driver.  It outlives
> + * us, but rmi_driver_set_input_name() pointed its name at devres memory of
> + * ours that is freed as soon as we are done - input_register_device() and both
> + * the add and the remove uevent print that name - so put the name back to a
> + * string with static storage duration before dropping the reference.
> + */
> +static void rmi_driver_put_input(struct rmi_device *rmi_dev,
> +				 struct rmi_driver_data *data)
> +{
> +	if (!data->input || data->input != rmi_dev->xport->input)
> +		return;
> +
> +	data->input->name = SYNAPTICS_INPUT_DEVICE_NAME;

[Severity: High]
This isn't a bug introduced by this patch, but could swapping this pointer
cause a TOCTOU race for lockless sysfs readers?

The sysfs name_show() function reads input_dev->name without locking.
If a concurrent sysfs reader thread loads the old devm-allocated string
pointer just before data->input->name is updated here:

CPU1
sysfs reader
    // reads input_dev->name (old devm string)

CPU2
rmi_driver_put_input()
    data->input->name = SYNAPTICS_INPUT_DEVICE_NAME;

When rmi_driver_remove() completes, devres frees the old string. If the
preempted sysfs reader resumes and uses the old pointer, will it cause a
use-after-free?

> +	input_put_device(data->input);
> +	data->input = NULL;
> +}
> +

[ ... ]

> @@ -1287,6 +1307,7 @@ static int rmi_driver_probe(struct device *dev)
>  err_destroy_functions:
>  	rmi_free_function_list(rmi_dev);
>  err:

[Severity: High]
This is a pre-existing issue, but does rmi_driver_probe() leak RMI functions
if sysfs creation fails?

In rmi_driver_probe(), if rmi_f34_create_sysfs() fails, the code jumps
directly to err:

	retval = rmi_f34_create_sysfs(rmi_dev);
	if (retval)
		goto err;

This bypasses err_destroy_functions:, which calls rmi_free_function_list().
Since the driver's devres memory is freed upon probe failure, will any
function devices left in data->function_list cause a use-after-free later?

[Severity: High]
This is a pre-existing issue, but does the probe error path leave dangling
sysfs files?

If probe fails after rmi_f34_create_sysfs() successfully creates the sysfs
group, the error handling jumps to err_destroy_functions: or
err_disable_irq:. None of these paths call rmi_f34_remove_sysfs().

Because the device is only unbound and not destroyed, the sysfs files remain
accessible. If userspace reads the lingering sysfs file, dev_get_drvdata(dev)
returns NULL. Could this cause rmi_driver_update_fw_status_show() to
dereference a NULL pointer?

[Severity: High]
This is a pre-existing issue, but does rmi_driver_probe() leak the irqdomain
on error paths?

In rmi_probe_interrupts(), the irqdomain is allocated:

	data->irqdomain = irq_domain_create_linear(fwnode, irq_count,
						   &irq_domain_simple_ops,
						   data);

If any failure occurs during probe, the code funnels to err: and returns
without calling irq_domain_remove().

Since the driver core frees data via devres, the leaked irqdomain retains a
freed host_data pointer. Will subsequent IRQ lookups reference this freed
memory?

> +	rmi_driver_put_input(rmi_dev, data);
>  	return retval;
>  }

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