On Sat, May 23, 2026 at 03:27:54PM +0200, Berkant Koc wrote:
> Two independent issues in the synthetic video driver that both stem
> from trusting unvalidated host data.
> 
> 1/2 bounds resolution_count from SYNTHVID_RESOLUTION_RESPONSE against
> the supported_resolution[] array, and populates WIN8 defaults for
> hv->screen_*_max / hv->preferred_* in both the WIN10-probe-failure
> path and the pre-WIN10 path, so a failed or pre-WIN10 probe yields a
> usable display instead of having drm_internal_framebuffer_create()
> reject every userspace framebuffer with -EINVAL.
> 
> 2/2 forwards bytes_recvd from vmbus_recvpacket() into the sub-handler,
> rejects packets that do not cover the synthvid header, and requires
> the type-specific payload size before memcpy/complete or before
> reading the feature-change byte. Rejected packets are logged via
> drm_err_ratelimited() instead of being silently dropped, matching the
> CoCo-hardened pattern in hv_kvp_onchannelcallback().
> 
> 1/2 is unchanged from v3/v4 and carries Michael Kelley's Reviewed-by.
> 
> Changes since v4 (per review by Michael Kelley):
> 
>   2/2: collapsed the leading "if (type == ... ) { ... switch ... }"
>        plus the separate "if (type == FEATURE_CHANGE)" into a single
>        switch on msg->vid_hdr.type. The three completion-driving cases
>        compute their per-type size and break to a shared exit that does
>        the size check + memcpy(init_buf) + complete(); FEATURE_CHANGE is
>        its own case that validates its payload and returns without
>        falling through; unknown types hit default and return. No
>        functional change, fewer lines.
> 
>   2/2: the vmbus_recvpacket() nonzero-return path (e.g. -ENOBUFS for a
>        too-big packet) is itself a malformed-message case. It is now
>        logged via drm_err_ratelimited(), consistent with the
>        sub-handler's other reject paths, instead of being silently
>        skipped. No channel recovery is attempted, as that is not worth
>        the added code for this rare host-side condition.
> 
> Changes since v3 (per review by Michael Kelley):
> 
>   2/2: validate SYNTHVID_RESOLUTION_RESPONSE against its actual
>        variable length. The response carries resolution_count entries,
>        not the full SYNTHVID_MAX_RESOLUTION_COUNT array, so requiring
>        sizeof(struct synthvid_supported_resolution_resp) rejected the
>        shorter responses the host legitimately sends and broke
>        resolution probing. Require the fixed prefix, read and bound
>        resolution_count, then require only the count-sized array.
> 
>   2/2: only run hyperv_receive_sub() when vmbus_recvpacket() returned
>        success. v3 dropped the bytes_recvd upper bound as redundant,
>        which holds only on a successful receive: on -ENOBUFS
>        vmbus_recvpacket() reports the required length in bytes_recvd,
>        which can exceed the 16 KiB hv->recv_buf, and the subsequent
>        memcpy(hv->init_buf, msg, bytes_recvd) would read and write
>        past both buffers. Gating on the success return restores the
>        invariant that made the bound redundant, so an oversized host
>        packet is dropped rather than copied.
> 
> Changes since v2 (per review by Michael Kelley):
> 
>   1/2: dropped the reinit_completion() change; the stale completion can
>        only outlive its request in hyperv_vmbus_resume() after a
>        get_supported_resolution() timeout, which is a narrower fix that
>        belongs in a separate patch against the resume path. Pre-WIN10
>        branch now also populates hv->preferred_*. The else branch is
>        gone; a single screen_width_max == 0 check covers both the
>        pre-WIN10 case and a failed WIN10 probe.
> 
>   2/2: added a per-type switch for the three completion-driving message
>        types so the wait-completion path validates payload size before
>        memcpy/complete. Every reject path emits drm_err_ratelimited()
>        rather than returning silently.
> 
> Changes since v1:
> 
>   1/2: bound resolution_count check folded into the existing zero check;
>        populate WIN8 defaults when hyperv_get_supported_resolution()
>        fails.
>   2/2: forward bytes_recvd into hyperv_receive_sub(); enforce the pipe +
>        synthvid header minimum; check synthvid_feature_change payload
>        size before reading is_dirt_needed.
> 
> The shared init_buf reuse (a duplicate or late host response can
> overwrite init_buf between successive request/response cycles) and the
> related completion reinit are real but orthogonal to this size
> validation. As discussed on v2, they are queued as a separate follow-up
> against the resume/expected-type path once this series lands.
> 
> This series is verified by static analysis and code inspection against
> the synthvid protocol structures and the vmbus_recvpacket() contract. I
> do not currently have a Hyper-V test environment to exercise the receive
> and resolution-probe paths at runtime, so confirmation from someone who
> can run it in a Hyper-V VM would be welcome.
> 
> Both patches carry an Assisted-by: Claude:claude-opus-4-7 berkoc-pipeline
> trailer per the kernel coding-assistants policy. Code, analysis and
> review responses are mine; the model is used as a structured reviewer
> under human verification.
> 
> Berkant Koc (2):
>   drm/hyperv: validate resolution_count and fix WIN8 fallback
>   drm/hyperv: validate VMBus packet size in receive callback
> 
>  drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 113 +++++++++++++++++++---
>  1 file changed, 97 insertions(+), 16 deletions(-)
> 
> 

Applied, thanks!

> base-commit: 4bf5d3da79c48e1df4bab82c9680c53adeff7820
> -- 
> 2.47.3
> 

Reply via email to