Claude review: drm/hyperv: validate VMBus packet size in receive callback

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Status: Good — minor observations, no blocking issues.**

**Header minimum check:** Correct. The header size computation matches the offset to the union payload in `struct synthvid_msg`:

```c
+	hdr_size = sizeof(struct pipe_msg_hdr) +
+		   sizeof(struct synthvid_msg_hdr);
+	if (bytes_recvd < hdr_size) {
```

`pipe_msg_hdr` is 8 bytes, `synthvid_msg_hdr` is 8 bytes, so `hdr_size = 16`. The cast to `struct synthvid_msg *` is correctly moved after this validation, ensuring `msg->vid_hdr.type` isn't read from an undersized buffer.

**SYNTHVID_VERSION_RESPONSE and SYNTHVID_VRAM_LOCATION_ACK:** Straightforward fixed-size checks. Both correctly add the inner struct size to `need` and fall through to the shared completion path.

**SYNTHVID_RESOLUTION_RESPONSE:** The two-stage validation is well thought out:

```c
+	case SYNTHVID_RESOLUTION_RESPONSE:
+		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, ...);
+			return;
+		}
+		need += msg->resolution_resp.resolution_count *
+			sizeof(struct hvd_screen_info);
+		break;
```

The `offsetof` correctly gives 131 bytes (128 EDID + 1 `resolution_count` + 1 `default_resolution_index` + 1 `is_standard`). The early `break` on undersized packets falls through to the shared path where `bytes_recvd < need` catches and logs it. The `return` on oversized `resolution_count` prevents the completion from firing, which is correct — the waiter will time out and report the failure. The duplicate bounds check with Patch 1's consumer-side check provides defense-in-depth.

**SYNTHVID_FEATURE_CHANGE:** Correctly handled as a self-contained case that validates and returns without falling through to the completion path:

```c
+	case SYNTHVID_FEATURE_CHANGE:
+		if (bytes_recvd < need +
+		    sizeof(struct synthvid_feature_change)) {
```

At this point `need == hdr_size` (16 bytes), plus `sizeof(synthvid_feature_change)` = 4 bytes, for a total of 20 bytes minimum. Correct.

**memcpy change:** The switch from `memcpy(hv->init_buf, msg, VMBUS_MAX_PACKET_SIZE)` to `memcpy(hv->init_buf, msg, bytes_recvd)` is a good improvement. Both buffers are `u8[VMBUS_MAX_PACKET_SIZE]` (0x4000 = 16 KiB), and `bytes_recvd <= VMBUS_MAX_PACKET_SIZE` is guaranteed on a successful `vmbus_recvpacket()` return since that's the buffer size passed. The consumers (`hyperv_negotiate_version`, `hyperv_get_supported_resolution`, `hyperv_update_vram_location`) only access fields within the validated payload extent, so stale bytes beyond `bytes_recvd` in `init_buf` are not a concern.

**vmbus_recvpacket error handling in hyperv_receive():**

```c
+		if (ret) {
+			drm_err_ratelimited(&hv->dev,
+					    "vmbus_recvpacket failed: %d (need %u)\n",
+					    ret, bytes_recvd);
+		} else if (bytes_recvd > 0 &&
+			   recv_buf->pipe_hdr.type == PIPE_MSG_DATA) {
+			hyperv_receive_sub(hdev, bytes_recvd);
+		}
```

This fixes a real bug in the original code: the `do { } while` loop body would execute before the loop condition `ret == 0` is tested, so on an error return (e.g., `-ENOBUFS`), the original code would still enter the `if (bytes_recvd > 0 ...)` body, potentially calling `hyperv_receive_sub()` with stale buffer contents and a `bytes_recvd` value representing the *required* length (which could exceed the 16 KiB buffer). The new gating on `ret` prevents this.

**Minor observations (non-blocking):**

1. The `default:` case silently drops unknown message types. A `drm_dbg_ratelimited()` could aid debugging during development, but silent drop is reasonable for production to avoid log noise from potential future message types.

2. The error message `"vmbus_recvpacket failed: %d (need %u)\n"` uses "need" to describe `bytes_recvd` — this is correct on `-ENOBUFS` where `bytes_recvd` reports the required length, but could be slightly confusing for other error codes. Minor nit; the context makes it clear enough.

3. The loop exit on error means any valid packets queued behind a bad one won't be processed in this callback invocation. This is acceptable — on `-ENOBUFS` the oversized packet is stuck in the ring and retrying would just fail again; the callback will be re-triggered by the next vmbus interrupt.

Overall both patches are correct, well-documented, and ready to merge. The commit messages are thorough and the changelog clearly tracks the evolution across versions.

---
Generated by Claude Code Patch Reviewer