[PATCH] HID: validate report length and constants

[PATCH] HID: validate report length and constants

[Davide Beatrici]

 From 4956d80ba2bcdb0f5410dfe9365331bcece781cb Mon Sep 17 00:00:00 2001
 From: Davide Beatrici <git@davidebeatrici.dev>
Date: Tue, 2 Dec 2025 05:00:52 +0100
Subject: [PATCH] HID: validate report length and constants
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The HID specification defines Input(Constant) fields as padding or 
reserved
bits that must remain zero at runtime. Currently, Linux accepts non‑zero
values in these fields, allowing malformed reports to propagate to 
userspace.

The ATK X1 SE (373b:1107) is a popular USB mouse that triggers weird 
behavior.
A few seconds after connecting it sends a packet that is detected as 
malformed
by WireShark, but the kernel happily accepts and parses it.

Until the device is disconnected, modifier keys are forced into a fixed 
state:

LeftControl (0xe0): UP
LeftShift (0xe1): UP
LeftAlt (0xe2): DOWN
LeftGUI (0xe3): UP
RightControl (0xe4): UP
RightShift (0xe5): DOWN
RightAlt (0xe6): DOWN
RightGUI (0xe7): UP

And unknown keys are kept pressed:

kernel: usb 5-1: Unknown key (scancode 0xb2) pressed.
kernel: usb 5-1: Unknown key (scancode 0xff) pressed.
kernel: usb 5-1: Unknown key (scancode 0xff) pressed.

kernel: usb 5-1: Unknown key (scancode 0xc0) pressed.
kernel: usb 5-1: Unknown key (scancode 0xb6) pressed.
kernel: usb 5-1: Unknown key (scancode 0xff) pressed.
kernel: usb 5-1: Unknown key (scancode 0xff) pressed.

This patch extends hid-core to record Constant slices during descriptor
parsing and validate them at runtime. Reports with non‑zero Constant 
bits are
rejected, and a ratelimited warning is logged. Legitimate Data/Relative 
fields
(buttons, motion axes, wheel) continue to pass through unchanged.

Malformed reports are suppressed and no longer show up with 
usbmon/WireShark.
All other USB devices I own still work flawlessly after applying this 
patch,
but this is definitely something that requires extensive testing!

Signed-off-by: Davide Beatrici <git@davidebeatrici.dev>
---
  drivers/hid/hid-core.c | 134 ++++++++++++++++++++++++++++++++++++++++-
  include/linux/hid.h    |  15 +++++
  2 files changed, 146 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index a5b3a8ca2fcb..bb45c5f6f4fb 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -280,6 +280,81 @@ static int hid_add_usage(struct hid_parser *parser, 
unsigned usage, u8 size)
  	return 0;
  }

+/*
+ * Append metadata for runtime payload validation.
+ */
+
+static int hid_validate_append_bits(struct hid_report *r, const __u16 
bit_off, const __u16 bit_len)
+{
+	struct hid_report_validate *v = &r->validate;
+	struct hid_const_slice *slices = krealloc(v->const_slices,
+		(v->const_count + 1) * sizeof(*slices), GFP_KERNEL);
+	if (!slices)
+		return -ENOMEM;
+
+	v->const_slices = slices;
+	v->const_slices[v->const_count].bit_off = bit_off;
+	v->const_slices[v->const_count].bit_len = bit_len;
+	v->const_count++;
+
+	return 0;
+}
+
+/*
+ * Validate runtime payload.
+ */
+
+static bool hid_validate_report(struct hid_device *hid, struct 
hid_report *r,
+		const __u8 *buf, size_t len)
+{
+	const __u8 *payload;
+	size_t payload_len;
+	__u16 i;
+
+	/* Report ID handling: if present, buf[0] is ID; payload follows */
+	payload = r->id ? buf + 1 : buf;
+	payload_len = r->id ? (len ? len - 1 : 0) : len;
+
+	if (r->validate.payload_len && payload_len != r->validate.payload_len) 
{
+		hid_warn_ratelimited(hid,
+			"Malformed report: length %zu != expected %u (ID %u)\n",
+			payload_len, r->validate.payload_len, r->id);
+		return false;
+	}
+
+	for (i = 0; i < r->validate.const_count; i++) {
+		const __u16 bit_off = r->validate.const_slices[i].bit_off;
+		const __u16 bit_len = r->validate.const_slices[i].bit_len;
+		const __u16 end_bit = bit_off + bit_len;
+
+		for (__u16 b = bit_off; b < end_bit;) {
+			size_t byte_off = b >> 3;
+			size_t bit_in_byte = b & 7;
+
+			__u16 rem_bits = end_bit - b;
+			__u8 span = (__u8)min_t(__u16, rem_bits, 8 - bit_in_byte);
+			__u8 mask = ((1u << span) - 1) << bit_in_byte;
+
+			if (byte_off >= payload_len) {
+				hid_warn_ratelimited(hid,
+					"Malformed report: const slice OOB (bit_off %u, len %u)\n",
+					bit_off, bit_len);
+				return false;
+			}
+			if (payload[byte_off] & mask) {
+				hid_warn_ratelimited(hid,
+					"Malformed report: non-zero constant at byte %zu mask 0x%02x val 
0x%02x\n",
+					byte_off, mask, payload[byte_off]);
+				return false;
+			}
+
+			b += span;
+		}
+	}
+
+	return true;
+}
+
  /*
   * Register a new field for this report.
   */
@@ -303,6 +378,8 @@ static int hid_add_field(struct hid_parser *parser, 
unsigned report_type, unsign
  		return -1;
  	}

+	parser->curr_report = report;
+
  	/* Handle both signed and unsigned cases properly */
  	if ((parser->global.logical_minimum < 0 &&
  		parser->global.logical_maximum <
@@ -638,11 +715,13 @@ static void hid_concatenate_last_usage_page(struct 
hid_parser *parser)
  static int hid_parser_main(struct hid_parser *parser, struct hid_item 
*item)
  {
  	__u32 data;
+	__u8 flags;
  	int ret;

  	hid_concatenate_last_usage_page(parser);

  	data = item_udata(item);
+	flags = (u8)data;

  	switch (item->tag) {
  	case HID_MAIN_ITEM_TAG_BEGIN_COLLECTION:
@@ -651,15 +730,61 @@ static int hid_parser_main(struct hid_parser 
*parser, struct hid_item *item)
  	case HID_MAIN_ITEM_TAG_END_COLLECTION:
  		ret = close_collection(parser);
  		break;
-	case HID_MAIN_ITEM_TAG_INPUT:
+	case HID_MAIN_ITEM_TAG_INPUT: {
+		__u16 offset_bits, size_bits;
+
+		if (flags & HID_MAIN_ITEM_RESERVED_MASK) {
+			hid_warn_ratelimited(parser->device,
+				"Malformed input descriptor: reserved bits set (0x%02x)\n",
+				flags);
+			return -EINVAL;
+		}
+
+		/* Compute field range in bits */
+		offset_bits = parser->curr_offset;
+		size_bits   = parser->global.report_size * 
parser->global.report_count;
+
+		/* Record Input(Constant) slices for runtime validation */
+		if ((flags & HID_MAIN_ITEM_CONSTANT) && parser->curr_report) {
+			/* Record bit-granular slice: store bit offset and size */
+			ret = hid_validate_append_bits(parser->curr_report, offset_bits, 
size_bits);
+			if (ret)
+				return ret;
+		}
+
+		/* Advance offset and add field */
+		parser->curr_offset += size_bits;
+
  		ret = hid_add_field(parser, HID_INPUT_REPORT, data);
+		if (!ret && parser->curr_report) {
+			/* Expected payload length (bytes) excluding the optional ID */
+			parser->curr_report->validate.payload_len = 
(parser->curr_report->size + 7) / 8;
+		}
+
  		break;
-	case HID_MAIN_ITEM_TAG_OUTPUT:
+	}
+	case HID_MAIN_ITEM_TAG_OUTPUT: {
+		if (flags & HID_MAIN_ITEM_RESERVED_MASK) {
+			hid_warn_ratelimited(parser->device,
+				"Malformed output descriptor: reserved bits set (0x%02x)\n",
+				flags);
+			return -EINVAL;
+		}
+
  		ret = hid_add_field(parser, HID_OUTPUT_REPORT, data);
  		break;
-	case HID_MAIN_ITEM_TAG_FEATURE:
+	}
+	case HID_MAIN_ITEM_TAG_FEATURE: {
+		if (flags & HID_MAIN_ITEM_RESERVED_MASK) {
+			hid_warn_ratelimited(parser->device,
+				"Malformed feature descriptor: reserved bits set (0x%02x)\n",
+				flags);
+			return -EINVAL;
+		}
+
  		ret = hid_add_field(parser, HID_FEATURE_REPORT, data);
  		break;
+	}
  	default:
  		if (item->tag >= HID_MAIN_ITEM_TAG_RESERVED_MIN &&
  			item->tag <= HID_MAIN_ITEM_TAG_RESERVED_MAX)
@@ -2062,6 +2187,9 @@ int hid_report_raw_event(struct hid_device *hid, 
enum hid_report_type type, u8 *
  		memset(cdata + csize, 0, rsize - csize);
  	}

+	if (!hid_validate_report(hid, report, data, size))
+		goto out;
+
  	if ((hid->claimed & HID_CLAIMED_HIDDEV) && hid->hiddev_report_event)
  		hid->hiddev_report_event(hid, report);
  	if (hid->claimed & HID_CLAIMED_HIDRAW) {
diff --git a/include/linux/hid.h b/include/linux/hid.h
index a4ddb94e3ee5..3c7b3a8faa48 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -88,6 +88,7 @@ struct hid_item {
   * HID report descriptor main item contents
   */

+#define HID_MAIN_ITEM_RESERVED_MASK	0x0c8 /* HID 1.11: flags reserved 
bits (3, 6, 7) must be zero */
  #define HID_MAIN_ITEM_CONSTANT		0x001
  #define HID_MAIN_ITEM_VARIABLE		0x002
  #define HID_MAIN_ITEM_RELATIVE		0x004
@@ -560,6 +561,17 @@ struct hid_field_entry {
  	__s32 priority;
  };

+struct hid_const_slice {
+	__u16 bit_off;
+	__u16 bit_len;
+};
+
+struct hid_report_validate {
+	struct hid_const_slice *const_slices;
+	__u16 const_count;
+	__u16 payload_len;
+};
+
  struct hid_report {
  	struct list_head list;
  	struct list_head hidinput_list;
@@ -576,6 +588,7 @@ struct hid_report {
  	/* tool related state */
  	bool tool_active;				/* whether the current tool is active */
  	unsigned int tool;				/* BTN_TOOL_* */
+	struct hid_report_validate validate;
  };

  #define HID_MAX_IDS 256
@@ -760,6 +773,8 @@ struct hid_parser {
  	unsigned int          collection_stack_size;
  	struct hid_device    *device;
  	unsigned int          scan_flags;
+	__u16                 curr_offset;
+	struct hid_report    *curr_report;
  };

  struct hid_class_descriptor {
-- 
2.51.2

Re: [PATCH] HID: validate report length and constants

[Benjamin Tissoires]

On Dec 02 2025, Davide Beatrici wrote:
> From 4956d80ba2bcdb0f5410dfe9365331bcece781cb Mon Sep 17 00:00:00 2001
> From: Davide Beatrici <git@davidebeatrici.dev>
> Date: Tue, 2 Dec 2025 05:00:52 +0100
> Subject: [PATCH] HID: validate report length and constants
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> The HID specification defines Input(Constant) fields as padding or reserved
> bits that must remain zero at runtime. Currently, Linux accepts non‑zero
> values in these fields, allowing malformed reports to propagate to
> userspace.

Yep, this is on purpose because Miscrosoft's own driver works that way
and many HID devices do not bother to mark the non constant bits as
data. So if you enforce the spec here, you'll break a numerous of
devices unfortunatelly.

> 
> The ATK X1 SE (373b:1107) is a popular USB mouse that triggers weird
> behavior.
> A few seconds after connecting it sends a packet that is detected as
> malformed
> by WireShark, but the kernel happily accepts and parses it.

Can you show us what packets are emitted?

If it's a firmware bug, we better have a specific driver for it could
be a HID-BPF program that just filters out the unwanted reports.

Also, how does Windows behave with this mouse? Does it need a special
driver?

> 
> Until the device is disconnected, modifier keys are forced into a fixed
> state:
> 
> LeftControl (0xe0): UP
> LeftShift (0xe1): UP
> LeftAlt (0xe2): DOWN
> LeftGUI (0xe3): UP
> RightControl (0xe4): UP
> RightShift (0xe5): DOWN
> RightAlt (0xe6): DOWN
> RightGUI (0xe7): UP
> 
> And unknown keys are kept pressed:
> 
> kernel: usb 5-1: Unknown key (scancode 0xb2) pressed.
> kernel: usb 5-1: Unknown key (scancode 0xff) pressed.
> kernel: usb 5-1: Unknown key (scancode 0xff) pressed.
> 
> kernel: usb 5-1: Unknown key (scancode 0xc0) pressed.
> kernel: usb 5-1: Unknown key (scancode 0xb6) pressed.
> kernel: usb 5-1: Unknown key (scancode 0xff) pressed.
> kernel: usb 5-1: Unknown key (scancode 0xff) pressed.

Looks like there is something wrong either in the report descriptor of
this mouse, either in the emitted reports.

> 
> This patch extends hid-core to record Constant slices during descriptor
> parsing and validate them at runtime. Reports with non‑zero Constant bits
> are
> rejected, and a ratelimited warning is logged. Legitimate Data/Relative
> fields
> (buttons, motion axes, wheel) continue to pass through unchanged.

Ouch. If I read you correctly, you are rejecting the entire report if a
constant field is not 0. It is common for constant fields to be just
garbage (whatever is in the memory, because writing firmware is hard),
so even if we were to accept this patch, this would break even more
devices :(

> 
> Malformed reports are suppressed and no longer show up with
> usbmon/WireShark.
> All other USB devices I own still work flawlessly after applying this patch,
> but this is definitely something that requires extensive testing!

I am pretty sure the HID selftests will fail with this patch applied,
because there are probably a couple of devices there with the "non
constant" behaviour.

Cheers,
Benjamin

> 
> Signed-off-by: Davide Beatrici <git@davidebeatrici.dev>
> ---
>  drivers/hid/hid-core.c | 134 ++++++++++++++++++++++++++++++++++++++++-
>  include/linux/hid.h    |  15 +++++
>  2 files changed, 146 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index a5b3a8ca2fcb..bb45c5f6f4fb 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -280,6 +280,81 @@ static int hid_add_usage(struct hid_parser *parser,
> unsigned usage, u8 size)
>  	return 0;
>  }
> 
> +/*
> + * Append metadata for runtime payload validation.
> + */
> +
> +static int hid_validate_append_bits(struct hid_report *r, const __u16
> bit_off, const __u16 bit_len)
> +{
> +	struct hid_report_validate *v = &r->validate;
> +	struct hid_const_slice *slices = krealloc(v->const_slices,
> +		(v->const_count + 1) * sizeof(*slices), GFP_KERNEL);
> +	if (!slices)
> +		return -ENOMEM;
> +
> +	v->const_slices = slices;
> +	v->const_slices[v->const_count].bit_off = bit_off;
> +	v->const_slices[v->const_count].bit_len = bit_len;
> +	v->const_count++;
> +
> +	return 0;
> +}
> +
> +/*
> + * Validate runtime payload.
> + */
> +
> +static bool hid_validate_report(struct hid_device *hid, struct hid_report
> *r,
> +		const __u8 *buf, size_t len)
> +{
> +	const __u8 *payload;
> +	size_t payload_len;
> +	__u16 i;
> +
> +	/* Report ID handling: if present, buf[0] is ID; payload follows */
> +	payload = r->id ? buf + 1 : buf;
> +	payload_len = r->id ? (len ? len - 1 : 0) : len;
> +
> +	if (r->validate.payload_len && payload_len != r->validate.payload_len) {
> +		hid_warn_ratelimited(hid,
> +			"Malformed report: length %zu != expected %u (ID %u)\n",
> +			payload_len, r->validate.payload_len, r->id);
> +		return false;
> +	}
> +
> +	for (i = 0; i < r->validate.const_count; i++) {
> +		const __u16 bit_off = r->validate.const_slices[i].bit_off;
> +		const __u16 bit_len = r->validate.const_slices[i].bit_len;
> +		const __u16 end_bit = bit_off + bit_len;
> +
> +		for (__u16 b = bit_off; b < end_bit;) {
> +			size_t byte_off = b >> 3;
> +			size_t bit_in_byte = b & 7;
> +
> +			__u16 rem_bits = end_bit - b;
> +			__u8 span = (__u8)min_t(__u16, rem_bits, 8 - bit_in_byte);
> +			__u8 mask = ((1u << span) - 1) << bit_in_byte;
> +
> +			if (byte_off >= payload_len) {
> +				hid_warn_ratelimited(hid,
> +					"Malformed report: const slice OOB (bit_off %u, len %u)\n",
> +					bit_off, bit_len);
> +				return false;
> +			}
> +			if (payload[byte_off] & mask) {
> +				hid_warn_ratelimited(hid,
> +					"Malformed report: non-zero constant at byte %zu mask 0x%02x val
> 0x%02x\n",
> +					byte_off, mask, payload[byte_off]);
> +				return false;
> +			}
> +
> +			b += span;
> +		}
> +	}
> +
> +	return true;
> +}
> +
>  /*
>   * Register a new field for this report.
>   */
> @@ -303,6 +378,8 @@ static int hid_add_field(struct hid_parser *parser,
> unsigned report_type, unsign
>  		return -1;
>  	}
> 
> +	parser->curr_report = report;
> +
>  	/* Handle both signed and unsigned cases properly */
>  	if ((parser->global.logical_minimum < 0 &&
>  		parser->global.logical_maximum <
> @@ -638,11 +715,13 @@ static void hid_concatenate_last_usage_page(struct
> hid_parser *parser)
>  static int hid_parser_main(struct hid_parser *parser, struct hid_item
> *item)
>  {
>  	__u32 data;
> +	__u8 flags;
>  	int ret;
> 
>  	hid_concatenate_last_usage_page(parser);
> 
>  	data = item_udata(item);
> +	flags = (u8)data;
> 
>  	switch (item->tag) {
>  	case HID_MAIN_ITEM_TAG_BEGIN_COLLECTION:
> @@ -651,15 +730,61 @@ static int hid_parser_main(struct hid_parser *parser,
> struct hid_item *item)
>  	case HID_MAIN_ITEM_TAG_END_COLLECTION:
>  		ret = close_collection(parser);
>  		break;
> -	case HID_MAIN_ITEM_TAG_INPUT:
> +	case HID_MAIN_ITEM_TAG_INPUT: {
> +		__u16 offset_bits, size_bits;
> +
> +		if (flags & HID_MAIN_ITEM_RESERVED_MASK) {
> +			hid_warn_ratelimited(parser->device,
> +				"Malformed input descriptor: reserved bits set (0x%02x)\n",
> +				flags);
> +			return -EINVAL;
> +		}
> +
> +		/* Compute field range in bits */
> +		offset_bits = parser->curr_offset;
> +		size_bits   = parser->global.report_size * parser->global.report_count;
> +
> +		/* Record Input(Constant) slices for runtime validation */
> +		if ((flags & HID_MAIN_ITEM_CONSTANT) && parser->curr_report) {
> +			/* Record bit-granular slice: store bit offset and size */
> +			ret = hid_validate_append_bits(parser->curr_report, offset_bits,
> size_bits);
> +			if (ret)
> +				return ret;
> +		}
> +
> +		/* Advance offset and add field */
> +		parser->curr_offset += size_bits;
> +
>  		ret = hid_add_field(parser, HID_INPUT_REPORT, data);
> +		if (!ret && parser->curr_report) {
> +			/* Expected payload length (bytes) excluding the optional ID */
> +			parser->curr_report->validate.payload_len = (parser->curr_report->size +
> 7) / 8;
> +		}
> +
>  		break;
> -	case HID_MAIN_ITEM_TAG_OUTPUT:
> +	}
> +	case HID_MAIN_ITEM_TAG_OUTPUT: {
> +		if (flags & HID_MAIN_ITEM_RESERVED_MASK) {
> +			hid_warn_ratelimited(parser->device,
> +				"Malformed output descriptor: reserved bits set (0x%02x)\n",
> +				flags);
> +			return -EINVAL;
> +		}
> +
>  		ret = hid_add_field(parser, HID_OUTPUT_REPORT, data);
>  		break;
> -	case HID_MAIN_ITEM_TAG_FEATURE:
> +	}
> +	case HID_MAIN_ITEM_TAG_FEATURE: {
> +		if (flags & HID_MAIN_ITEM_RESERVED_MASK) {
> +			hid_warn_ratelimited(parser->device,
> +				"Malformed feature descriptor: reserved bits set (0x%02x)\n",
> +				flags);
> +			return -EINVAL;
> +		}
> +
>  		ret = hid_add_field(parser, HID_FEATURE_REPORT, data);
>  		break;
> +	}
>  	default:
>  		if (item->tag >= HID_MAIN_ITEM_TAG_RESERVED_MIN &&
>  			item->tag <= HID_MAIN_ITEM_TAG_RESERVED_MAX)
> @@ -2062,6 +2187,9 @@ int hid_report_raw_event(struct hid_device *hid, enum
> hid_report_type type, u8 *
>  		memset(cdata + csize, 0, rsize - csize);
>  	}
> 
> +	if (!hid_validate_report(hid, report, data, size))
> +		goto out;
> +
>  	if ((hid->claimed & HID_CLAIMED_HIDDEV) && hid->hiddev_report_event)
>  		hid->hiddev_report_event(hid, report);
>  	if (hid->claimed & HID_CLAIMED_HIDRAW) {
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index a4ddb94e3ee5..3c7b3a8faa48 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -88,6 +88,7 @@ struct hid_item {
>   * HID report descriptor main item contents
>   */
> 
> +#define HID_MAIN_ITEM_RESERVED_MASK	0x0c8 /* HID 1.11: flags reserved bits
> (3, 6, 7) must be zero */
>  #define HID_MAIN_ITEM_CONSTANT		0x001
>  #define HID_MAIN_ITEM_VARIABLE		0x002
>  #define HID_MAIN_ITEM_RELATIVE		0x004
> @@ -560,6 +561,17 @@ struct hid_field_entry {
>  	__s32 priority;
>  };
> 
> +struct hid_const_slice {
> +	__u16 bit_off;
> +	__u16 bit_len;
> +};
> +
> +struct hid_report_validate {
> +	struct hid_const_slice *const_slices;
> +	__u16 const_count;
> +	__u16 payload_len;
> +};
> +
>  struct hid_report {
>  	struct list_head list;
>  	struct list_head hidinput_list;
> @@ -576,6 +588,7 @@ struct hid_report {
>  	/* tool related state */
>  	bool tool_active;				/* whether the current tool is active */
>  	unsigned int tool;				/* BTN_TOOL_* */
> +	struct hid_report_validate validate;
>  };
> 
>  #define HID_MAX_IDS 256
> @@ -760,6 +773,8 @@ struct hid_parser {
>  	unsigned int          collection_stack_size;
>  	struct hid_device    *device;
>  	unsigned int          scan_flags;
> +	__u16                 curr_offset;
> +	struct hid_report    *curr_report;
>  };
> 
>  struct hid_class_descriptor {
> -- 
> 2.51.2
> 

Re: [PATCH] HID: validate report length and constants

[Davide Beatrici]

Thank you very much for your feedback!

I can send you a new identical device for testing if you would like.

> Can you show us what packets are emitted?
> 
> If it's a firmware bug, we better have a specific driver for it could
> be a HID-BPF program that just filters out the unwanted reports.
> 
> Also, how does Windows behave with this mouse? Does it need a special
> driver?

Sorry, I should've mentioned the malformed packet also shows up on 
Windows,
but is seemingly ignored because there appear to be no side effects 
whatsoever.

No special driver needed, it's detected as a standard HID mouse.

WireShark capture:
https://dl.houseof.software/misc/atk_x1_se_malformed_packet.pcapng

Packet screenshot:
https://dl.houseof.software/misc/atk_x1_se_malformed_packet.png

> Looks like there is something wrong either in the report descriptor of
> this mouse, either in the emitted reports.

Definitely. I have already informed the manufacturer, who confirmed the 
mouse
has only been tested on Windows.

My inquiry was forwarded to their R&D team, hopefully a firmware update 
will
be released soon.

> Yep, this is on purpose because Miscrosoft's own driver works that way
> and many HID devices do not bother to mark the non constant bits as
> data. So if you enforce the spec here, you'll break a numerous of
> devices unfortunatelly.

> Ouch. If I read you correctly, you are rejecting the entire report if a
> constant field is not 0. It is common for constant fields to be just
> garbage (whatever is in the memory, because writing firmware is hard),
> so even if we were to accept this patch, this would break even more
> devices :(

> I am pretty sure the HID selftests will fail with this patch applied,
> because there are probably a couple of devices there with the "non
> constant" behaviour.

Oh, in that case let's just drop that part from the patch, since it's
actually not altering the behavior with this specific device.

The malformed packet is detected and rejected by two checks:

Malformed report: raw_len=1 payload_len=1 expected=8 (ID 0)
Malformed report: const slice OOB (bit_off 8, len 8)

Re: [PATCH] HID: validate report length and constants

[Terry Junge]

On 12/2/25 11:41 AM, Davide Beatrici wrote:
> Thank you very much for your feedback!
> 
> I can send you a new identical device for testing if you would like.
> 

Can you supply the Device, Configuration, and Report Descriptors?

Thanks,
Terry

>> Can you show us what packets are emitted?
>>
>> If it's a firmware bug, we better have a specific driver for it could
>> be a HID-BPF program that just filters out the unwanted reports.
>>
>> Also, how does Windows behave with this mouse? Does it need a special
>> driver?
> 
> Sorry, I should've mentioned the malformed packet also shows up on Windows,
> but is seemingly ignored because there appear to be no side effects whatsoever.
> 
> No special driver needed, it's detected as a standard HID mouse.
> 
> WireShark capture:
> https://dl.houseof.software/misc/atk_x1_se_malformed_packet.pcapng
> 
> Packet screenshot:
> https://dl.houseof.software/misc/atk_x1_se_malformed_packet.png
> 
>> Looks like there is something wrong either in the report descriptor of
>> this mouse, either in the emitted reports.
> 
> Definitely. I have already informed the manufacturer, who confirmed the mouse
> has only been tested on Windows.
> 
> My inquiry was forwarded to their R&D team, hopefully a firmware update will
> be released soon.
> 
>> Yep, this is on purpose because Miscrosoft's own driver works that way
>> and many HID devices do not bother to mark the non constant bits as
>> data. So if you enforce the spec here, you'll break a numerous of
>> devices unfortunatelly.
> 
>> Ouch. If I read you correctly, you are rejecting the entire report if a
>> constant field is not 0. It is common for constant fields to be just
>> garbage (whatever is in the memory, because writing firmware is hard),
>> so even if we were to accept this patch, this would break even more
>> devices :(
> 
>> I am pretty sure the HID selftests will fail with this patch applied,
>> because there are probably a couple of devices there with the "non
>> constant" behaviour.
> 
> Oh, in that case let's just drop that part from the patch, since it's
> actually not altering the behavior with this specific device.
> 
> The malformed packet is detected and rejected by two checks:
> 
> Malformed report: raw_len=1 payload_len=1 expected=8 (ID 0)
> Malformed report: const slice OOB (bit_off 8, len 8)
> 

Re: [PATCH] HID: validate report length and constants

[Davide Beatrici]

> Can you supply the Device, Configuration, and Report Descriptors?

Sure.

Device Descriptor:
   idVendor           0x373b Compx
   idProduct          0x1107 ATK X1 SE Nearlink
   bcdDevice          1.21
   bcdUSB             2.00
   bMaxPacketSize0    64
   iManufacturer      1 Compx
   iProduct           2 ATK X1 SE Nearlink
   bNumConfigurations 1

Configuration Descriptor:
   wTotalLength       0x0054
   bNumInterfaces     3
   bmAttributes       0xa0 (Bus Powered, Remote Wakeup)
   MaxPower           494mA

Interface 0: HID Keyboard
   HID Descriptor: wDescriptorLength 77
   Endpoint IN 0x81, Interrupt, 64 bytes

Interface 1: HID (non‑boot)
   HID Descriptor: wDescriptorLength 156
   Endpoint IN 0x82, Interrupt, 64 bytes

Interface 2: HID Mouse
   HID Descriptor: wDescriptorLength 87
   Endpoint IN 0x83, Interrupt, 64 bytes

Report Descriptors:

Interface 2 (Mouse):
   05 01 09 02 A1 01 09 01 A1 00 05 09 19 01 29 05
   15 00 25 01 95 05 75 01 81 02 95 01 75 03 81 01
   05 01 09 30 09 31 16 00 80 26 FF 7F 75 10 95 02
   81 06 C0 A1 00 05 01 09 38 15 81 25 7F 75 08 95
   01 81 06 C0 A1 00 05 0C 0A 38 02 95 01 75 08 15
   81 25 7F 81 06 C0 C0

Interface 1 (HID composite):
   05 0C 09 01 A1 01 85 05 15 00 26 14 05 19 00 2A
   14 05 75 10 95 01 81 00 C0 05 01 09 80 A1 01 85
   03 19 81 29 83 15 00 25 01 95 03 75 01 81 02 95
   01 75 05 81 01 C0 05 01 09 06 A1 01 85 04 05 07
   15 00 25 01 19 00 29 9F 95 A0 75 01 81 02 C0 06
   02 FF 09 02 A1 01 85 13 15 00 26 FF 00 75 08 95
   13 09 02 81 00 09 02 91 00 C0 06 02 FF 09 02 A1
   01 85 08 15 00 26 FF 00 75 08 95 10 09 02 81 00
   09 02 91 00 C0 06 04 FF 09 02 A1 01 85 06 09 02
   15 00 26 FF 00 75 08 95 07 B1 02 C0

Interface 0 (Keyboard):
   05 01 09 06 A1 01 05 08 19 01 29 03 15 00 25 01
   75 01 95 03 91 02 95 05 91 01 05 07 19 E0 29 E7
   15 00 25 01 75 01 95 08 81 02 75 08 95 01 81 01
   05 07 19 00 2A FF 00 15 00 26 FF 00 75 08 95 05
   81 00 05 FF 09 03 75 08 95 01 81 02 C0

Re: [PATCH] HID: validate report length and constants

[Benjamin Tissoires]

On Dec 02 2025, Davide Beatrici wrote:
> > Can you supply the Device, Configuration, and Report Descriptors?
> 
> Sure.
> 
> Device Descriptor:
>   idVendor           0x373b Compx
>   idProduct          0x1107 ATK X1 SE Nearlink
>   bcdDevice          1.21
>   bcdUSB             2.00
>   bMaxPacketSize0    64
>   iManufacturer      1 Compx
>   iProduct           2 ATK X1 SE Nearlink
>   bNumConfigurations 1
> 
> Configuration Descriptor:
>   wTotalLength       0x0054
>   bNumInterfaces     3
>   bmAttributes       0xa0 (Bus Powered, Remote Wakeup)
>   MaxPower           494mA
> 
> Interface 0: HID Keyboard
>   HID Descriptor: wDescriptorLength 77
>   Endpoint IN 0x81, Interrupt, 64 bytes
> 
> Interface 1: HID (non‑boot)
>   HID Descriptor: wDescriptorLength 156
>   Endpoint IN 0x82, Interrupt, 64 bytes
> 
> Interface 2: HID Mouse
>   HID Descriptor: wDescriptorLength 87
>   Endpoint IN 0x83, Interrupt, 64 bytes
> 
> Report Descriptors:
> 
> Interface 2 (Mouse):
>   05 01 09 02 A1 01 09 01 A1 00 05 09 19 01 29 05
>   15 00 25 01 95 05 75 01 81 02 95 01 75 03 81 01
>   05 01 09 30 09 31 16 00 80 26 FF 7F 75 10 95 02
>   81 06 C0 A1 00 05 01 09 38 15 81 25 7F 75 08 95
>   01 81 06 C0 A1 00 05 0C 0A 38 02 95 01 75 08 15
>   81 25 7F 81 06 C0 C0
> 
> Interface 1 (HID composite):
>   05 0C 09 01 A1 01 85 05 15 00 26 14 05 19 00 2A
>   14 05 75 10 95 01 81 00 C0 05 01 09 80 A1 01 85
>   03 19 81 29 83 15 00 25 01 95 03 75 01 81 02 95
>   01 75 05 81 01 C0 05 01 09 06 A1 01 85 04 05 07
>   15 00 25 01 19 00 29 9F 95 A0 75 01 81 02 C0 06
>   02 FF 09 02 A1 01 85 13 15 00 26 FF 00 75 08 95
>   13 09 02 81 00 09 02 91 00 C0 06 02 FF 09 02 A1
>   01 85 08 15 00 26 FF 00 75 08 95 10 09 02 81 00
>   09 02 91 00 C0 06 04 FF 09 02 A1 01 85 06 09 02
>   15 00 26 FF 00 75 08 95 07 B1 02 C0
> 
> Interface 0 (Keyboard):
>   05 01 09 06 A1 01 05 08 19 01 29 03 15 00 25 01
>   75 01 95 03 91 02 95 05 91 01 05 07 19 E0 29 E7
>   15 00 25 01 75 01 95 08 81 02 75 08 95 01 81 01
>   05 07 19 00 2A FF 00 15 00 26 FF 00 75 08 95 05
>   81 00 05 FF 09 03 75 08 95 01 81 02 C0

Thanks for the logs.

So after analysing the wireshark capture and these, the problem is that
the device sends a USB report of length 1 on the keyboard interface when
we should actually get one of size 8.

However, the device also shows an output report of size 1, but it is not
supposed to send it as an input report. I wonder if the firmware bug is
not that it tries to give the host the current state of its output
report at plug (which is wrong but Windows must be papering over it).

Anyway couple of observations:
- the URB is of size 1, so the fact that the constant field is not 0
	means that we are just reading random memory at offset 1 in the
	provided data, so you might have a chance that it eventually becomes 0

- the fix should be focusing on the length of the provided report, not
	on the content. However, in hid_report_raw_event(), just before you
	inserted your call to your hid_validate_report(), there is already a
	check on the length of the report which memsets to 0 the rest of the
	buffer. This seems a little bit optimistic if the provided buffer from
	USB is exactly the size of the provided "size" argument.
	But then, why would you get random data in the const fields if there
	is a memset if the provided length is "1"?

So, can you add a printk before your call to hid_validate_report() to
show the provided "size" argument (csize), or just enable the hid_dbg()
trace output which should tell us if we enter that test and do the
memset (which I suppose we are not).

Cheers,
Benjamin

Re: [PATCH] HID: validate report length and constants

[Davide Beatrici]

> However, the device also shows an output report of size 1, but it is 
> not
> supposed to send it as an input report. I wonder if the firmware bug is
> not that it tries to give the host the current state of its output
> report at plug (which is wrong but Windows must be papering over it).

On that note, I noticed the malformed packet is not sent upon 
reconnecting the
device if it's been plugged in for some time.
When that happens I can reproduce the issue by enabling a wireless mode 
through
the switch on the bottom and wait a bit before disabling it and 
connecting the
device again.
I suspect the battery gets slightly discharged and the device sends a
"charging complete" signal or something.

> - the URB is of size 1, so the fact that the constant field is not 0
> means that we are just reading random memory at offset 1 in the
> provided data, so you might have a chance that it eventually becomes 0

Good point, if it's effectively random memory we cannot rely on that.

> - the fix should be focusing on the length of the provided report, not
> on the content. However, in hid_report_raw_event(), just before you
> inserted your call to your hid_validate_report(), there is already a
> check on the length of the report which memsets to 0 the rest of the
> buffer. This seems a little bit optimistic if the provided buffer from
> USB is exactly the size of the provided "size" argument.
> But then, why would you get random data in the const fields if there
> is a memset if the provided length is "1"?
> 
> So, can you add a printk before your call to hid_validate_report() to
> show the provided "size" argument (csize), or just enable the hid_dbg()
> trace output which should tell us if we enter that test and do the
> memset (which I suppose we are not).

report 8 has csize=16 rsize=16
report 0 has csize=1 rsize=8
report 0 is too short, (1 < 8)

Which means we do enter the test and execute the memset()...

Re: [PATCH] HID: validate report length and constants

[Davide Beatrici]

> report 8 has csize=16 rsize=16
> report 0 has csize=1 rsize=8
> report 0 is too short, (1 < 8)
> 
> Which means we do enter the test and execute the memset()...

I added further debug prints to trace the flow after that:

hid-generic 0003:373B:1107.000F: report 8 has csize=16 rsize=16
hid-generic 0003:373B:1107.000F: Calling hiddev_report_event()
hid-generic 0003:373B:1107.000F: Calling hidraw_report_event()
hid-generic 0003:373B:1107.000F: Calling hid_process_report()
hid-generic 0003:373B:1107.000F: Calling hidinput_report_event()
hid-generic 0003:373B:1107.000E: report 0 has csize=1 rsize=8
hid-generic 0003:373B:1107.000E: report 0 is too short, (1 < 8)
hid-generic 0003:373B:1107.000E: Calling hidraw_report_event()
hid-generic 0003:373B:1107.000E: Calling hid_process_report()
hid-generic 0003:373B:1107.000E: Calling hidinput_report_event()
hid-generic 0003:373B:1107.0010: report 0 has csize=7 rsize=7
hid-generic 0003:373B:1107.0010: Calling hidraw_report_event()
hid-generic 0003:373B:1107.0010: Calling hid_process_report()
hid-generic 0003:373B:1107.0010: Calling hidinput_report_event()

The last report is a normal mouse movement.

Re: [PATCH] HID: validate report length and constants

[Benjamin Tissoires]

On Dec 05 2025, Davide Beatrici wrote:
> > report 8 has csize=16 rsize=16
> > report 0 has csize=1 rsize=8
> > report 0 is too short, (1 < 8)
> > 
> > Which means we do enter the test and execute the memset()...
> 
> I added further debug prints to trace the flow after that:
> 
> hid-generic 0003:373B:1107.000F: report 8 has csize=16 rsize=16
> hid-generic 0003:373B:1107.000F: Calling hiddev_report_event()
> hid-generic 0003:373B:1107.000F: Calling hidraw_report_event()
> hid-generic 0003:373B:1107.000F: Calling hid_process_report()
> hid-generic 0003:373B:1107.000F: Calling hidinput_report_event()
> hid-generic 0003:373B:1107.000E: report 0 has csize=1 rsize=8
> hid-generic 0003:373B:1107.000E: report 0 is too short, (1 < 8)
> hid-generic 0003:373B:1107.000E: Calling hidraw_report_event()
> hid-generic 0003:373B:1107.000E: Calling hid_process_report()
> hid-generic 0003:373B:1107.000E: Calling hidinput_report_event()
> hid-generic 0003:373B:1107.0010: report 0 has csize=7 rsize=7
> hid-generic 0003:373B:1107.0010: Calling hidraw_report_event()
> hid-generic 0003:373B:1107.0010: Calling hid_process_report()
> hid-generic 0003:373B:1107.0010: Calling hidinput_report_event()
> 
> The last report is a normal mouse movement.

Thanks for the logs.

So the most conservative change should be to either:
- have a HID-BPF program that strips out reports of size 1
- have a new kernel driver for this device which maps to .raw_event()
	and rejects reports of size 1.

AFAICT, all the transport drivers are allocating the buffer with enough
space, so the memset should be safe, meaning that we can not enforce
the size to be at least the report size without risking of breaking
devices as this code has been around for a while.

IMO, the simplest is the HID-BPF route, as it's a matter of going to the
udev-hid-bpf project [1], add your program in the testing dir, and
submit a merge request. This way your device will be fixed and I'll
eventually take care of putting the HID-BPF program in
drivers/hid/bpf/progs so it gets installed in all distributions.


Cheers,
Benjamin

[1] https://gitlab.freedesktop.org/libevdev/udev-hid-bpf

Re: [PATCH] HID: validate report length and constants

[Davide Beatrici]

> IMO, the simplest is the HID-BPF route, as it's a matter of going to 
> the
> udev-hid-bpf project [1], add your program in the testing dir, and
> submit a merge request. This way your device will be fixed and I'll
> eventually take care of putting the HID-BPF program in
> drivers/hid/bpf/progs so it gets installed in all distributions.

I apologize for the delay, I had to figure out why OpenMandriva's kernel 
was
not exposing the hid_bpf_ops structure.

Turns out CONFIG_HID_BPF had to be enabled and CONFIG_FUNCTION_TRACER 
too.

Here's the merge request for udev-hid-bpf:
https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/212

> have a new kernel driver for this device which maps to .raw_event()
> and rejects reports of size 1.

I actually just realized my previous mouse, a Kysona M600, has a 
specific
driver (hid-kysona) that seemingly handles battery status reports.

That may be the cause for it randomly stopping registering clicks and 
scrolls
until it is moved (i.e. registers a movement).

But that's a story for another thread!

I wonder if we can put together a single universal driver for these 
devices...