[PATCH 0/9] HID: Hardening report validation and unnumbered report handling

[PATCH 0/9] HID: Hardening report validation and unnumbered report handling

[Dmitry Torokhov]

This series improves validation and consistency across HID core,
transport drivers, and individual device drivers for numbered and
unnumbered reports.

Bluetooth HIDP and HID core report retrieval handling are updated to
properly respect the kernel convention where raw report request buffers
reserve the first byte for the report ID, using 0 for unnumbered
reports.

Report descriptors mixing numbered and unnumbered reports within the
same report enumeration are rejected in accordance with the HID
specification.

Report ID verification for successful report retrieval transfers is
centralized in the HID core, with a new device quirk to accommodate
buggy firmware (such as the ASUS ROG Flow Z13 touchpad). Redundant
report ID checks are removed across the I2C-HID, Goodix SPI, Intel
QuickI2C, multitouch, and PlayStation drivers.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
Dmitry Torokhov (9):
      Bluetooth: HIDP: Fix GET_REPORT and SET_REPORT for unnumbered reports
      HID: core: Fix GET_REPORT input processing for unnumbered reports
      HID: core: Reject mixing numbered and unnumbered reports
      HID: core: Validate GET_REPORT report ID centrally
      HID: i2c-hid: Drop redundant report ID validation
      HID: goodix-spi: Drop redundant report ID validation
      HID: intel-thc-hid: Drop redundant report ID validation in quicki2c
      HID: multitouch: Drop redundant report ID validation in mt_get_feature
      HID: playstation: Drop redundant report ID validation in ps_get_report

 drivers/hid/hid-core.c                             | 26 ++++++++++++++++--
 drivers/hid/hid-goodix-spi.c                       |  5 ----
 drivers/hid/hid-multitouch.c                       | 32 ----------------------
 drivers/hid/hid-playstation.c                      |  5 ----
 drivers/hid/hid-quirks.c                           |  2 ++
 drivers/hid/i2c-hid/i2c-hid-core.c                 |  7 -----
 .../intel-quicki2c/quicki2c-protocol.c             |  6 ++--
 include/linux/hid.h                                |  2 ++
 net/bluetooth/hidp/core.c                          | 29 ++++++++++++++++----
 9 files changed, 53 insertions(+), 61 deletions(-)
---
base-commit: b018686719706a781c45a874ed51374a2dc4b767
change-id: 20261012-hid-core-report-validation-8ca85d385ca7

Thanks.

-- 
Dmitry

[PATCH 1/9] Bluetooth: HIDP: Fix GET_REPORT and SET_REPORT for unnumbered reports

[Dmitry Torokhov]

Internally, the kernel HID subsystem expects that report buffers for
both numbered and unnumbered raw report requests begin with a report ID
byte, using report ID 0 for unnumbered reports.

Adjust Bluetooth HIDP transport callbacks to adhere to this convention.
When fetching an unnumbered report, request the report without a report
ID over the control channel, prepend the report ID 0 byte to the
received data, and account for it in the returned length. Similarly,
when setting an unnumbered report, strip the leading dummy report ID
before sending the transfer while including it in the reported transfer
count.

Fixes: 0a7f364e8122 ("HID: Add the transport-driver functions to the HIDP driver.")
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 net/bluetooth/hidp/core.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index f5bdf9f1ca63..004a8eed51fb 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -248,7 +248,6 @@ static int hidp_get_raw_report(struct hid_device *hid,
 	struct hidp_session *session = hid->driver_data;
 	struct sk_buff *skb;
 	size_t len;
-	int numbered_reports = hid->report_enum[report_type].numbered;
 	int ret;
 
 	if (atomic_read(&session->terminate))
@@ -273,10 +272,11 @@ static int hidp_get_raw_report(struct hid_device *hid,
 
 	/* Set up our wait, and send the report request to the device. */
 	session->waiting_report_type = report_type & HIDP_DATA_RTYPE_MASK;
-	session->waiting_report_number = numbered_reports ? report_number : -1;
+	session->waiting_report_number = report_number ? report_number : -1;
 	set_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
+
 	data[0] = report_number;
-	ret = hidp_send_ctrl_message(session, report_type, data, 1);
+	ret = hidp_send_ctrl_message(session, report_type, data, !!report_number);
 	if (ret < 0)
 		goto err;
 
@@ -302,11 +302,25 @@ static int hidp_get_raw_report(struct hid_device *hid,
 		}
 	}
 
+	/*
+	 * In case of unnumbered reports the response from the device will
+	 * not have the report ID that the upper layers expect, so we need
+	 * to stash it in the buffer ourselves and adjust the data size.
+	 */
+	if (!report_number) {
+		data[0] = 0;
+		data++;
+		count--;
+	}
+
 	skb = session->report_return;
 	if (skb) {
-		len = skb->len < count ? skb->len : count;
+		len = min_t(size_t, skb->len, count);
 		memcpy(data, skb->data, len);
 
+		if (!report_number && len)
+			len++;
+
 		kfree_skb(skb);
 		session->report_return = NULL;
 	} else {
@@ -350,9 +364,12 @@ static int hidp_set_raw_report(struct hid_device *hid, unsigned char reportnum,
 		return -ERESTARTSYS;
 
 	/* Set up our wait, and send the report request to the device. */
-	data[0] = reportnum;
+	if (reportnum)
+		data[0] = reportnum;
+
 	set_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags);
-	ret = hidp_send_ctrl_message(session, report_type, data, count);
+	ret = hidp_send_ctrl_message(session, report_type,
+				     data + !reportnum, count - !reportnum);
 	if (ret < 0)
 		goto err;
 

-- 
2.56.0.385.gd3acb90ef8-goog

RE: HID: Hardening report validation and unnumbered report handling

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 853 bytes --]

This is an automated email and please do not reply to this email.

Dear Submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
While preparing the CI tests, the patches you submitted couldn't be applied to the current HEAD of the repository.

----- Output -----

error: patch failed: drivers/hid/hid-multitouch.c:79
error: drivers/hid/hid-multitouch.c: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"

Please resolve the issue and submit the patches again.


---
Regards,
Linux Bluetooth

[PATCH 2/9] HID: core: Fix GET_REPORT input processing for unnumbered reports

[Dmitry Torokhov]

When emulating report retrieval requests in the HID core, low-level
transport drivers store a dummy report ID of 0 at the start of the
buffer for unnumbered reports, with the actual report payload placed
after the initial byte.

Passing the raw transfer buffer and total returned length directly to
the input layer causes unnumbered reports to be processed with an
extraneous leading byte and an incorrect payload length. Offset the
buffer past the leading byte and deduct the dummy report ID from the
length when forwarding unnumbered reports to the input layer.

Fixes: 0d0777ccaa2d ("HID: core: ensure __hid_request reserves the report ID as the first byte")
Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/hid/hid-core.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index ec7c2860c93e..a33e864a25f9 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2033,8 +2033,13 @@ int __hid_request(struct hid_device *hid, struct hid_report *report,
 		return ret;
 	}
 
-	if (reqtype == HID_REQ_GET_REPORT)
-		hid_input_report(hid, report->type, buf, ret, 0);
+	if (reqtype == HID_REQ_GET_REPORT) {
+		if (report->id == 0)
+			ret--;
+
+		if (ret > 0)
+			hid_input_report(hid, report->type, data_buf, ret, 0);
+	}
 
 	return 0;
 }

-- 
2.56.0.385.gd3acb90ef8-goog

[PATCH 3/9] HID: core: Reject mixing numbered and unnumbered reports

[Dmitry Torokhov]

According to the Device Class Definition for HID, if any report of a
given type includes a Report ID tag, all reports of that type must
include a Report ID tag. Mixing numbered and unnumbered reports within
the same report enumeration is invalid and causes confusion when sizing
and parsing incoming and outgoing transfers.

Reject descriptors that attempt to mix numbered and unnumbered reports
within the same report enumeration by warning and aborting report
registration.

Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/hid/hid-core.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index a33e864a25f9..3a9bd56dbf85 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -96,6 +96,12 @@ struct hid_report *hid_register_report(struct hid_device *device,
 	if (report_enum->report_id_hash[id])
 		return report_enum->report_id_hash[id];
 
+	if ((id == 0 && report_enum->numbered) ||
+	    (id != 0 && report_enum->report_id_hash[0])) {
+		hid_warn(device, "not allowed to mix numbered and unnumbered reports\n");
+		return NULL;
+	}
+
 	report = kzalloc_obj(struct hid_report);
 	if (!report)
 		return NULL;

-- 
2.56.0.385.gd3acb90ef8-goog

[PATCH 4/9] HID: core: Validate GET_REPORT report ID centrally

[Dmitry Torokhov]

Several transport drivers and device drivers individually check whether
a successful report retrieval response returns the expected report ID.

Centralize this check in the core HID raw request path so that all
report retrieval requests verify that the start of the returned buffer
matches the requested report ID. Introduce a new quirk,
HID_QUIRK_IGNORE_REPORT_ID_MISMATCH, for hardware with buggy firmware
(such as the ASUS ROG Flow Z13 touchpad) that returns mismatched report
IDs on feature fetches.

Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/hid/hid-core.c   | 11 ++++++++++-
 drivers/hid/hid-quirks.c |  2 ++
 include/linux/hid.h      |  2 ++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 3a9bd56dbf85..cd099a420b9b 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2565,8 +2565,17 @@ int __hid_hw_raw_request(struct hid_device *hdev,
 	if (ret)
 		return ret;
 
-	return hdev->ll_driver->raw_request(hdev, reportnum, buf, len,
+	ret = hdev->ll_driver->raw_request(hdev, reportnum, buf, len,
 					    rtype, reqtype);
+	if (ret > 0 && reqtype == HID_REQ_GET_REPORT &&
+	    !(hdev->quirks & HID_QUIRK_IGNORE_REPORT_ID_MISMATCH) &&
+	    buf[0] != reportnum) {
+		hid_warn(hdev, "mismatched report ID %u (expected %u)\n",
+			 buf[0], reportnum);
+		return -EINVAL;
+	}
+
+	return ret;
 }
 
 /**
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 96a36c5ba04f..0ea592d1c9e7 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -37,6 +37,8 @@ static const struct hid_device_id hid_quirks[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ALPS, USB_DEVICE_ID_IBM_GAMEPAD), HID_QUIRK_BADPAD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_AMI, USB_DEVICE_ID_AMI_VIRT_KEYBOARD_AND_MOUSE), HID_QUIRK_ALWAYS_POLL },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_ALU_REVB_ANSI), HID_QUIRK_ALWAYS_POLL },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_ROG_Z13_FOLIO),
+		HID_QUIRK_IGNORE_REPORT_ID_MISMATCH },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ATEN, USB_DEVICE_ID_ATEN_2PORTKVM), HID_QUIRK_NOGET },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ATEN, USB_DEVICE_ID_ATEN_4PORTKVMC), HID_QUIRK_NOGET },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ATEN, USB_DEVICE_ID_ATEN_4PORTKVM), HID_QUIRK_NOGET },
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 8d17b741638c..8f5e485b1351 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -388,6 +388,7 @@ struct hid_item {
  * | @HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE:
  * | @HID_QUIRK_IGNORE_SPECIAL_DRIVER
  * | @HID_QUIRK_POWER_ON_AFTER_BACKLIGHT
+ * | @HID_QUIRK_IGNORE_REPORT_ID_MISMATCH:
  * | @HID_QUIRK_FULLSPEED_INTERVAL:
  * | @HID_QUIRK_NO_INIT_REPORTS:
  * | @HID_QUIRK_NO_IGNORE:
@@ -416,6 +417,7 @@ struct hid_item {
 #define HID_QUIRK_NOINVERT			BIT(21)
 #define HID_QUIRK_IGNORE_SPECIAL_DRIVER		BIT(22)
 #define HID_QUIRK_POWER_ON_AFTER_BACKLIGHT	BIT(23)
+#define HID_QUIRK_IGNORE_REPORT_ID_MISMATCH	BIT(24)
 #define HID_QUIRK_FULLSPEED_INTERVAL		BIT(28)
 #define HID_QUIRK_NO_INIT_REPORTS		BIT(29)
 #define HID_QUIRK_NO_IGNORE			BIT(30)

-- 
2.56.0.385.gd3acb90ef8-goog

[PATCH 5/9] HID: i2c-hid: Drop redundant report ID validation

[Dmitry Torokhov]

Now that the core HID request handling validates returned report IDs
for report retrieval requests centrally, the report ID mismatch check
in the I2C-HID transport driver is redundant. Drop it.

Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/hid/i2c-hid/i2c-hid-core.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index a8a9854854f5..d8ed7bb8e80c 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -311,13 +311,6 @@ static int i2c_hid_get_report(struct i2c_hid *ihid,
 	recv_len = min(recv_len, ret_count - sizeof(__le16));
 	memcpy(recv_buf, ihid->rawbuf + sizeof(__le16), recv_len);
 
-	if (report_id && recv_len != 0 && recv_buf[0] != report_id) {
-		dev_err(&ihid->client->dev,
-			"device returned incorrect report (%d vs %d expected)\n",
-			recv_buf[0], report_id);
-		return -EINVAL;
-	}
-
 	return recv_len;
 }
 

-- 
2.56.0.385.gd3acb90ef8-goog

[PATCH 6/9] HID: goodix-spi: Drop redundant report ID validation

[Dmitry Torokhov]

Now that the core HID request handling validates returned report IDs
for report retrieval requests centrally, the report ID check in the
Goodix SPI transport driver is redundant. Drop it.

Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/hid/hid-goodix-spi.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/hid/hid-goodix-spi.c b/drivers/hid/hid-goodix-spi.c
index 03d549efbdce..81603581ae40 100644
--- a/drivers/hid/hid-goodix-spi.c
+++ b/drivers/hid/hid-goodix-spi.c
@@ -461,11 +461,6 @@ static int goodix_hid_get_raw_report(struct hid_device *hid,
 		return error;
 	}
 
-	if (buf[0] != reportnum) {
-		dev_err(ts->dev, "incorrect report (%d vs %d expected)",
-			buf[0], reportnum);
-		return -EINVAL;
-	}
 	return len;
 }
 

-- 
2.56.0.385.gd3acb90ef8-goog

[PATCH 7/9] HID: intel-thc-hid: Drop redundant report ID validation in quicki2c

[Dmitry Torokhov]

Now that the core HID request handling validates returned report IDs
for report retrieval requests centrally, the report ID check in the
Intel QuickI2C driver is redundant. Drop it while retaining packet
length validation.

Assisted-by: LLM
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-protocol.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-protocol.c b/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-protocol.c
index 41271301215a..7a362a2ebe2b 100644
--- a/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-protocol.c
+++ b/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-protocol.c
@@ -160,9 +160,9 @@ int quicki2c_get_report(struct quicki2c_device *qcdev, u8 report_type,
 		return ret;
 	}
 
-	if (HIDI2C_DATA_LEN(le16_to_cpu(rpt->len)) != buf_len || rpt->data[0] != reportnum) {
-		dev_err_once(qcdev->dev, "Invalid packet, len (%d vs %zu) report id (%d vs %d)\n",
-			     le16_to_cpu(rpt->len), buf_len, rpt->data[0], reportnum);
+	if (HIDI2C_DATA_LEN(le16_to_cpu(rpt->len)) != buf_len) {
+		dev_err_once(qcdev->dev, "Invalid packet length (%d vs %zu)\n",
+			     le16_to_cpu(rpt->len), buf_len);
 		return -EINVAL;
 	}
 

-- 
2.56.0.385.gd3acb90ef8-goog