Linux-HyperV List
 help / color / mirror / Atom feed
* [PATCH 0/2] HID: hyperv: bound initial device info descriptor
@ 2026-07-10  2:28 Michael Bommarito
  2026-07-10  2:28 ` [PATCH 1/2] HID: hyperv: validate initial device info bounds Michael Bommarito
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Michael Bommarito @ 2026-07-10  2:28 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, kys, Haiyang Zhang, Wei Liu
  Cc: Dexuan Cui, Long Li, linux-input, linux-hyperv, linux-kernel,
	stable

A malicious Hyper-V host or backend can crash a guest with a short
SYNTH_HID_INITIAL_DEVICE_INFO message. mousevsc_on_receive_device_info()
trusts the HID descriptor bLength and wDescriptorLength without checking
that the received VMBus packet actually contains both byte ranges, so a
truncated packet with an oversized report-descriptor length makes the
guest read past the received packet while copying the descriptor. This
matters most for a confidential guest, where the host is outside the trust
boundary.

Patch 1 passes the received initial-device-info size into the parser and
rejects descriptor lengths that exceed the packet. Patch 2 adds
same-translation-unit KUnit coverage: a well-formed message that must
still parse and the truncated/oversized message that must now be rejected.

Reproduced with the KUnit/KASAN test: stock reads past the packet on the
short message after the benign control passes; patched rejects it and both
cases pass.

Cc: stable@vger.kernel.org

Michael Bommarito (2):
  HID: hyperv: validate initial device info bounds
  HID: hyperv: add KUnit coverage for device info bounds

 drivers/hid/Kconfig      |  10 +++
 drivers/hid/hid-hyperv.c | 144 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 144 insertions(+), 10 deletions(-)

--
2.53.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] HID: hyperv: validate initial device info bounds
  2026-07-10  2:28 [PATCH 0/2] HID: hyperv: bound initial device info descriptor Michael Bommarito
@ 2026-07-10  2:28 ` Michael Bommarito
  2026-07-10  2:40   ` sashiko-bot
  2026-07-10  2:28 ` [PATCH 2/2] HID: hyperv: add KUnit coverage for " Michael Bommarito
  2026-07-11 18:06 ` [PATCH 0/2] HID: hyperv: bound initial device info descriptor Michael Kelley
  2 siblings, 1 reply; 6+ messages in thread
From: Michael Bommarito @ 2026-07-10  2:28 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, kys, Haiyang Zhang, Wei Liu
  Cc: Dexuan Cui, Long Li, linux-input, linux-hyperv, linux-kernel,
	stable

The Hyper-V synthetic HID host supplies SYNTH_HID_INITIAL_DEVICE_INFO
messages that contain a HID descriptor followed by the report descriptor
bytes. mousevsc_on_receive_device_info() trusts bLength and
wDescriptorLength without checking that the received packet contains both
byte ranges.

A malformed host or backend message can therefore make the guest read
past the received VMBus packet while copying the report descriptor. Pass
the received initial-device-info size into the parser and reject
descriptor lengths that exceed the packet.

Impact: A malicious Hyper-V host or backend can crash a guest by sending
a short initial device-info message with an oversized HID report
descriptor length.

Fixes: b95f5bcb811e ("HID: Move the hid-hyperv driver out of staging")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5-5-xhigh
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 drivers/hid/hid-hyperv.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
index 7d2b0063df151..fd90196430e29 100644
--- a/drivers/hid/hid-hyperv.c
+++ b/drivers/hid/hid-hyperv.c
@@ -171,18 +171,32 @@ static void mousevsc_free_device(struct mousevsc_dev *device)
 }
 
 static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
-				struct synthhid_device_info *device_info)
+					    struct synthhid_device_info *device_info,
+					    u32 device_info_size)
 {
 	int ret = 0;
 	struct hid_descriptor *desc;
 	struct mousevsc_prt_msg ack;
+	size_t desc_offset;
+	size_t desc_size;
 
 	input_device->dev_info_status = -ENOMEM;
 
+	if (device_info_size < sizeof(*device_info)) {
+		input_device->dev_info_status = -EINVAL;
+		goto cleanup;
+	}
+
 	input_device->hid_dev_info = device_info->hid_dev_info;
 	desc = &device_info->hid_descriptor;
+	desc_offset = offsetof(struct synthhid_device_info, hid_descriptor);
+	desc_size = device_info_size - desc_offset;
 	if (desc->bLength == 0)
 		goto cleanup;
+	if (desc->bLength < sizeof(*desc) || desc->bLength > desc_size) {
+		input_device->dev_info_status = -EINVAL;
+		goto cleanup;
+	}
 
 	/* The pointer is not NULL when we resume from hibernation */
 	kfree(input_device->hid_desc);
@@ -197,6 +211,10 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
 		input_device->dev_info_status = -EINVAL;
 		goto cleanup;
 	}
+	if (input_device->report_desc_size > desc_size - desc->bLength) {
+		input_device->dev_info_status = -EINVAL;
+		goto cleanup;
+	}
 
 	/* The pointer is not NULL when we resume from hibernation */
 	kfree(input_device->report_desc);
@@ -273,14 +291,17 @@ static void mousevsc_on_receive(struct hv_device *device,
 		break;
 
 	case SYNTH_HID_INITIAL_DEVICE_INFO:
-		WARN_ON(pipe_msg->size < sizeof(struct hv_input_dev_info));
+		if (WARN_ON_ONCE(pipe_msg->size <
+				 sizeof(struct synthhid_device_info)))
+			break;
 
 		/*
 		 * Parse out the device info into device attr,
 		 * hid desc and report desc
 		 */
 		mousevsc_on_receive_device_info(input_dev,
-			(struct synthhid_device_info *)pipe_msg->data);
+						(struct synthhid_device_info *)pipe_msg->data,
+						pipe_msg->size);
 		break;
 	case SYNTH_HID_INPUT_REPORT:
 		input_report =
-- 
2.53.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] HID: hyperv: add KUnit coverage for device info bounds
  2026-07-10  2:28 [PATCH 0/2] HID: hyperv: bound initial device info descriptor Michael Bommarito
  2026-07-10  2:28 ` [PATCH 1/2] HID: hyperv: validate initial device info bounds Michael Bommarito
@ 2026-07-10  2:28 ` Michael Bommarito
  2026-07-10  2:41   ` sashiko-bot
  2026-07-11 18:06 ` [PATCH 0/2] HID: hyperv: bound initial device info descriptor Michael Kelley
  2 siblings, 1 reply; 6+ messages in thread
From: Michael Bommarito @ 2026-07-10  2:28 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, kys, Haiyang Zhang, Wei Liu
  Cc: Dexuan Cui, Long Li, linux-input, linux-hyperv, linux-kernel,
	stable

Add KUnit coverage for Hyper-V synthetic HID initial device-info parsing.
The tests cover zero bLength, a valid descriptor plus report descriptor,
and a malformed report descriptor length that exceeds the received
message.

The same-translation-unit test uses a KUnit-only ACK bypass so parser
coverage does not require a live VMBus channel.

Assisted-by: Codex:gpt-5-5-xhigh
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 drivers/hid/Kconfig      |  10 ++++
 drivers/hid/hid-hyperv.c | 117 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 120 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index c1d9f7c6a5f23..41ca48d9adc9e 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1183,6 +1183,16 @@ config HID_HYPERV_MOUSE
 	help
 	Select this option to enable the Hyper-V mouse driver.
 
+config HID_HYPERV_MOUSE_KUNIT_TEST
+	bool "KUnit tests for Hyper-V mouse driver" if !KUNIT_ALL_TESTS
+	depends on KUNIT && HID_HYPERV_MOUSE
+	default KUNIT_ALL_TESTS
+	help
+	  Builds unit tests for the Hyper-V synthetic HID driver.
+	  These tests exercise the initial device-info parser with
+	  malformed host-provided HID descriptors and are only useful
+	  for kernel developers running KUnit.
+
 config HID_SMARTJOYPLUS
 	tristate "SmartJoy PLUS PS2/USB adapter support"
 	help
diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
index fd90196430e29..6579bd19da13a 100644
--- a/drivers/hid/hid-hyperv.c
+++ b/drivers/hid/hid-hyperv.c
@@ -13,6 +13,9 @@
 #include <linux/hiddev.h>
 #include <linux/hyperv.h>
 
+#if IS_ENABLED(CONFIG_HID_HYPERV_MOUSE_KUNIT_TEST)
+#include <kunit/test.h>
+#endif
 
 struct hv_input_dev_info {
 	unsigned int size;
@@ -240,13 +243,18 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
 	ack.ack.header.size = 1;
 	ack.ack.reserved = 0;
 
-	ret = vmbus_sendpacket(input_device->device->channel,
-			&ack,
-			sizeof(struct pipe_prt_msg) +
-			sizeof(struct synthhid_device_info_ack),
-			(unsigned long)&ack,
-			VM_PKT_DATA_INBAND,
-			VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
+	if (IS_ENABLED(CONFIG_HID_HYPERV_MOUSE_KUNIT_TEST) &&
+	    !input_device->device) {
+		ret = 0;
+	} else {
+		ret = vmbus_sendpacket(input_device->device->channel,
+				       &ack,
+				       sizeof(struct pipe_prt_msg) +
+				       sizeof(struct synthhid_device_info_ack),
+				       (unsigned long)&ack,
+				       VM_PKT_DATA_INBAND,
+				       VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
+	}
 
 	if (!ret)
 		input_device->dev_info_status = 0;
@@ -635,5 +643,100 @@ static void __exit mousevsc_exit(void)
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Microsoft Hyper-V Synthetic HID Driver");
 
+#if IS_ENABLED(CONFIG_HID_HYPERV_MOUSE_KUNIT_TEST)
+static struct mousevsc_dev *mousevsc_kunit_alloc_dev(struct kunit *test)
+{
+	struct mousevsc_dev *input_dev;
+
+	input_dev = kunit_kzalloc(test, sizeof(*input_dev), GFP_KERNEL);
+	if (!input_dev)
+		return NULL;
+
+	init_completion(&input_dev->wait_event);
+
+	return input_dev;
+}
+
+static void mousevsc_device_info_zero_blength(struct kunit *test)
+{
+	struct synthhid_device_info *info;
+	struct mousevsc_dev *input_dev;
+
+	input_dev = mousevsc_kunit_alloc_dev(test);
+	KUNIT_ASSERT_NOT_NULL(test, input_dev);
+	info = kunit_kzalloc(test, sizeof(*info), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, info);
+
+	info->hid_descriptor.bLength = 0;
+
+	mousevsc_on_receive_device_info(input_dev, info, sizeof(*info));
+
+	KUNIT_EXPECT_EQ(test, input_dev->dev_info_status, -ENOMEM);
+}
+
+static void mousevsc_device_info_valid_descriptor(struct kunit *test)
+{
+	struct synthhid_device_info *info;
+	struct mousevsc_dev *input_dev;
+	u8 *report;
+
+	input_dev = mousevsc_kunit_alloc_dev(test);
+	KUNIT_ASSERT_NOT_NULL(test, input_dev);
+	info = kunit_kzalloc(test, sizeof(*info) + 4, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, info);
+
+	info->hid_descriptor.bLength = sizeof(struct hid_descriptor);
+	info->hid_descriptor.rpt_desc.wDescriptorLength = cpu_to_le16(4);
+	report = ((u8 *)&info->hid_descriptor) + info->hid_descriptor.bLength;
+	memset(report, 0x42, 4);
+
+	mousevsc_on_receive_device_info(input_dev, info, sizeof(*info) + 4);
+
+	KUNIT_EXPECT_EQ(test, input_dev->dev_info_status, 0);
+	KUNIT_EXPECT_EQ(test, input_dev->report_desc_size, 4);
+	KUNIT_EXPECT_MEMEQ(test, input_dev->report_desc, report, 4);
+
+	kfree(input_dev->hid_desc);
+	kfree(input_dev->report_desc);
+}
+
+static void mousevsc_device_info_report_desc_oob(struct kunit *test)
+{
+	struct synthhid_device_info *info;
+	struct mousevsc_dev *input_dev;
+	u8 *report;
+
+	input_dev = mousevsc_kunit_alloc_dev(test);
+	KUNIT_ASSERT_NOT_NULL(test, input_dev);
+	info = kunit_kzalloc(test, sizeof(*info) + 8, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, info);
+
+	info->hid_descriptor.bLength = sizeof(struct hid_descriptor);
+	info->hid_descriptor.rpt_desc.wDescriptorLength = cpu_to_le16(64);
+	report = ((u8 *)&info->hid_descriptor) + info->hid_descriptor.bLength;
+	memset(report, 0x42, 8);
+
+	mousevsc_on_receive_device_info(input_dev, info, sizeof(*info) + 8);
+
+	KUNIT_EXPECT_EQ(test, input_dev->dev_info_status, -EINVAL);
+
+	kfree(input_dev->hid_desc);
+}
+
+static struct kunit_case mousevsc_test_cases[] = {
+	KUNIT_CASE(mousevsc_device_info_zero_blength),
+	KUNIT_CASE(mousevsc_device_info_valid_descriptor),
+	KUNIT_CASE(mousevsc_device_info_report_desc_oob),
+	{}
+};
+
+static struct kunit_suite mousevsc_test_suite = {
+	.name = "hid_hyperv_mouse",
+	.test_cases = mousevsc_test_cases,
+};
+
+kunit_test_suite(mousevsc_test_suite);
+#endif
+
 module_init(mousevsc_init);
 module_exit(mousevsc_exit);
-- 
2.53.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] HID: hyperv: validate initial device info bounds
  2026-07-10  2:28 ` [PATCH 1/2] HID: hyperv: validate initial device info bounds Michael Bommarito
@ 2026-07-10  2:40   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-10  2:40 UTC (permalink / raw)
  To: Michael Bommarito; +Cc: linux-hyperv, dmitry.torokhov, linux-input

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

New issues:
- [High] Incomplete Vulnerability Fix / Out-of-Bounds Read
- [Medium] Missing Completion / Timeout Hang
- [Low] Dead Code / Redundant Check

Pre-existing issues:
- [High] Race Condition / Use-After-Free
- [High] Out-of-Bounds Access (Read/Write) in Probe Workaround
- [High] Out-of-Bounds Read (Information Leak) in Input Report
--

--- Patch [1]: [PATCH 1/2] HID: hyperv: validate initial device info bounds ---
commit a1876077d0b8322cccf9c09f43d240367749b037
Author: Michael Bommarito <michael.bommarito@gmail.com>
Subject: HID: hyperv: validate initial device info bounds

This commit attempts to fix an out-of-bounds read by passing the received
initial-device-info size into the parser and rejecting descriptor lengths
that exceed the packet.

> diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
> --- a/drivers/hid/hid-hyperv.c
> +++ b/drivers/hid/hid-hyperv.c
> @@ -171,18 +171,32 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
> -				struct synthhid_device_info *device_info)
> +					    struct synthhid_device_info *device_info,
> +					    u32 device_info_size)
>  {
>  	int ret = 0;
>  	struct hid_descriptor *desc;
>  	struct mousevsc_prt_msg ack;
> +	size_t desc_offset;
> +	size_t desc_size;
>  
>  	input_device->dev_info_status = -ENOMEM;
>  
> +	if (device_info_size < sizeof(*device_info)) {
> +		input_device->dev_info_status = -EINVAL;
> +		goto cleanup;
> +	}

[Severity: Low]
Is this check necessary? In mousevsc_on_receive(), before calling this
function, the code explicitly verifies pipe_msg->size >=
sizeof(struct synthhid_device_info). Since device_info_size is passed
directly from pipe_msg->size, this inner check appears to be unreachable.

> +
>  	input_device->hid_dev_info = device_info->hid_dev_info;
>  	desc = &device_info->hid_descriptor;
> +	desc_offset = offsetof(struct synthhid_device_info, hid_descriptor);
> +	desc_size = device_info_size - desc_offset;
>  	if (desc->bLength == 0)
>  		goto cleanup;
> +	if (desc->bLength < sizeof(*desc) || desc->bLength > desc_size) {
> +		input_device->dev_info_status = -EINVAL;
> +		goto cleanup;
> +	}
>  
>  	/* The pointer is not NULL when we resume from hibernation */
>  	kfree(input_device->hid_desc);
> @@ -197,6 +211,10 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
>  		input_device->dev_info_status = -EINVAL;
>  		goto cleanup;
>  	}
> +	if (input_device->report_desc_size > desc_size - desc->bLength) {
> +		input_device->dev_info_status = -EINVAL;
> +		goto cleanup;
> +	}
>  
>  	/* The pointer is not NULL when we resume from hibernation */
>  	kfree(input_device->report_desc);

[Severity: High]
This is a pre-existing issue, but can this lead to a use-after-free?

During mousevsc_probe(), the driver waits for the first
SYNTH_HID_INITIAL_DEVICE_INFO and proceeds to hid_add_device(), which parses
input_dev->report_desc.

If the host concurrently sends a second SYNTH_HID_INITIAL_DEVICE_INFO packet,
the channel callback processes it asynchronously and calls
mousevsc_on_receive_device_info(). This executes
kfree(input_device->report_desc) without any locking while the HID core is
actively reading it.

[ ... ]
> @@ -273,14 +291,17 @@ static void mousevsc_on_receive(struct hv_device *device,
>  		break;
>  
>  	case SYNTH_HID_INITIAL_DEVICE_INFO:
> -		WARN_ON(pipe_msg->size < sizeof(struct hv_input_dev_info));
> +		if (WARN_ON_ONCE(pipe_msg->size <
> +				 sizeof(struct synthhid_device_info)))
> +			break;

[Severity: Medium]
If this break is taken, are we skipping the completion event?

By breaking out of the switch statement early, we skip the call to
mousevsc_on_receive_device_info(). As a result, the
complete(&input_device->wait_event) inside its cleanup path is never
executed. This might cause the waiting thread in mousevsc_connect_to_vsp()
to stall until the 5-second timeout expires.

>  
>  		/*
>  		 * Parse out the device info into device attr,
>  		 * hid desc and report desc
>  		 */
>  		mousevsc_on_receive_device_info(input_dev,
> -			(struct synthhid_device_info *)pipe_msg->data);
> +						(struct synthhid_device_info *)pipe_msg->data,
> +						pipe_msg->size);

[Severity: High]
Does this fully prevent out-of-bounds reads if the host sends a malicious
payload?

The pipe_msg->size field comes from the untrusted payload. If a malicious
host sends a short physical VMBus packet but sets pipe_msg->size to an
artificially large value, the bounds checks in
mousevsc_on_receive_device_info() will pass because they are evaluated
against this large device_info_size.

Could we restrict the size to the actual VMBus packet boundary, perhaps using
something like (packet->len8 - packet->offset8) << 3, to ensure we don't
read past the received packet?

>  		break;
>  	case SYNTH_HID_INPUT_REPORT:
>  		input_report =
> 			(struct synthhid_input_report *)pipe_msg->data;
> 		if (!input_dev->init_complete)
> 			break;
> 
> 		len = min(input_report->header.size,
> 			  (u32)sizeof(input_dev->input_buf));
> 		memcpy(input_dev->input_buf, input_report->buffer, len);

[Severity: High]
This is a pre-existing issue, but does this blind trust of
input_report->header.size enable an out-of-bounds read?

When handling SYNTH_HID_INPUT_REPORT, a malicious host could send a
physically short packet but set input_report->header.size to 4096.

The memcpy length is bounded using:

len = min(input_report->header.size, (u32)sizeof(input_dev->input_buf));

However, it isn't validated against the actual VMBus packet envelope. This
could result in copying beyond the packet boundary, potentially causing an
access violation or exposing memory adjacent to the packet.


[Severity: High]
This is a pre-existing issue, but I noticed another potential out-of-bounds
access during the probe workaround in mousevsc_probe():

drivers/hid/hid-hyperv.c:mousevsc_probe() {
    ...
    if (input_dev->report_desc[14] == 0x25)
        input_dev->report_desc[14] = 0x29;
    ...
}

If a malicious host provides a wDescriptorLength of less than 15 bytes
(for instance, 8 bytes), this accesses and potentially writes to memory past
the dynamically allocated report_desc buffer. Should we ensure the buffer is
at least 15 bytes long before accessing index 14?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710022854.3739558-1-michael.bommarito@gmail.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] HID: hyperv: add KUnit coverage for device info bounds
  2026-07-10  2:28 ` [PATCH 2/2] HID: hyperv: add KUnit coverage for " Michael Bommarito
@ 2026-07-10  2:41   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-10  2:41 UTC (permalink / raw)
  To: Michael Bommarito; +Cc: linux-hyperv, linux-input, dmitry.torokhov

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

New issues:
- [Low] The embedded KUnit test will be silently disabled and cannot be compiled when both KUnit and the Hyper-V mouse driver are configured as modules.
- [Low] Test-specific mock logic is injected directly into the production VMBus I/O path.

Pre-existing issues:
- [High] The driver and the newly added bounds checking rely on unvalidated inner payload sizes (`pipe_msg->size`, `input_report->header.size`) instead of validating against the actual physical VMBus transport packet size, exposing the guest to OOB reads.
- [High] Missing bounds check before accessing `report_desc[14]` allows out-of-bounds heap memory corruption.
--

--- Patch [2]: [PATCH 2/2] HID: hyperv: add KUnit coverage for device info bounds ---
commit d676a1b2eb911b6f9e0db4e282e431e24c17d123
Author: Michael Bommarito <michael.bommarito@gmail.com>
Subject: HID: hyperv: add KUnit coverage for device info bounds

This commit adds KUnit tests for the Hyper-V synthetic HID initial
device-info parser. The tests exercise malformed host-provided HID
descriptors using a KUnit-only ACK bypass.

> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -1215,6 +1215,16 @@ config HID_HYPERV_MOUSE
[ ... ]
> +config HID_HYPERV_MOUSE_KUNIT_TEST
> +	bool "KUnit tests for Hyper-V mouse driver" if !KUNIT_ALL_TESTS
> +	depends on KUNIT && HID_HYPERV_MOUSE
[Severity: Low]
When both KUNIT and HID_HYPERV_MOUSE are configured as modules, will this
regression cause the dependency expression to evaluate to "m", forcing the
boolean config to "n" and silently preventing the test from being enabled for
module builds?

The standard kernel idiom for this is "depends on KUNIT=y || KUNIT=HID_HYPERV_MOUSE".

> +	default KUNIT_ALL_TESTS
[ ... ]
> diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
> --- a/drivers/hid/hid-hyperv.c
> +++ b/drivers/hid/hid-hyperv.c
[ ... ]
> @@ -240,13 +243,18 @@ static void mousevsc_on_receive_device_info(struct mousevsc_dev *input_device,
[ ... ]
> -	ret = vmbus_sendpacket(input_device->device->channel,
> -			&ack,
> -			sizeof(struct pipe_prt_msg) +
> -			sizeof(struct synthhid_device_info_ack),
> -			(unsigned long)&ack,
> -			VM_PKT_DATA_INBAND,
> -			VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
> +	if (IS_ENABLED(CONFIG_HID_HYPERV_MOUSE_KUNIT_TEST) &&
> +	    !input_device->device) {
[Severity: Low]
Is this regression injecting test-specific mock logic directly into the
production VMBus I/O path?

Could we instead extract the device-info validation into a pure helper
function that the KUnit tests can call directly, avoiding the need to
conditionally bypass network sends in the core logic?

> +		ret = 0;
> +	} else {
> +		ret = vmbus_sendpacket(input_device->device->channel,
[ ... ]
> @@ -635,5 +643,100 @@ static void __exit mousevsc_exit(void)
[ ... ]
> +static void mousevsc_device_info_valid_descriptor(struct kunit *test)
> +{
[ ... ]
> +	info->hid_descriptor.bLength = sizeof(struct hid_descriptor);
> +	info->hid_descriptor.rpt_desc.wDescriptorLength = cpu_to_le16(4);
[Severity: High]
This is a pre-existing issue, but does this test highlight an incomplete
bounds check in the driver?

The test sets the report descriptor length to 4 bytes. However, looking at
mousevsc_probe(), the driver contains a workaround that accesses index 14:

mousevsc_probe() {
    ...
    /* workaround SA-167 */
    if (input_dev->report_desc[14] == 0x25)
        input_dev->report_desc[14] = 0x29;
    ...
}

If a compromised host provides a valid descriptor with a length between 1 and
14, as tested here, could this result in an out-of-bounds heap memory read
and write since the bounds check in mousevsc_on_receive_device_info() only
ensures the size is greater than zero?

> +	report = ((u8 *)&info->hid_descriptor) + info->hid_descriptor.bLength;
[ ... ]
> +static void mousevsc_device_info_report_desc_oob(struct kunit *test)
> +{
[ ... ]
> +	info->hid_descriptor.bLength = sizeof(struct hid_descriptor);
> +	info->hid_descriptor.rpt_desc.wDescriptorLength = cpu_to_le16(64);
> +	report = ((u8 *)&info->hid_descriptor) + info->hid_descriptor.bLength;
> +	memset(report, 0x42, 8);
> +
> +	mousevsc_on_receive_device_info(input_dev, info, sizeof(*info) + 8);
[Severity: High]
This is a pre-existing issue, but does the bounds validation tested here
rely on an unvalidated inner payload size rather than the physical VMBus
transport packet size?

If a compromised or malicious Hyper-V host sends a VMBus packet with a
massive pipe_msg->size but a small physical payload:

mousevsc_on_receive() {
    ...
    mousevsc_on_receive_device_info(input_dev,
                                    (struct synthhid_device_info *)pipe_msg->data,
                                    pipe_msg->size);
    ...
}

Could mousevsc_on_receive() pass this spoofable size to
mousevsc_on_receive_device_info(), leading to out-of-bounds reads during
memcpy() or kmemdup() because the bounds checking trusts this inner size?

> +
> +	KUNIT_EXPECT_EQ(test, input_dev->dev_info_status, -EINVAL);
> +
> +	kfree(input_dev->hid_desc);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710022854.3739558-1-michael.bommarito@gmail.com?part=2

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH 0/2] HID: hyperv: bound initial device info descriptor
  2026-07-10  2:28 [PATCH 0/2] HID: hyperv: bound initial device info descriptor Michael Bommarito
  2026-07-10  2:28 ` [PATCH 1/2] HID: hyperv: validate initial device info bounds Michael Bommarito
  2026-07-10  2:28 ` [PATCH 2/2] HID: hyperv: add KUnit coverage for " Michael Bommarito
@ 2026-07-11 18:06 ` Michael Kelley
  2 siblings, 0 replies; 6+ messages in thread
From: Michael Kelley @ 2026-07-11 18:06 UTC (permalink / raw)
  To: Michael Bommarito, Jiri Kosina, Benjamin Tissoires,
	kys@microsoft.com, Haiyang Zhang, Wei Liu
  Cc: Dexuan Cui, Long Li, linux-input@vger.kernel.org,
	linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org

From: Michael Bommarito <michael.bommarito@gmail.com> Sent: Thursday, July 9, 2026 7:29 PM
> 
> A malicious Hyper-V host or backend can crash a guest with a short
> SYNTH_HID_INITIAL_DEVICE_INFO message. mousevsc_on_receive_device_info()
> trusts the HID descriptor bLength and wDescriptorLength without checking
> that the received VMBus packet actually contains both byte ranges, so a
> truncated packet with an oversized report-descriptor length makes the
> guest read past the received packet while copying the descriptor. This
> matters most for a confidential guest, where the host is outside the trust
> boundary.

For some additional background on the assumed threat model that
underlies this kind of validation (and lack thereof), see [1]. This Hyper-V
mouse driver has .allowed_in_isolated set to "false", so it is never loaded
in a CoCo VM. In normal VMs, the threat model says that we trust the
Hyper-V host not to provide bad values.

But as I said in [1], I'm good with taking additional validations. But Wei
Liu as the maintainer for the Hyper-V drivers is the person who should
decide whether we want to take additional validations.

If we take these additional validations, there's a separate question of
whether to backport them to stable kernels. I'm inclined to *not*
backport to avoid introducing churn (and the risk of breaking something)
when it isn't fixing an observed or likely-to-happen problem. But Wei Liu
should probably weigh in on that as well.

[1] https://lore.kernel.org/linux-hyperv/SN6PR02MB4157D595B990A321BFA85B40D4002@SN6PR02MB4157.namprd02.prod.outlook.com/

Michael

> 
> Patch 1 passes the received initial-device-info size into the parser and
> rejects descriptor lengths that exceed the packet. Patch 2 adds
> same-translation-unit KUnit coverage: a well-formed message that must
> still parse and the truncated/oversized message that must now be rejected.
> 
> Reproduced with the KUnit/KASAN test: stock reads past the packet on the
> short message after the benign control passes; patched rejects it and both
> cases pass.
> 
> Cc: stable@vger.kernel.org
> 
> Michael Bommarito (2):
>   HID: hyperv: validate initial device info bounds
>   HID: hyperv: add KUnit coverage for device info bounds
> 
>  drivers/hid/Kconfig      |  10 +++
>  drivers/hid/hid-hyperv.c | 144 ++++++++++++++++++++++++++++++++++++---
>  2 files changed, 144 insertions(+), 10 deletions(-)
> 
> --
> 2.53.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-11 18:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-10  2:28 [PATCH 0/2] HID: hyperv: bound initial device info descriptor Michael Bommarito
2026-07-10  2:28 ` [PATCH 1/2] HID: hyperv: validate initial device info bounds Michael Bommarito
2026-07-10  2:40   ` sashiko-bot
2026-07-10  2:28 ` [PATCH 2/2] HID: hyperv: add KUnit coverage for " Michael Bommarito
2026-07-10  2:41   ` sashiko-bot
2026-07-11 18:06 ` [PATCH 0/2] HID: hyperv: bound initial device info descriptor Michael Kelley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox