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

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

Patch Review

**Verdict: Good. Well-reasoned defense-in-depth.**

**Header size check** — Correct. The minimum `hdr_size` of `sizeof(struct pipe_msg_hdr) + sizeof(struct synthvid_msg_hdr)` = 16 bytes ensures `msg->vid_hdr.type` is safe to read before the type dispatch:

```c
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;
}
```

**Per-type payload validation** — The switch inside the completion block is the right structure. Each response type gets its own minimum size:

- `SYNTHVID_VERSION_RESPONSE`: `hdr_size + sizeof(struct synthvid_version_resp)` = 22 bytes
- `SYNTHVID_VRAM_LOCATION_ACK`: `hdr_size + sizeof(struct synthvid_vram_location_ack)` = 24 bytes
- `SYNTHVID_FEATURE_CHANGE`: `hdr_size + sizeof(struct synthvid_feature_change)` = 20 bytes

**Variable-length SYNTHVID_RESOLUTION_RESPONSE** — This is the most complex case and it's handled well. The two-step validation is correct:

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

First validates the fixed prefix (EDID block + count/index/is_standard = 131 bytes), then reads `resolution_count`, bounds it, and requires exactly the count-sized array. This avoids the v3 bug where requiring the full `sizeof(struct synthvid_supported_resolution_resp)` (= 387 bytes) would reject the shorter responses the host legitimately sends when fewer than 64 resolutions are reported.

**memcpy size change** — Critical improvement:

```c
// old: copies full 16 KiB regardless of actual data
memcpy(hv->init_buf, msg, VMBUS_MAX_PACKET_SIZE);
// new: copies only received data
memcpy(hv->init_buf, msg, bytes_recvd);
```

This eliminates the stale-data leak where `hyperv_get_supported_resolution()` and other consumers could read residual data from a prior message in `init_buf` beyond `bytes_recvd`. Combined with patch 1's bounds check on `resolution_count`, this closes the window where a short response combined with stale `init_buf` content could produce corrupt resolution data.

**`!ret` gating** — Essential with the new `memcpy(... bytes_recvd)`:

```c
if (!ret && bytes_recvd > 0 &&
    recv_buf->pipe_hdr.type == PIPE_MSG_DATA)
    hyperv_receive_sub(hdev, bytes_recvd);
```

On `vmbus_recvpacket()` returning `-ENOBUFS`, `bytes_recvd` contains the *required* buffer size, which can exceed `VMBUS_MAX_PACKET_SIZE`. Without the `!ret` gate, the old `VMBUS_MAX_PACKET_SIZE` memcpy was safe (both buffers are that size), but the new `bytes_recvd` memcpy would overflow both `recv_buf` and `init_buf`. The cover letter's analysis of this is accurate.

**Observation on completion timeout behavior:** When `hyperv_receive_sub()` rejects a packet (e.g., resolution count too large), it returns without calling `complete()`. The corresponding `wait_for_completion_timeout()` in `hyperv_get_supported_resolution()` / `hyperv_negotiate_version()` / `hyperv_update_vram_location()` will eventually time out and return `-ETIMEDOUT`. This is the correct degraded behavior — a corrupt response should not be treated as valid. Patch 1's fallback ensures the driver still gets usable WIN8 defaults.

**One subtlety worth noting:** The `need` variable is declared as `size_t` inside the `if` block, and `bytes_recvd` is `u32`. The comparison `bytes_recvd < need` promotes `bytes_recvd` to `size_t`, which is safe since `u32` → `size_t` is a widening conversion. No issue here.

Overall, this is a solid hardening series that follows good defensive coding patterns. Both patches are ready for merge.

---
Generated by Claude Code Patch Reviewer