[PATCH 0/2] HID: lenovo: two ordering fixes in remove() and tpkbd probe

[PATCH 0/2] HID: lenovo: two ordering fixes in remove() and tpkbd probe

[Oleg Keri]

Two long-standing ordering bugs in hid-lenovo, both pointed out by the
Sashiko review of an unrelated patch of mine [1].  Neither is specific to
any recent change; the first dates from 2020, the second from the original
2012 trackpoint keyboard driver.

Patch 1: lenovo_remove() cancels the tablet keyboards' fn_lock_sync_work
while the hardware is still delivering the events that re-queue it, so a
key press in that window leaves the work pending across the devres free
of its data.  Stop the hardware before the per-model teardown.

Patch 2: lenovo_probe_tpkbd() exposes sysfs attributes before allocating
the drvdata they dereference.  Allocate first.

Compile-tested only: I have neither a ThinkPad 10 Ultrabook Keyboard nor a
ThinkPad USB Keyboard with TrackPoint.  Both changes are pure reorderings
of existing calls.

[1] https://lore.kernel.org/all/20260910064345.9BF1E1F000FF@smtp.kernel.org/

Oleg Keri (2):
  HID: lenovo: stop the hardware before the per-model teardown
  HID: lenovo: allocate the trackpoint drvdata before exposing its sysfs
    attributes

 drivers/hid/hid-lenovo.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

-- 
2.55.0


base-commit: df2908090cda368b01ff43709f51890076c56157

[PATCH 1/2] HID: lenovo: stop the hardware before the per-model teardown

[Oleg Keri]

lenovo_remove() runs the per-model teardown first and calls hid_hw_stop()
last.  For the ThinkPad 10 Ultrabook Keyboard and the X1/X12/X13 Tablet
keyboards that teardown ends in cancel_work_sync(&data->fn_lock_sync_work),
but the hardware is still delivering events at that point, and
lenovo_event_tp10ubkbd() re-queues that work on every Fn-Esc press.  A
press landing after the cancel leaves the work pending when remove()
returns; devres then frees the drvdata and the work runs on freed memory.

Stop the hardware first so no event can arrive once the teardown starts.
The LED-off writes issued by led_classdev_unregister() after the stop are
harmless: usbhid sets HID_DISCONNECTED in usbhid_stop() and
__usbhid_submit_report() drops the request on it, and the raw_request path
used by the tablet keyboards is a plain synchronous control transfer.

Fixes: bc04b37ea0ec ("HID: lenovo: Add ThinkPad 10 Ultrabook Keyboard support")
Link: https://lore.kernel.org/all/20260910064345.9BF1E1F000FF@smtp.kernel.org/
Signed-off-by: Oleg Keri <okerixx@gmail.com>
---
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 617bba6..ff7c5ef 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -1498,6 +1498,8 @@ static void lenovo_remove_tp10ubkbd(struct hid_device *hdev)
 
 static void lenovo_remove(struct hid_device *hdev)
 {
+	hid_hw_stop(hdev);
+
 	switch (hdev->product) {
 	case USB_DEVICE_ID_LENOVO_TPKBD:
 		lenovo_remove_tpkbd(hdev);
@@ -1518,8 +1520,6 @@ static void lenovo_remove(struct hid_device *hdev)
 		lenovo_remove_tp10ubkbd(hdev);
 		break;
 	}
-
-	hid_hw_stop(hdev);
 }
 
 static int lenovo_input_configured(struct hid_device *hdev,
-- 
2.55.0

[PATCH 2/2] HID: lenovo: allocate the trackpoint drvdata before exposing its sysfs attributes

[Oleg Keri]

lenovo_probe_tpkbd() creates its sysfs group before allocating the
lenovo_drvdata that every attribute handler dereferences through
hid_get_drvdata(), and drvdata is explicitly NULL at that point.  A read of
e.g. the sensitivity attribute between the two steps is a NULL pointer
dereference.

Allocate and install the drvdata first, then create the group.  The
allocation failure path no longer has a group to remove, so it returns
directly.

Fixes: c1dcad2d32d0 ("HID: Driver for Lenovo Keyboard with Trackpoint")
Link: https://lore.kernel.org/all/20260910064345.9BF1E1F000FF@smtp.kernel.org/
Signed-off-by: Oleg Keri <okerixx@gmail.com>
---
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index ff7c5ef..62bb805 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -1253,17 +1253,12 @@ static int lenovo_probe_tpkbd(struct hid_device *hdev)
 	if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 3, 0, 2))
 		return -ENODEV;
 
-	ret = sysfs_create_group(&hdev->dev.kobj, &lenovo_attr_group_tpkbd);
-	if (ret)
-		hid_warn(hdev, "Could not create sysfs group: %d\n", ret);
-
 	data_pointer = devm_kzalloc(&hdev->dev,
 				    sizeof(struct lenovo_drvdata),
 				    GFP_KERNEL);
 	if (data_pointer == NULL) {
 		hid_err(hdev, "Could not allocate memory for driver data\n");
-		ret = -ENOMEM;
-		goto err;
+		return -ENOMEM;
 	}
 
 	// set same default values as windows driver
@@ -1272,6 +1267,10 @@ static int lenovo_probe_tpkbd(struct hid_device *hdev)
 
 	hid_set_drvdata(hdev, data_pointer);
 
+	ret = sysfs_create_group(&hdev->dev.kobj, &lenovo_attr_group_tpkbd);
+	if (ret)
+		hid_warn(hdev, "Could not create sysfs group: %d\n", ret);
+
 	ret = lenovo_register_leds(hdev);
 	if (ret)
 		goto err;
-- 
2.55.0