Skip to contentSkip to conversation

drm/hyperv: harden host message parsing

PATCH Superseded Berkant Koc 3 participants 6 messages May 21 – May 22

Subsystems (5)

v4 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

v4 cover drm/hyperv: harden host message parsing

Two independent issues in the synthetic video driver that both stem from trusting unvalidated host data.

1/2 bounds resolution_count from SYNTHVID_RESOLUTION_RESPONSE against the supported_resolution[] array, and populates WIN8 defaults for hv->screen_*_max / hv->preferred_* in both the WIN10-probe-failure path and the pre-WIN10 path, so a failed or pre-WIN10 probe yields a usable display instead of having drm_internal_framebuffer_create() reject every userspace framebuffer with -EINVAL.

2/2 forwards bytes_recvd from vmbus_recvpacket() into the sub-handler, rejects packets that do not cover the synthvid header, and requires the type-specific payload size before memcpy/complete or before reading the feature-change byte. Rejected packets are logged via drm_err_ratelimited() instead of being silently dropped, matching the CoCo-hardened pattern in hv_kvp_onchannelcallback().

1/2 is unchanged from v3 and carries Michael Kelley's Reviewed-by.

Changes since v3 (per review by Michael Kelley):

2/2: validate SYNTHVID_RESOLUTION_RESPONSE against its actual
variable length. The response carries resolution_count entries, not the full SYNTHVID_MAX_RESOLUTION_COUNT array, so requiring sizeof(struct synthvid_supported_resolution_resp) rejected the shorter responses the host legitimately sends and broke resolution probing. Require the fixed prefix, read and bound resolution_count, then require only the count-sized array.

2/2: only run hyperv_receive_sub() when vmbus_recvpacket() returned
success. v3 dropped the bytes_recvd upper bound as redundant, which holds only on a successful receive: on -ENOBUFS vmbus_recvpacket() reports the required length in bytes_recvd, which can exceed the 16 KiB hv->recv_buf, and the subsequent memcpy(hv->init_buf, msg, bytes_recvd) would read and write past both buffers. Gating on the success return restores the invariant that made the bound redundant, so an oversized host packet is dropped rather than copied.

Changes since v2 (per review by Michael Kelley):

1/2: dropped the reinit_completion() change; the stale completion can
only outlive its request in hyperv_vmbus_resume() after a get_supported_resolution() timeout, which is a narrower fix that belongs in a separate patch against the resume path. Pre-WIN10 branch now also populates hv->preferred_*. The else branch is gone; a single screen_width_max == 0 check covers both the pre-WIN10 case and a failed WIN10 probe.

2/2: added a per-type switch for the three completion-driving message
types so the wait-completion path validates payload size before memcpy/complete. Every reject path emits drm_err_ratelimited() rather than returning silently.

Changes since v1:

1/2: bound resolution_count check folded into the existing zero check;
populate WIN8 defaults when hyperv_get_supported_resolution() fails.
2/2: forward bytes_recvd into hyperv_receive_sub(); enforce the pipe +
synthvid header minimum; check synthvid_feature_change payload size before reading is_dirt_needed.

The shared init_buf reuse (a duplicate or late host response can overwrite init_buf between successive request/response cycles) and the related completion reinit are real but orthogonal to this size validation. As discussed on v2, they are queued as a separate follow-up against the resume/expected-type path once this series lands.

This series is verified by static analysis and code inspection against the synthvid protocol structures and the vmbus_recvpacket() contract. I do not currently have a Hyper-V test environment to exercise the receive and resolution-probe paths at runtime, so confirmation from someone who can run it in a Hyper-V VM would be welcome.

Both patches carry an Assisted-by: Claude:claude-opus-4-7 berkoc-pipeline trailer per the kernel coding-assistants policy. Code, analysis and review responses are mine; the model is used as a structured reviewer under human verification.

Berkant Koc (2):
drm/hyperv: validate resolution_count and fix WIN8 fallback drm/hyperv: validate VMBus packet size in receive callback

 drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 76 ++++++++++++++++++++---
 1 file changed, 69 insertions(+), 7 deletions(-)

base-commit: 4bf5d3da79c48e1df4bab82c9680c53adeff7820

Signature · 1 line
2.47.3
#1 lore raw

PATCH v4 1/2 drm/hyperv: validate resolution_count and fix WIN8 fallback

A SYNTHVID_RESOLUTION_RESPONSE with resolution_count > 64 walks past the supported_resolution[SYNTHVID_MAX_RESOLUTION_COUNT] array in the parse loop. Bound resolution_count against the array size, folded into the existing zero-check.

When the WIN10 resolution probe fails, the caller in hyperv_connect_vsp() left hv->screen_*_max / preferred_* unpopulated, which sets mode_config.max_width / max_height to 0 and makes drm_internal_framebuffer_create() reject every userspace framebuffer with -EINVAL. The pre-WIN10 branch had the same gap for preferred_width / preferred_height. Use a single post-probe fallback guarded by screen_width_max == 0 so both paths converge on the WIN8 defaults.

Assisted-by Claude:claude-opus-4-7 berkoc-pipeline
Fixes 76c56a5affeb ("drm/hyperv: Add DRM driver for hyperv synthetic video device")
---
 drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +10 -3
index 051ecc526..c3d0ff229 100644
@@ -391,8 +391,11 @@ static int hyperv_get_supported_resolution(struct hv_device *hdev)
 		return -ETIMEDOUT;
 	}
 
-	if (msg->resolution_resp.resolution_count == 0) {
-		drm_err(dev, "No supported resolutions\n");
+	if (msg->resolution_resp.resolution_count == 0 ||
+	    msg->resolution_resp.resolution_count >
+	    SYNTHVID_MAX_RESOLUTION_COUNT) {
+		drm_err(dev, "Invalid resolution count: %d\n",
+			msg->resolution_resp.resolution_count);
 		return -ENODEV;
 	}
 
@@ -508,9 +511,13 @@ int hyperv_connect_vsp(struct hv_device *hdev)
 		ret = hyperv_get_supported_resolution(hdev);
 		if (ret)
 			drm_err(dev, "Failed to get supported resolution from host, use default\n");
-	} else {
+	}
+
+	if (!hv->screen_width_max) {
 		hv->screen_width_max = SYNTHVID_WIDTH_WIN8;
 		hv->screen_height_max = SYNTHVID_HEIGHT_WIN8;
+		hv->preferred_width = SYNTHVID_WIDTH_WIN8;
+		hv->preferred_height = SYNTHVID_HEIGHT_WIN8;
 	}
 
 	hv->mmio_megabytes = hdev->channel->offermsg.offer.mmio_megabytes;
Signature · 1 line
2.47.3
#2 lore raw

Re: [PATCH v4 1/2] drm/hyperv: validate resolution_count and fix WIN8 fallback

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

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

Pre-existing issues:
- [High] Missing Completion Reinitialization causes state machine desynchronization.
- [High] Missing Message Type Validation allows parsing of incorrect response payloads. --

commit e5968f0d509475850afc95a1bf9a982ea5002949
Author: Berkant Koc <[email protected]>

drm/hyperv: validate resolution_count and fix WIN8 fallback

This commit bounds resolution_count against the array size to prevent walking past the supported_resolution array in the parse loop. It also provides a single post-probe fallback to ensure userspace framebuffers are not rejected when the WIN10 resolution probe fails.

drivers/gpu/drm/hyperv/hyperv_drm_proto.c static int hyperv_get_supported_resolution(struct hv_device *hdev) · line 392

Quoted 7 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +0 -0
index 051ecc5268321..c3d0ff229e3d0 100644
@@ -391,8 +391,11 @@ static int hyperv_get_supported_resolution(struct hv_device *hdev)
 		return -ETIMEDOUT;
 	}

[Severity: High]
This is a pre-existing issue, but is it possible for the state machine to become desynchronized here if a timeout occurs?

The driver uses wait_for_completion_timeout() for synchronous VMBUS requests. If the host is slow and the wait times out, the hv->wait completion counter is not decremented since it returns early.

If the host responds after the timeout, hyperv_receive_sub() calls complete(&hv->wait), incrementing the counter. Because the driver never calls reinit_completion(&hv->wait) before sending a new request, the next transaction could consume the late completion from the previous request immediately.

For example, a timeout during hyperv_get_supported_resolution() followed by a late response could cause a subsequent hyperv_update_vram_location() to complete immediately before the host has actually acknowledged the VRAM location, which might lead to memory corruption or VM crashes.

drivers/gpu/drm/hyperv/hyperv_drm_proto.c static int hyperv_get_supported_resolution(struct hv_device *hdev) · line 396

Quoted 6 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +3 -2
-	if (msg->resolution_resp.resolution_count == 0) {
-		drm_err(dev, "No supported resolutions\n");
+	if (msg->resolution_resp.resolution_count == 0 ||
+	    msg->resolution_resp.resolution_count >
+	    SYNTHVID_MAX_RESOLUTION_COUNT) {

[Severity: High]
This isn't a bug introduced by this patch, but should the code verify the message type before accessing the payload?

The driver uses a single wait completion (hv->wait) and a single response buffer (hv->init_buf) for multiple VMBUS message types. In hyperv_receive_sub(), receiving any valid response type (e.g., SYNTHVID_VERSION_RESPONSE, SYNTHVID_RESOLUTION_RESPONSE, SYNTHVID_VRAM_LOCATION_ACK) will trigger complete(&hv->wait).

Functions waiting for responses, like hyperv_get_supported_resolution(), appear to assume the payload in hv->init_buf is the correct response type without verifying msg->vid_hdr.type.

If the state machine is desynchronized (e.g., due to a previous timeout) or a buggy host sends an unsolicited response, could the driver misinterpret the payload by reading fields like resolution_count from stale memory or data belonging to a different response type?

Quoted 4 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +2 -0
+		drm_err(dev, "Invalid resolution count: %d\n",
+			msg->resolution_resp.resolution_count);
 		return -ENODEV;
 	}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
#3 lore raw

PATCH v4 2/2 drm/hyperv: validate VMBus packet size in receive callback

In reply to Berkant Koc

hyperv_receive_sub() reads msg->vid_hdr.type and dispatches into one of four message-type branches without knowing how many bytes the host wrote into hv->recv_buf. The completion path then runs memcpy(hv->init_buf, msg, VMBUS_MAX_PACKET_SIZE), so the consumer that wakes on wait_for_completion_timeout() can read up to 16 KiB of residue from a prior message as if it were the response payload.

Pass bytes_recvd into hyperv_receive_sub() and reject any packet that does not cover the pipe + synthvid header. For the three completion-driving types (SYNTHVID_VERSION_RESPONSE, SYNTHVID_RESOLUTION_RESPONSE, SYNTHVID_VRAM_LOCATION_ACK) require the type-specific payload before memcpy/complete, and apply the same rule to SYNTHVID_FEATURE_CHANGE before reading is_dirt_needed.

SYNTHVID_RESOLUTION_RESPONSE is variable length: the host fills resolution_count entries, not the full SYNTHVID_MAX_RESOLUTION_COUNT array. Validate the fixed prefix first so resolution_count can be read, bound it against the array, then require only the count-sized array, so the shorter responses the host actually sends are accepted.

Only run the sub-handler when vmbus_recvpacket() returned success. The memcpy length is bytes_recvd, which is bounded by VMBUS_MAX_PACKET_SIZE only on a successful receive; on -ENOBUFS vmbus_recvpacket() instead reports the required length, which can exceed hv->recv_buf, so copying bytes_recvd would read and write past the 16 KiB buffers. Gating on the success return keeps the copy bounded.

Rejected packets are reported via drm_err_ratelimited() rather than silently dropped, matching the CoCo-hardened pattern in hv_kvp_onchannelcallback().

Fixes 76c56a5affeb ("drm/hyperv: Add DRM driver for hyperv synthetic video device")
Assisted-by Claude:claude-opus-4-7 berkoc-pipeline
---
 drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 63 +++++++++++++++++++++--
 1 file changed, 59 insertions(+), 4 deletions(-)
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +59 -4
index c3d0ff229..48054b607 100644
@@ -420,26 +420,81 @@ static int hyperv_get_supported_resolution(struct hv_device *hdev)
 	return 0;
 }
 
-static void hyperv_receive_sub(struct hv_device *hdev)
+static void hyperv_receive_sub(struct hv_device *hdev, u32 bytes_recvd)
 {
 	struct hyperv_drm_device *hv = hv_get_drvdata(hdev);
 	struct synthvid_msg *msg;
+	size_t hdr_size;
 
 	if (!hv)
 		return;
 
+	hdr_size = sizeof(struct pipe_msg_hdr) +
+		   sizeof(struct synthvid_msg_hdr);
+	if (bytes_recvd < hdr_size) {
+		drm_err_ratelimited(&hv->dev,
+				    "synthvid packet too small for header: %u\n",
+				    bytes_recvd);
+		return;
+	}
+
 	msg = (struct synthvid_msg *)hv->recv_buf;
 
 	/* Complete the wait event */
 	if (msg->vid_hdr.type == SYNTHVID_VERSION_RESPONSE ||
 	    msg->vid_hdr.type == SYNTHVID_RESOLUTION_RESPONSE ||
 	    msg->vid_hdr.type == SYNTHVID_VRAM_LOCATION_ACK) {
-		memcpy(hv->init_buf, msg, VMBUS_MAX_PACKET_SIZE);
+		size_t need = hdr_size;
+
+		switch (msg->vid_hdr.type) {
+		case SYNTHVID_VERSION_RESPONSE:
+			need += sizeof(struct synthvid_version_resp);
+			break;
+		case SYNTHVID_RESOLUTION_RESPONSE:
+			/*
+			 * The resolution response is variable length: the host
+			 * fills resolution_count entries, not the full
+			 * SYNTHVID_MAX_RESOLUTION_COUNT array. Require the fixed
+			 * prefix first so resolution_count can be read, then
+			 * demand exactly the count-sized array.
+			 */
+			need += offsetof(struct synthvid_supported_resolution_resp,
+					 supported_resolution);
+			if (bytes_recvd < need)
+				break;
+			if (msg->resolution_resp.resolution_count >
+			    SYNTHVID_MAX_RESOLUTION_COUNT) {
+				drm_err_ratelimited(&hv->dev,
+						    "synthvid resolution count too large: %u\n",
+						    msg->resolution_resp.resolution_count);
+				return;
+			}
+			need += msg->resolution_resp.resolution_count *
+				sizeof(struct hvd_screen_info);
+			break;
+		case SYNTHVID_VRAM_LOCATION_ACK:
+			need += sizeof(struct synthvid_vram_location_ack);
+			break;
+		}
+		if (bytes_recvd < need) {
+			drm_err_ratelimited(&hv->dev,
+					    "synthvid packet too small for type %u: %u < %zu\n",
+					    msg->vid_hdr.type, bytes_recvd, need);
+			return;
+		}
+		memcpy(hv->init_buf, msg, bytes_recvd);
 		complete(&hv->wait);
 		return;
 	}
 
 	if (msg->vid_hdr.type == SYNTHVID_FEATURE_CHANGE) {
+		if (bytes_recvd < hdr_size +
+		    sizeof(struct synthvid_feature_change)) {
+			drm_err_ratelimited(&hv->dev,
+					    "synthvid feature change packet too small: %u\n",
+					    bytes_recvd);
+			return;
+		}
 		hv->dirt_needed = msg->feature_chg.is_dirt_needed;
 		if (hv->dirt_needed)
 			hyperv_hide_hw_ptr(hv->hdev);
@@ -464,9 +519,9 @@ static void hyperv_receive(void *ctx)
 		ret = vmbus_recvpacket(hdev->channel, recv_buf,
 				       VMBUS_MAX_PACKET_SIZE,
 				       &bytes_recvd, &req_id);
-		if (bytes_recvd > 0 &&
+		if (!ret && bytes_recvd > 0 &&
 		    recv_buf->pipe_hdr.type == PIPE_MSG_DATA)
-			hyperv_receive_sub(hdev);
+			hyperv_receive_sub(hdev, bytes_recvd);
 	} while (bytes_recvd > 0 && ret == 0);
 }
 
Signature · 1 line
2.47.3
#4 lore raw

Re: [PATCH v4 2/2] drm/hyperv: validate VMBus packet size in receive callback

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

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

Pre-existing issues:
- [High] The completion hv->wait is not re-initialized before sending a new request, leading to premature completions and use of stale data.
- [High] The wait thread unconditionally trusts that the received message in hv->init_buf matches the expected response type without validating vid_hdr.type.
- [High] If the host sends a packet larger than VMBUS_MAX_PACKET_SIZE, the channel becomes permanently wedged.
- [High] Unprotected concurrent access to hv->init_buf causes outbound request corruption.
--

commit 6ae2d5d611e1bfae5ccaf2facc0f88f11136806d

Author: Berkant Koc <[email protected]>
Date:   Tue May 19 20:08:53 2026 +0000

drm/hyperv: validate VMBus packet size in receive callback

This commit validates the size of received VMBus packets to prevent reading past the end of the receive buffer. It requires specific payload sizes based on the message type. It also prevents copying when vmbus_recvpacket() returns an error.

Quoted 5 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +0 -0
index c3d0ff229e3d0..48054b6078f54 100644
@@ -420,26 +420,81 @@ static int hyperv_get_supported_resolution(struct hv_device *hdev)

[ ... ]

Quoted 7 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +2 -1
 	/* Complete the wait event */
 	if (msg->vid_hdr.type == SYNTHVID_VERSION_RESPONSE ||
 	    msg->vid_hdr.type == SYNTHVID_RESOLUTION_RESPONSE ||
 	    msg->vid_hdr.type == SYNTHVID_VRAM_LOCATION_ACK) {
-		memcpy(hv->init_buf, msg, VMBUS_MAX_PACKET_SIZE);
+		size_t need = hdr_size;
+

[ ... ]

drivers/gpu/drm/hyperv/hyperv_drm_proto.c static int hyperv_get_supported_resolution(struct hv_device *hdev) · line 485

Quoted 7 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +7 -0
+		if (bytes_recvd < need) {
+			drm_err_ratelimited(&hv->dev,
+					    "synthvid packet too small for type %u: %u < %zu\n",
+					    msg->vid_hdr.type, bytes_recvd, need);
+			return;
+		}
+		memcpy(hv->init_buf, msg, bytes_recvd);

[Severity: High]
This isn't a bug introduced by this patch, but does the receive callback overwrite hv->init_buf concurrently while process-context functions are using it to construct outbound requests?

For example, if hyperv_get_supported_resolution() is building a request in hv->init_buf, and a delayed response from a previous request arrives, this memcpy() will overwrite the buffer.

[Severity: High]
This is a pre-existing issue, but does the wait thread unconditionally trust that the message in hv->init_buf matches its expected response type?

Since this callback completes the wait for multiple message types, an unexpected or delayed message could cause the waiting thread to awaken and parse overlapping union fields incorrectly.

Quoted 1 line

complete(&hv->wait);

[Severity: High]
This is a pre-existing issue, but is the completion hv->wait ever re-initialized before being reused?

If a request times out and the host responds later, complete() will be called, leaving the completion signaled. The next request calling wait_for_completion_timeout() will return immediately, leading to desynchronization. Should callers use reinit_completion() before sending a request?

Quoted 2 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +0 -0
 		return;
 	}

[ ... ]

drivers/gpu/drm/hyperv/hyperv_drm_proto.c static void hyperv_receive(void *ctx) · line 522

Quoted 6 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +1 -1
@@ -464,9 +519,9 @@ static void hyperv_receive(void *ctx)
 		ret = vmbus_recvpacket(hdev->channel, recv_buf,
 				       VMBUS_MAX_PACKET_SIZE,
 				       &bytes_recvd, &req_id);
-		if (bytes_recvd > 0 &&
+		if (!ret && bytes_recvd > 0 &&

[Severity: High]
This is a pre-existing issue, but if vmbus_recvpacket() returns -ENOBUFS because the host sends a packet larger than VMBUS_MAX_PACKET_SIZE, does the channel become permanently wedged?

The loop condition will exit, and the oversized packet remains in the ring buffer. Subsequent interrupts will continue to fail with -ENOBUFS, resulting in a permanent channel stall.

Quoted 5 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +1 -1
 		    recv_buf->pipe_hdr.type == PIPE_MSG_DATA)
-			hyperv_receive_sub(hdev);
+			hyperv_receive_sub(hdev, bytes_recvd);
 	} while (bytes_recvd > 0 && ret == 0);
 }
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#5 lore raw

RE: [PATCH v4 2/2] drm/hyperv: validate VMBus packet size in receive callback

In reply to Berkant Koc

From: Berkant Koc <[email protected]> Sent: Tuesday, May 19, 2026 1:09 PM

Quoted 68 lines

hyperv_receive_sub() reads msg->vid_hdr.type and dispatches into one of four message-type branches without knowing how many bytes the host wrote into hv->recv_buf. The completion path then runs memcpy(hv->init_buf, msg, VMBUS_MAX_PACKET_SIZE), so the consumer that wakes on wait_for_completion_timeout() can read up to 16 KiB of residue from a prior message as if it were the response payload.

Pass bytes_recvd into hyperv_receive_sub() and reject any packet that does not cover the pipe + synthvid header. For the three completion-driving types (SYNTHVID_VERSION_RESPONSE, SYNTHVID_RESOLUTION_RESPONSE, SYNTHVID_VRAM_LOCATION_ACK) require the type-specific payload before memcpy/complete, and apply the same rule to SYNTHVID_FEATURE_CHANGE before reading is_dirt_needed.

SYNTHVID_RESOLUTION_RESPONSE is variable length: the host fills resolution_count entries, not the full SYNTHVID_MAX_RESOLUTION_COUNT array. Validate the fixed prefix first so resolution_count can be read, bound it against the array, then require only the count-sized array, so the shorter responses the host actually sends are accepted.

Only run the sub-handler when vmbus_recvpacket() returned success. The memcpy length is bytes_recvd, which is bounded by VMBUS_MAX_PACKET_SIZE only on a successful receive; on -ENOBUFS vmbus_recvpacket() instead reports the required length, which can exceed hv->recv_buf, so copying bytes_recvd would read and write past the 16 KiB buffers. Gating on the success return keeps the copy bounded.

Rejected packets are reported via drm_err_ratelimited() rather than silently dropped, matching the CoCo-hardened pattern in hv_kvp_onchannelcallback().

Fixes 76c56a5affeb ("drm/hyperv: Add DRM driver for hyperv synthetic video device")
Assisted-by Claude:claude-opus-4-7 berkoc-pipeline
---
 drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 63 +++++++++++++++++++++--
 1 file changed, 59 insertions(+), 4 deletions(-)
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +11 -1
index c3d0ff229..48054b607 100644
@@ -420,26 +420,81 @@ static int hyperv_get_supported_resolution(struct hv_device *hdev)
 	return 0;
 }
 
-static void hyperv_receive_sub(struct hv_device *hdev)
+static void hyperv_receive_sub(struct hv_device *hdev, u32 bytes_recvd)
 {
 	struct hyperv_drm_device *hv = hv_get_drvdata(hdev);
 	struct synthvid_msg *msg;
+	size_t hdr_size;
 
 	if (!hv)
 		return;
 
+	hdr_size = sizeof(struct pipe_msg_hdr) +
+		   sizeof(struct synthvid_msg_hdr);
+	if (bytes_recvd < hdr_size) {
+		drm_err_ratelimited(&hv->dev,
+				    "synthvid packet too small for header: %u\n",
+				    bytes_recvd);
+		return;
+	}
+

drivers/gpu/drm/hyperv/hyperv_drm_proto.c static int hyperv_get_supported_resolution(struct hv_device *hdev) · line 449

Quoted 10 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +3 -1
 	msg = (struct synthvid_msg *)hv->recv_buf;
 
 	/* Complete the wait event */
 	if (msg->vid_hdr.type == SYNTHVID_VERSION_RESPONSE ||
 	    msg->vid_hdr.type == SYNTHVID_RESOLUTION_RESPONSE ||
 	    msg->vid_hdr.type == SYNTHVID_VRAM_LOCATION_ACK) {
-		memcpy(hv->init_buf, msg, VMBUS_MAX_PACKET_SIZE);
+		size_t need = hdr_size;
+
+		switch (msg->vid_hdr.type) {

Having the combination of the above "if" tests followed by the switch statement on the same value is logical duplication that suggests to me that some code reorganization is appropriate. Consider the following approach:

1) The drop the big "if" statement and make the switch and cases be the main decision point. Include SYNTHVID_FEATURE_CHANGE as a case in the switch statement.

2) The check against "need" followed by the memcpy() and complete() are used by the original three cases in the switch. Make that the normal exit path for the function so that the "break" statements for those three cases still do what you want.

3) For the SYNTHVID_FEATURE_CHANGE case, just do a "return" when done since that case doesn't want the check against "need" or the memcpy()/complete() operations.

I haven't coded this up, but I think it should be cleaner and be fewer lines of code.

Quoted 52 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +43 -0
+		case SYNTHVID_VERSION_RESPONSE:
+			need += sizeof(struct synthvid_version_resp);
+			break;
+		case SYNTHVID_RESOLUTION_RESPONSE:
+			/*
+			 * The resolution response is variable length: the host
+			 * fills resolution_count entries, not the full
+			 * SYNTHVID_MAX_RESOLUTION_COUNT array. Require the fixed
+			 * prefix first so resolution_count can be read, then
+			 * demand exactly the count-sized array.
+			 */
+			need += offsetof(struct synthvid_supported_resolution_resp,
+					 supported_resolution);
+			if (bytes_recvd < need)
+				break;
+			if (msg->resolution_resp.resolution_count >
+			    SYNTHVID_MAX_RESOLUTION_COUNT) {
+				drm_err_ratelimited(&hv->dev,
+						    "synthvid resolution count too large: %u\n",
+						    msg->resolution_resp.resolution_count);
+				return;
+			}
+			need += msg->resolution_resp.resolution_count *
+				sizeof(struct hvd_screen_info);
+			break;
+		case SYNTHVID_VRAM_LOCATION_ACK:
+			need += sizeof(struct synthvid_vram_location_ack);
+			break;
+		}
+		if (bytes_recvd < need) {
+			drm_err_ratelimited(&hv->dev,
+					    "synthvid packet too small for type %u: %u < %zu\n",
+					    msg->vid_hdr.type, bytes_recvd, need);
+			return;
+		}
+		memcpy(hv->init_buf, msg, bytes_recvd);
 		complete(&hv->wait);
 		return;
 	}
 
 	if (msg->vid_hdr.type == SYNTHVID_FEATURE_CHANGE) {
+		if (bytes_recvd < hdr_size +
+		    sizeof(struct synthvid_feature_change)) {
+			drm_err_ratelimited(&hv->dev,
+					    "synthvid feature change packet too small: %u\n",
+					    bytes_recvd);
+			return;
+		}
 		hv->dirt_needed = msg->feature_chg.is_dirt_needed;
 		if (hv->dirt_needed)
 			hyperv_hide_hw_ptr(hv->hdev);

drivers/gpu/drm/hyperv/hyperv_drm_proto.c static void hyperv_receive(void *ctx) · line 522

Quoted 5 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +1 -1
@@ -464,9 +519,9 @@ static void hyperv_receive(void *ctx)
 		ret = vmbus_recvpacket(hdev->channel, recv_buf,
 				       VMBUS_MAX_PACKET_SIZE,
 				       &bytes_recvd, &req_id);
-		if (bytes_recvd > 0 &&
+		if (!ret && bytes_recvd > 0 &&

This patch is all about detecting malformed messages from Hyper-V, and the ret != 0 case is another example of a malformed message because the message is too big. Since malformed messages are no longer being silently ignored, output a rate limited error message in that case, just like in the other malformed message cases.

As Sashiko pointed out, a message that's too big will block receipt of any further messages on this channel, but I don't think it's worth trying to code any kind of recovery. The channel is broken, presumably due to some bug in Hyper-V. The only recovery is for a sysadmin to manually unbind the driver from the synthetic device (which should close the channel), then manually rebind and try to start over again. And then report the problem to the Hyper-V team. :-)

Quoted 9 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +1 -2
 		    recv_buf->pipe_hdr.type == PIPE_MSG_DATA)
-			hyperv_receive_sub(hdev);
+			hyperv_receive_sub(hdev, bytes_recvd);
 	} while (bytes_recvd > 0 && ret == 0);
 }
 
--

2.47.3