[PATCH 0/2] HID: alps: fix DualPoint Stick input device lifecycle

[PATCH 0/2] HID: alps: fix DualPoint Stick input device lifecycle

[Chen Changcheng]

This series fixes two issues in the hid-alps driver related to the
separate input device ("DualPoint Stick", input2) that is allocated in
alps_input_configured() but not registered in hdev->inputs:

  Patch 1: input2 is never unregistered on remove.
           The driver struct has no .remove handler, and input2 is not
           tracked in hdev->inputs, so the default remove path
           (hid_hw_stop -> hidinput_disconnect) skips it.  Result: every
           device removal leaks one struct input_dev.

  Patch 2: data->input2 is stored before registration, and on
           registration failure the dangling pointer is read by
           u1_raw_event() (URBs are already active because
           alps_input_configured() calls hid_hw_open() before
           allocating input2) -> use-after-free.

Both patches apply against linux.git master at 7.2-rc7.

---
Chen Changcheng (2):
  HID: alps: unregister DualPoint Stick input device on remove
  HID: alps: fix use-after-free on input2 registration failure

 drivers/hid/hid-alps.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

Thanks,
Chen Changcheng
-- 
2.25.1

[PATCH 1/2] HID: alps: unregister DualPoint Stick input device on remove

[Chen Changcheng]

alps_input_configured() allocates a second input device ("DualPoint
Stick") with input_allocate_device() and registers it, but the
alps_driver struct has no .remove handler and input2 is not tracked in
hdev->inputs.  The default remove path (hid_hw_stop -> hidinput_disconnect)
only iterates hdev->inputs, so input2 is never unregistered and leaks
on every device removal.

Add a .remove handler that stops the device first (preventing URB
callbacks from touching input2 during teardown) and then unregisters
input2.

Fixes: 2562756dde55 ("HID: add Alps I2C HID Touchpad-Stick support")
Cc: stable@vger.kernel.org
Signed-off-by: Chen Changcheng <chenchangcheng@kylinos.cn>
---
 drivers/hid/hid-alps.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/hid/hid-alps.c b/drivers/hid/hid-alps.c
index 67179e3fe39b..370635f5b704 100644
--- a/drivers/hid/hid-alps.c
+++ b/drivers/hid/hid-alps.c
@@ -823,6 +823,24 @@ static int alps_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	return 0;
 }
 
+static void alps_remove(struct hid_device *hdev)
+{
+	struct alps_dev *data = hid_get_drvdata(hdev);
+
+	/*
+	 * input2 ("DualPoint Stick") is allocated separately and is not
+	 * tracked in hdev->inputs, so the default remove path
+	 * (hid_hw_stop -> hidinput_disconnect) does not unregister it.
+	 *
+	 * Stop the device first so that no URB callback can touch input2
+	 * while it is being unregistered, then drop it explicitly.
+	 */
+	hid_hw_stop(hdev);
+
+	if (data->input2)
+		input_unregister_device(data->input2);
+}
+
 static const struct hid_device_id alps_id[] = {
 	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_ANY,
 		USB_VENDOR_ID_ALPS_JP, HID_DEVICE_ID_ALPS_U1_DUAL) },
@@ -845,6 +863,7 @@ static struct hid_driver alps_driver = {
 	.input_configured	= alps_input_configured,
 	.resume			= pm_ptr(alps_post_resume),
 	.reset_resume		= pm_ptr(alps_post_reset),
+	.remove			= alps_remove,
 };
 
 module_hid_driver(alps_driver);
-- 
2.25.1

Re: [PATCH 1/2] HID: alps: unregister DualPoint Stick input device on remove

[sashiko-bot]

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

New issues:
- [High] If `input_register_device()` fails for `data->input2` in `alps_input_configured()`, the driver frees `input2` but leaves `data->input2` as a dangling pointer. The newly added `alps_remove()` unconditionally uses this dangling pointer, causing a Use-After-Free.

Pre-existing issues:
- [High] Untrusted USB devices can trigger a NULL pointer dereference in `u1_raw_event()` by sending a StickPointer report when the device was not initialized with one.
--

--- Patch [1]: [PATCH 1/2] HID: alps: unregister DualPoint Stick input device on remove ---
commit 93188845b9160c2ea2789c012e11b634ca66357c
Author: Chen Changcheng <chenchangcheng@kylinos.cn>

HID: alps: unregister DualPoint Stick input device on remove

alps_input_configured() allocates a second input device ("DualPoint
Stick") with input_allocate_device() and registers it, but the
alps_driver struct has no .remove handler and input2 is not tracked in
hdev->inputs. The default remove path only iterates hdev->inputs, so
input2 is never unregistered and leaks on every device removal.
This patch adds a .remove handler that stops the device first and then
unregisters input2.

> diff --git a/drivers/hid/hid-alps.c b/drivers/hid/hid-alps.c
> index 67179e3fe39b7..370635f5b7043 100644
> --- a/drivers/hid/hid-alps.c
> +++ b/drivers/hid/hid-alps.c

[ ... ]

> @@ -823,6 +823,24 @@ static int alps_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	return 0;
>  }
>  
> +static void alps_remove(struct hid_device *hdev)
> +{
> +	struct alps_dev *data = hid_get_drvdata(hdev);
> +
> +	/*
> +	 * input2 ("DualPoint Stick") is allocated separately and is not
> +	 * tracked in hdev->inputs, so the default remove path
> +	 * (hid_hw_stop -> hidinput_disconnect) does not unregister it.
> +	 *
> +	 * Stop the device first so that no URB callback can touch input2
> +	 * while it is being unregistered, then drop it explicitly.
> +	 */
> +	hid_hw_stop(hdev);
> +
> +	if (data->input2)
> +		input_unregister_device(data->input2);

[Severity: High]
If input_register_device() fails in alps_input_configured(), input2 is freed
but data->input2 is not set to NULL:

drivers/hid/hid-alps.c:alps_input_configured() {
    ...
		if (input_register_device(data->input2)) {
			input_free_device(input2);
			ret = -ENOENT;
			goto exit;
		}
    ...
}

Because the driver has a .raw_event handler, hid_connect() considers
the device bound and ignores the failure in hidinput_connect(), allowing
probe to succeed.

When the device is later removed, could this result in a use-after-free
when alps_remove() attempts to unregister the dangling data->input2
pointer?

[Severity: High]
This isn't a bug introduced by this patch, but if a malicious or
malfunctioning device indicates it doesn't support the StickPointer
(has_sp == 0), data->input2 is never allocated.

If that device then sends a raw report with U1_SP_ABSOLUTE_REPORT_ID (0x06),
would this trigger a NULL pointer dereference in u1_raw_event()?

drivers/hid/hid-alps.c:u1_raw_event() {
    ...
	case U1_SP_ABSOLUTE_REPORT_ID:
		sp_x = get_unaligned_le16(data+2);
		sp_y = get_unaligned_le16(data+4);

		sp_x = sp_x / 8;
		sp_y = sp_y / 8;

		input_report_rel(hdata->input2, REL_X, sp_x);
    ...
}

> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814070621.68557-1-chenchangcheng@kylinos.cn?part=1

[PATCH 2/2] HID: alps: fix use-after-free on input2 registration failure

[Chen Changcheng]

alps_input_configured() stores data->input2 before calling
input_register_device().  If registration fails, input_free_device()
frees the input device but data->input2 still points to the freed memory.
alps_input_configured() calls hid_hw_open() before allocating input2, so
URBs are already active and raw_event can fire during the failure window.
A U1_SP_ABSOLUTE_REPORT_ID report arriving then causes u1_raw_event()
to dereference the freed data->input2 -> use-after-free.

Fix by only storing input2 into drvdata after successful registration
and adding a NULL guard in the raw_event path.

Fixes: 2562756dde55 ("HID: add Alps I2C HID Touchpad-Stick support")
Cc: stable@vger.kernel.org
Signed-off-by: Chen Changcheng <chenchangcheng@kylinos.cn>
---
 drivers/hid/hid-alps.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-alps.c b/drivers/hid/hid-alps.c
index 370635f5b704..0556cb5645eb 100644
--- a/drivers/hid/hid-alps.c
+++ b/drivers/hid/hid-alps.c
@@ -407,6 +407,8 @@ static int u1_raw_event(struct alps_dev *hdata, u8 *data, int size)
 		return 1;
 
 	case U1_SP_ABSOLUTE_REPORT_ID:
+		if (!hdata->input2)
+			return 0;
 		sp_x = get_unaligned_le16(data+2);
 		sp_y = get_unaligned_le16(data+4);
 
@@ -738,7 +740,6 @@ static int alps_input_configured(struct hid_device *hdev, struct hid_input *hi)
 			goto exit;
 		}
 
-		data->input2 = input2;
 		input2->phys = input->phys;
 		input2->name = "DualPoint Stick";
 		input2->id.bustype = BUS_I2C;
@@ -762,11 +763,12 @@ static int alps_input_configured(struct hid_device *hdev, struct hid_input *hi)
 		__set_bit(INPUT_PROP_POINTER, input2->propbit);
 		__set_bit(INPUT_PROP_POINTING_STICK, input2->propbit);
 
-		if (input_register_device(data->input2)) {
+		if (input_register_device(input2)) {
 			input_free_device(input2);
 			ret = -ENOENT;
 			goto exit;
 		}
+		data->input2 = input2;
 	}
 
 exit:
-- 
2.25.1