Re: [PATCH v2 4/6] drm_hyperv: Remove support for synth video protocol of old Hyper-V hosts

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing `reinit_completion()` and lack of transaction ID or message type verification causes data races and state machine corruption when VMBus requests time out and receive late responses.
--

--- Patch [4]: [PATCH v2 4/6] drm_hyperv: Remove support for synth video protocol of old Hyper-V hosts ---
commit d7a97b3cab0a3a58ec215d4c2c173f39a5e2648a
Author: Michael Kelley <mhkelley58@gmail.com>

drm_hyperv: Remove support for synth video protocol of old Hyper-V hosts

This commit removes support for synthetic video protocols from Hyper-V
hosts older than Windows 10. It replaces the version negotiation loop
with a single negotiation for the WIN10 version.

> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> index f0ef627b4898a..1d09bba1efdd8 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c
[ ... ]
> @@ -581,11 +559,9 @@ int hv_drm_connect_vsp(struct hv_device *hdev)
>  
>  	hv->screen_depth = SYNTHVID_DEPTH_WIN8;
>  
> -	if (hv_drm_version_ge(hv->synthvid_version, SYNTHVID_VERSION_WIN10)) {
> -		ret = hv_drm_get_supported_resolution(hdev);
> -		if (ret)
> -			drm_err(dev, "Failed to get supported resolution from host, use default\n");
> -	}
> +	ret = hv_drm_get_supported_resolution(hdev);
> +	if (ret)
> +		drm_err(dev, "Failed to get supported resolution from host, use default\n");

[Severity: High]
This is a pre-existing issue, but if hv_drm_get_supported_resolution() times
out here, is it safe to proceed and ignore the error?

hv_drm_get_supported_resolution() issues a request via hv->init_buf and waits
using wait_for_completion_timeout(&hv->wait, ...). If a timeout occurs, the
error is logged but the driver initialization continues.

Later, if another VMBus request is made using the same shared hv->init_buf
(such as in hv_drm_update_vram_location()), a concurrently arriving late
response to the original resolution query could cause a data race:

hv_drm_receive_sub() would overwrite hv->init_buf via memcpy() while the new
request is being built, corrupting the outgoing message. It also signals
complete(&hv->wait), which could prematurely wake up the new request's wait
loop, falsely accepting the old response since transaction IDs aren't verified.

Does the driver need to reinitialize completions or discard late responses
after a timeout to prevent this state machine corruption?

>  
>  	if (!hv->screen_width_max) {
>  		hv->screen_width_max = SYNTHVID_WIDTH_WIN8;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260805203751.2678-1-mhklinux@outlook.com?part=4