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 <[email protected]> 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/[email protected]?part=1
