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

[sashiko-bot@kernel.org]

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 <me@berkoc.com>
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: stable@vger.kernel.org # 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/cover.1779396074.git.me@berkoc.com?part=2