Re: [PATCH 1/2] drm/hyperv: validate resolution_count from host VMBus message

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Uninitialized State on Error Path Leaves Display Unusable
- [High] Missing Completion Re-Initialization Breaks Driver Synchronization
--

commit 94c5684ae582179bbf9c456d94f6a0bd0f92785f
Author: Berkant Koc <me@berkoc.com>
Subject: drm/hyperv: validate resolution_count from host VMBus message

This patch adds validation to ensure the host-provided resolution_count does
not exceed SYNTHVID_MAX_RESOLUTION_COUNT. It folds this check into the
existing zero-count check to prevent reading past the end of the supported
resolution array.

> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> index 051ecc5268321..003bb118d64c9 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> @@ -391,8 +391,11 @@ static int hyperv_get_supported_resolution(struct hv_device *hdev)
>  		return -ETIMEDOUT;
>  	}

This isn't a bug in your patch, but looking at the wait_for_completion_timeout
call just before this hunk, does the driver reuse the hv->wait completion
structure for multiple sequential VMBus requests without calling
reinit_completion()?

If a previous request times out but the host sends a delayed response, the
receive callback might execute complete(&hv->wait). Could a subsequent
request's wait_for_completion_timeout incorrectly consume this stray
completion, return immediately, and parse stale data from hv->init_buf?

>  
> -	if (msg->resolution_resp.resolution_count == 0) {
> -		drm_err(dev, "No supported resolutions\n");
> +	if (msg->resolution_resp.resolution_count == 0 ||
> +	    msg->resolution_resp.resolution_count >
> +	    SYNTHVID_MAX_RESOLUTION_COUNT) {
> +		drm_err(dev, "Invalid resolution count: %d\n",
> +			msg->resolution_resp.resolution_count);
>  		return -ENODEV;

This is also a pre-existing issue, but I noticed that returning an error
here can leave the display unusable.

When hyperv_get_supported_resolution() fails due to this new bounds check,
the caller hyperv_connect_vsp() logs a warning to use defaults, but it
skips setting up the resolution limits. As a result, hv->screen_width_max
and hv->screen_height_max remain 0.

Later, hyperv_mode_config_init() sets dev->mode_config.max_width and
max_height to 0. Since the DRM core's drm_internal_framebuffer_create()
strictly bounds framebuffer dimensions to dev->mode_config.max_width,
would this cause all userspace attempts to create a framebuffer to fail
with -EINVAL?

>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517-drm-hyperv-cover@berkoc.com?part=1