From: Berkant Koc <[email protected]> Sent: Saturday, May 23, 2026 6:28 AM > > 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. A single switch on > msg->vid_hdr.type then computes the type-specific payload size: the > three completion-driving types (SYNTHVID_VERSION_RESPONSE, > SYNTHVID_RESOLUTION_RESPONSE, SYNTHVID_VRAM_LOCATION_ACK) fall through > to a shared exit that requires that size before memcpy/complete, while > SYNTHVID_FEATURE_CHANGE validates its own payload and returns before > reading is_dirt_needed. Unknown types are dropped. > > 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. The nonzero-return path is itself > a malformed-message case and is now logged rather than silently skipped; > channel recovery is not attempted. > > 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") > Cc: [email protected] # 5.14+ > Signed-off-by: Berkant Koc <[email protected]> > Assisted-by: Claude:claude-opus-4-7 berkoc-pipeline
This looks good now. The error checking and reporting is robust and the code is well-structured. Thanks for putting up with my sometimes picky feedback. :-) I also ran a basic smoke-test on my local Hyper-V instance. I can confirm that no error messages or failure were generated in the "good" case where the messages from Hyper-V are properly formatted and sized. I did not simulate bad messages and ensure they are detected. Reviewed-by: Michael Kelley <[email protected]> Tested-by: Michael Kelley <[email protected]> > --- > drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 100 +++++++++++++++++++--- > 1 file changed, 87 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c > b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c > index c3d0ff229..4e6f703a1 100644 > --- a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c > +++ b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c > @@ -420,30 +420,92 @@ 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; > + size_t need; > > if (!hv) > 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); > - complete(&hv->wait); > + 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; > } > > - if (msg->vid_hdr.type == SYNTHVID_FEATURE_CHANGE) { > + msg = (struct synthvid_msg *)hv->recv_buf; > + 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; > + case SYNTHVID_FEATURE_CHANGE: > + /* > + * Not a completion-driving message: validate its own payload > + * and consume it here rather than falling through to the > + * memcpy/complete shared by the wait-event responses. > + */ > + if (bytes_recvd < need + > + 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); > + return; > + default: > + return; > + } > + > + /* > + * Shared completion path for the wait-event responses > + * (VERSION_RESPONSE, RESOLUTION_RESPONSE, VRAM_LOCATION_ACK): > + * require the type-specific payload before handing the buffer to > + * the waiter. > + */ > + 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); > } > > static void hyperv_receive(void *ctx) > @@ -464,9 +526,21 @@ 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 && > - recv_buf->pipe_hdr.type == PIPE_MSG_DATA) > - hyperv_receive_sub(hdev); > + if (ret) { > + /* > + * A nonzero return (e.g. -ENOBUFS for an oversized > + * packet) is itself a malformed message: bytes_recvd > + * then reports the required length rather than a copied > + * payload, so it must not be forwarded to the > + * sub-handler. Channel recovery is not attempted. > + */ > + 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); > + } > } while (bytes_recvd > 0 && ret == 0); > } > > -- > 2.47.3 >
