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. Cc: [email protected] # 5.14+ > diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c > b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c > index c3d0ff229e3d0..48054b6078f54 100644 > --- a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c > +++ b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c > @@ -420,26 +420,81 @@ static int hyperv_get_supported_resolution(struct > hv_device *hdev) [ ... ] > /* 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; > + [ ... ] > + 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. > 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? > return; > } [ ... ] > @@ -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. > recv_buf->pipe_hdr.type == PIPE_MSG_DATA) > - hyperv_receive_sub(hdev); > + hyperv_receive_sub(hdev, bytes_recvd); > } while (bytes_recvd > 0 && ret == 0); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
