[PATCH] Input: synaptics-rmi4 - fix input device name lifetime with hid-rmi

[PATCH] Input: synaptics-rmi4 - fix input device name lifetime with hid-rmi

[Dmitry Torokhov]

rmi_driver_set_input_name() allocates the input device name string
using devm_kasprintf(&rmi_dev->dev, ...). When the input device is
supplied by a transport driver via rmi_dev->xport->input (such as
hid-rmi), the input device's lifetime is owned by the transport device
rather than rmi_dev.

When rmi_remove() in hid-rmi calls rmi_unregister_transport_device()
before hid_hw_stop(), or if rmi_driver_probe() fails after setting the
name, or if rmi_dev is unbound via sysfs, devres releases the name
string while the HID input device remains registered, causing a
use-after-free when input->name is later accessed (for example when
generating the KOBJ_REMOVE uevent or reading /proc/bus/input/devices).

Allocate the name string on input->dev.parent instead of &rmi_dev->dev:
- For hid-rmi, input->dev.parent is &hdev->dev, whose driver devres
  group is released in hid_device_remove() after rmi_remove() and
  hid_hw_stop() have unregistered the input device.
- When rmi_driver_probe() allocates the input device itself via
  devm_input_allocate_device(), input->dev.parent is &rmi_dev->dev, and
  the subsequent input_register_device() call pushes the unregistration
  devres action onto &rmi_dev->dev after the name allocation, so the
  input device is unregistered before the name is freed.

Fixes: 2b6a321da9a2 ("Input: synaptics-rmi4 - add support for Synaptics RMI4 devices")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/rmi4/rmi_driver.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index a349dfd17519..4ab00b2cf4f0 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -361,7 +361,14 @@ static void rmi_driver_set_input_name(struct rmi_device *rmi_dev,
 	const char *device_name = rmi_f01_get_product_ID(data->f01_container);
 	char *name;
 
-	name = devm_kasprintf(&rmi_dev->dev, GFP_KERNEL,
+	/*
+	 * Allocate the name string on the input device's parent rather than
+	 * rmi_dev->dev so that when the input device is supplied by a
+	 * transport driver (such as hid-rmi), the name string is not freed
+	 * when rmi_dev is torn down before the transport unregisters its
+	 * input device.
+	 */
+	name = devm_kasprintf(input->dev.parent, GFP_KERNEL,
 			      "Synaptics %s", device_name);
 	if (!name)
 		return;
-- 
2.56.0.rc1.315.gc6ed9934b7-goog


-- 
Dmitry

Re: [PATCH] Input: synaptics-rmi4 - fix input device name lifetime with hid-rmi

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm: devm memory leak of input device name on rmi_driver probe failure or unbind
--

commit cb8d75744bf1ae82958b47feab42ec5de7126321
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Input: synaptics-rmi4 - fix input device name lifetime with hid-rmi

This commit changes the input device name allocation to use the parent device
instead of the child device. This addresses a use-after-free issue when the
input device is supplied by a transport driver such as hid-rmi.

> diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> index 5d49a9021c7d0..5b104b2b8e19a 100644
> --- a/drivers/input/rmi4/rmi_driver.c
> +++ b/drivers/input/rmi4/rmi_driver.c
> @@ -361,7 +361,14 @@ static void rmi_driver_set_input_name(struct rmi_device *rmi_dev,
>  	const char *device_name = rmi_f01_get_product_ID(data->f01_container);
>  	char *name;
>  
> -	name = devm_kasprintf(&rmi_dev->dev, GFP_KERNEL,
> +	/*
> +	 * Allocate the name string on the input device's parent rather than
> +	 * rmi_dev->dev so that when the input device is supplied by a
> +	 * transport driver (such as hid-rmi), the name string is not freed
> +	 * when rmi_dev is torn down before the transport unregisters its
> +	 * input device.
> +	 */
> +	name = devm_kasprintf(input->dev.parent, GFP_KERNEL,

[Severity: Medium]
Does tying this devm allocation to the parent transport device (e.g., hdev->dev
when using hid-rmi) cause a memory leak when the child device fails to probe
or is unbound/rebound?

If the RMI driver probe fails subsequently, or if the RMI driver is manually
unbound via sysfs and rebound, the devres group for the parent HID device is
not cleaned up. Each probe attempt allocates a new string on the parent's
devres list, leaking the previous allocations until the parent HID device
is eventually unbound.

>  			      "Synaptics %s", device_name);
>  	if (!name)
>  		return;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/ar38b7ToAe8iGU5v@google.com?part=1