From 04515e08bca9b5a1f9729b8d99d8b322e56d7454 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 10 Jul 2025 12:41:20 +0300 Subject: HID: debug: Remove duplicate entry (BTN_WHEEL) BTN_WHEEL is duplicated (by value) and compiler is not happy about that: drivers/hid/hid-debug.c:3302:16: error: initializer overrides prior initialization of this subobject [-Werror,-Winitializer-overrides] 3302 | [BTN_WHEEL] = "BtnWheel", [KEY_OK] = "Ok", | ^~~~~~~~~~ drivers/hid/hid-debug.c:3301:20: note: previous initialization is here 3301 | [BTN_GEAR_DOWN] = "BtnGearDown", [BTN_GEAR_UP] = "BtnGearUp", | ^~~~~~~~~~~~~ Remove it again, as the commit 7b2daa648eb7 ("HID: debug: Remove duplicates from 'keys'") already did this once in the past. Fixes: 194808a1ea39 ("HID: Fix debug name for BTN_GEAR_DOWN, BTN_GEAR_UP, BTN_WHEEL") Signed-off-by: Andy Shevchenko Link: https://patch.msgid.link/20250710094120.753358-1-andriy.shevchenko@linux.intel.com Signed-off-by: Benjamin Tissoires --- drivers/hid/hid-debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/hid') diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c index c6b6b1029540..4424c0512bae 100644 --- a/drivers/hid/hid-debug.c +++ b/drivers/hid/hid-debug.c @@ -3299,7 +3299,7 @@ static const char *keys[KEY_MAX + 1] = { [BTN_STYLUS2] = "Stylus2", [BTN_TOOL_DOUBLETAP] = "ToolDoubleTap", [BTN_TOOL_TRIPLETAP] = "ToolTripleTap", [BTN_TOOL_QUADTAP] = "ToolQuadrupleTap", [BTN_GEAR_DOWN] = "BtnGearDown", [BTN_GEAR_UP] = "BtnGearUp", - [BTN_WHEEL] = "BtnWheel", [KEY_OK] = "Ok", + [KEY_OK] = "Ok", [KEY_SELECT] = "Select", [KEY_GOTO] = "Goto", [KEY_CLEAR] = "Clear", [KEY_POWER2] = "Power2", [KEY_OPTION] = "Option", [KEY_INFO] = "Info", -- cgit v1.2.3 From 4f15ee98304b96e164ff2340e1dfd6181c3f42aa Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Thu, 10 Jul 2025 16:01:33 +0200 Subject: HID: core: ensure the allocated report buffer can contain the reserved report ID When the report ID is not used, the low level transport drivers expect the first byte to be 0. However, currently the allocated buffer not account for that extra byte, meaning that instead of having 8 guaranteed bytes for implement to be working, we only have 7. Reported-by: Alan Stern Closes: https://lore.kernel.org/linux-input/c75433e0-9b47-4072-bbe8-b1d14ea97b13@rowland.harvard.edu/ Cc: stable@vger.kernel.org Suggested-by: Alan Stern Link: https://patch.msgid.link/20250710-report-size-null-v2-1-ccf922b7c4e5@kernel.org Signed-off-by: Benjamin Tissoires --- drivers/hid/hid-core.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/hid') diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index b348d0464314..1a231dd9e4bc 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1883,9 +1883,12 @@ u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags) /* * 7 extra bytes are necessary to achieve proper functionality * of implement() working on 8 byte chunks + * 1 extra byte for the report ID if it is null (not used) so + * we can reserve that extra byte in the first position of the buffer + * when sending it to .raw_request() */ - u32 len = hid_report_len(report) + 7; + u32 len = hid_report_len(report) + 7 + (report->id == 0); return kzalloc(len, flags); } -- cgit v1.2.3 From 0d0777ccaa2d46609d05b66ba0096802a2746193 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Thu, 10 Jul 2025 16:01:34 +0200 Subject: HID: core: ensure __hid_request reserves the report ID as the first byte The low level transport driver expects the first byte to be the report ID, even when the report ID is not use (in which case they just shift the buffer). However, __hid_request() whas not offsetting the buffer it used by one in this case, meaning that the raw_request() callback emitted by the transport driver would be stripped of the first byte. Note: this changes the API for uhid devices when a request is made through hid_hw_request. However, several considerations makes me think this is fine: - every request to a HID device made through hid_hw_request() would see that change, but every request made through hid_hw_raw_request() already has the new behaviour. So that means that the users are already facing situations where they might have or not the first byte being the null report ID when it is 0. We are making things more straightforward in the end. - uhid is mainly used for BLE devices - uhid is also used for testing, but I don't see that change a big issue - for BLE devices, we can check which kernel module is calling hid_hw_request() - and in those modules, we can check which are using a Bluetooth device - and then we can check if the command is used with a report ID or not. - surprise: none of the kernel module are using a report ID 0 - and finally, bluez, in its function set_report()[0], does the same shift if the report ID is 0 and the given buffer has a size > 0. [0] https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/profiles/input/hog-lib.c#n879 Reported-by: Alan Stern Closes: https://lore.kernel.org/linux-input/c75433e0-9b47-4072-bbe8-b1d14ea97b13@rowland.harvard.edu/ Reported-by: syzbot+8258d5439c49d4c35f43@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=8258d5439c49d4c35f43 Tested-by: syzbot+8258d5439c49d4c35f43@syzkaller.appspotmail.com Fixes: 4fa5a7f76cc7 ("HID: core: implement generic .request()") Cc: stable@vger.kernel.org Link: https://patch.msgid.link/20250710-report-size-null-v2-2-ccf922b7c4e5@kernel.org Signed-off-by: Benjamin Tissoires --- drivers/hid/hid-core.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'drivers/hid') diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 1a231dd9e4bc..320887c365f7 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1976,7 +1976,7 @@ static struct hid_report *hid_get_report(struct hid_report_enum *report_enum, int __hid_request(struct hid_device *hid, struct hid_report *report, enum hid_class_request reqtype) { - char *buf; + char *buf, *data_buf; int ret; u32 len; @@ -1984,10 +1984,17 @@ int __hid_request(struct hid_device *hid, struct hid_report *report, if (!buf) return -ENOMEM; + data_buf = buf; len = hid_report_len(report); + if (report->id == 0) { + /* reserve the first byte for the report ID */ + data_buf++; + len++; + } + if (reqtype == HID_REQ_SET_REPORT) - hid_output_report(report, buf); + hid_output_report(report, data_buf); ret = hid->ll_driver->raw_request(hid, report->id, buf, len, report->type, reqtype); -- cgit v1.2.3 From c2ca42f190b6714d6c481dfd3d9b62ea091c946b Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Thu, 10 Jul 2025 16:01:35 +0200 Subject: HID: core: do not bypass hid_hw_raw_request hid_hw_raw_request() is actually useful to ensure the provided buffer and length are valid. Directly calling in the low level transport driver function bypassed those checks and allowed invalid paramto be used. Reported-by: Alan Stern Closes: https://lore.kernel.org/linux-input/c75433e0-9b47-4072-bbe8-b1d14ea97b13@rowland.harvard.edu/ Cc: stable@vger.kernel.org Link: https://patch.msgid.link/20250710-report-size-null-v2-3-ccf922b7c4e5@kernel.org Signed-off-by: Benjamin Tissoires --- drivers/hid/hid-core.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/hid') diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 320887c365f7..b31b8a2fd540 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1996,8 +1996,7 @@ int __hid_request(struct hid_device *hid, struct hid_report *report, if (reqtype == HID_REQ_SET_REPORT) hid_output_report(report, data_buf); - ret = hid->ll_driver->raw_request(hid, report->id, buf, len, - report->type, reqtype); + ret = hid_hw_raw_request(hid, report->id, buf, len, report->type, reqtype); if (ret < 0) { dbg_hid("unable to complete request: %d\n", ret); goto out; -- cgit v1.2.3