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

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

[Wei Jie Law]

rmi_driver_probe() reuses the transport driver's input device when it has
one, and then renames it out of memory owned by the RMI device:

	if (rmi_dev->xport->input) {
		data->input = rmi_dev->xport->input;
	...
	name = devm_kasprintf(&rmi_dev->dev, GFP_KERNEL,
			      "Synaptics %s", device_name);
	if (!name)
		return;

	input->name = name;

In the borrowed case the name outlives its allocation.  hid-rmi is the
transport that hits it: rmi_input_configured() hands us its hidinput
device, and rmi_remove() then tears the RMI device down first:

	rmi_unregister_transport_device(&hdata->xport);
	hid_hw_stop(hdev);

The first line unbinds this driver, so devres frees the name.  The second
reaches input_unregister_device(), whose device_del() emits the
KOBJ_REMOVE uevent, and input_dev_uevent() does

	if (dev->name)
		INPUT_ADD_HOTPLUG_VAR("NAME=\"%s\"", dev->name);

so vsnprintf() walks the freed string and copies it into the uevent that
is broadcast to userspace.  With slub_debug=FZPU the remove event carries

  NAME="kkkkkkkkkkkkkkkkkkkkkkk\xa5\xbb\xbb\xbb\xbb\xbb\xbb\xbb\xbb..."

i.e. POISON_FREE, POISON_END, the redzone and the SLUB tracking metadata
past the end of the object - the read runs to the first NUL, so it leaves
the object as well.

  BUG: KASAN: slab-use-after-free in string+0x2a9/0x330
  Read of size 1 at addr ffff88810a379d31 by task name/8056
   string+0x2a9/0x330
   vsnprintf+0x5ec/0x1680
   add_uevent_var+0x165/0x390
   input_dev_uevent+0x14c/0x750
   kobject_uevent_env+0x4ed/0x11b0
   device_del+0x5ac/0x940
   input_unregister_device+0x88/0xc0
   hidinput_disconnect+0x144/0x3e0
   hid_hw_stop+0x13/0x70
  Allocated by task 7810:
   devm_kasprintf+0xb0/0xe0
   rmi_driver_probe+0x3e0/0xbf0 [rmi_core]
   rmi_input_configured+0x184/0x2e0 [hid_rmi]
  Freed by task 8056:
   devres_release_all+0x106/0x170
   device_release_driver_internal+0x3f9/0x550
   rmi_unregister_transport_device+0x34/0x50 [rmi_core]
   rmi_remove+0xd0/0x100 [hid_rmi]

This is not limited to the RMI_DEVICE_HAS_PHYS_BUTTONS models: any RMI4
sensor reaches it, because rmi_init_functions() runs inside
rmi_input_configured() and F11/F12/F30 call input_set_capability() on the
borrowed device, so hidinput_connect() finds it populated and registers
it.  11 to 14 reports per three unplugs here.

The same string is read on the add side too.
rmi_register_transport_device() returns 0 whenever device_add() succeeded,
so a probe failure of this driver - rmi_enable_sensor() failing on a
register read is enough, and a HID device that stops answering can arrange
that - does not stop the transport: devres frees the name, and
hidinput_connect() then goes on to call input_register_device(), which
prints the name and emits KOBJ_ADD, with the same KASAN report under
input_register_device() <- hidinput_connect().

Moving the allocation onto the input device does not help - device_del()
releases devres before it emits the uevent - so put the name back to a
string with static storage duration when this driver lets go of an input
device it does not own, both on remove and on the probe error paths.

That means writing to the borrowed device after hidinput_connect() may
already have thrown it away: if none of the RMI functions populated it,
hidinput_connect() calls hidinput_cleanup_hidinput() and frees it while
data->input still points there.  Take a reference for as long as we keep
the pointer, which also stops rmi_process_interrupt_requests() from
input_sync()ing a freed device.

Fixes: 2b6a321da9a2 ("Input: synaptics-rmi4 - add support for Synaptics RMI4 devices")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Assisted-by: GLM:glm-5.3
Signed-off-by: Wei Jie Law <98lawweijie@gmail.com>
---
Changes in v2:
 - No code change: the diff is identical to v1.  Adds the Assisted-by
   tags required by Documentation/process/coding-assistants.rst.
 - Commit message trimmed; the KASAN reports are quoted in short form
   and the add-path report is summarised rather than repeated in full.

v1: https://lore.kernel.org/linux-input/20260825031315.51860-1-98lawweijie@gmail.com/

 drivers/input/rmi4/rmi_driver.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 5d49a9021c7d..8696a6aa0fa9 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;
+	input_put_device(data->input);
+	data->input = NULL;
+}
+
 static int rmi_driver_set_irq_bits(struct rmi_device *rmi_dev,
 				   unsigned long *mask)
 {
@@ -1026,6 +1044,8 @@ static int rmi_driver_remove(struct device *dev)
 	rmi_f34_remove_sysfs(rmi_dev);
 	rmi_free_function_list(rmi_dev);
 
+	rmi_driver_put_input(rmi_dev, data);
+
 	irq_domain_remove(data->irqdomain);
 	data->irqdomain = NULL;
 
@@ -1231,7 +1251,7 @@ static int rmi_driver_probe(struct device *dev)
 		 * One example is some HID touchpads report "pass-through"
 		 * button events are not reported by rmi registers.
 		 */
-		data->input = rmi_dev->xport->input;
+		data->input = input_get_device(rmi_dev->xport->input);
 	} else {
 		data->input = devm_input_allocate_device(dev);
 		if (!data->input) {
@@ -1287,6 +1307,7 @@ static int rmi_driver_probe(struct device *dev)
 err_destroy_functions:
 	rmi_free_function_list(rmi_dev);
 err:
+	rmi_driver_put_input(rmi_dev, data);
 	return retval;
 }
 
-- 
2.43.0

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

[sashiko-bot]

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