Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Incomplete validation of synchronous response payload sizes causes state leakage and deterministic false success. --
commit 31b0cdbb563323a78bd6df8c18d507462a03a084 Author: Berkant Koc <[email protected]> drm/hyperv: validate VMBus packet size in receive callback This commit updates the hyperv_receive callback to validate the received packet size before processing it. It ensures that the packet contains at least the message headers, passes bytes_recvd to the sub-handler, and uses it to bound the memcpy for synchronous responses. > diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c > b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c > index 3b5065fe06e46..cdab4895dd404 100644 > --- a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c > +++ b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c [ ... ] > @@ -423,26 +423,35 @@ 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 || bytes_recvd > VMBUS_MAX_PACKET_SIZE) > + 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); > + memcpy(hv->init_buf, msg, bytes_recvd); Is it possible for a truncated response to incorrectly report success here? Since hv->init_buf is reused for both sending requests and receiving responses, and their structures alias each other in the synthvid_msg union, a short packet would leave the payload section of hv->init_buf unmodified. For example, if a truncated SYNTHVID_VRAM_LOCATION_ACK packet contains only headers, memcpy() will only copy the headers. The request's vram.user_ctx and the response's vram_ack.user_ctx reside at the exact same memory offset in the union. Because the payload in hv->init_buf is unmodified, msg->vram_ack.user_ctx will perfectly retain the request's vram_pp value. The waiting thread checks this with: if (msg->vram_ack.user_ctx != vram_pp) Because the memory was unchanged, this check will deterministically evaluate to false, causing the driver to proceed as if the VRAM location was successfully acknowledged despite the malformed packet. Should this code validate that bytes_recvd is large enough to contain the specific response type's payload before copying and calling complete()? > complete(&hv->wait); > return; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
