Re: [PATCH v2 2/2] drm/hyperv: validate VMBus packet size in receive callback

[sashiko-bot@kernel.org]

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 <me@berkoc.com>

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/20260517-drm-hyperv-cover-v2@berkoc.com?part=2