drm/hyperv: harden host message parsing
Subsystems (5)
v5 cover drm/hyperv: harden host message parsing
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(-)
base-commit: 4bf5d3da79c48e1df4bab82c9680c53adeff7820
Signature · 1 line
2.47.3
PATCH v5 1/2 drm/hyperv: validate resolution_count and fix WIN8 fallback
A SYNTHVID_RESOLUTION_RESPONSE with resolution_count > 64 walks past the supported_resolution[SYNTHVID_MAX_RESOLUTION_COUNT] array in the parse loop. Bound resolution_count against the array size, folded into the existing zero-check.
When the WIN10 resolution probe fails, the caller in hyperv_connect_vsp() left hv->screen_*_max / preferred_* unpopulated, which sets mode_config.max_width / max_height to 0 and makes drm_internal_framebuffer_create() reject every userspace framebuffer with -EINVAL. The pre-WIN10 branch had the same gap for preferred_width / preferred_height. Use a single post-probe fallback guarded by screen_width_max == 0 so both paths converge on the WIN8 defaults.
--- drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +10 -3
@@ -391,8 +391,11 @@ static int hyperv_get_supported_resolution(struct hv_device *hdev)return -ETIMEDOUT ; } -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 ; }
@@ -508,9 +511,13 @@ int hyperv_connect_vsp(struct hv_device *hdev) ret =hyperv_get_supported_resolution (hdev);if (ret)drm_err (dev,"Failed to get supported resolution from host, use default\n" ); - }else { + } + +if (!hv->screen_width_max) { hv->screen_width_max =SYNTHVID_WIDTH_WIN8 ; hv->screen_height_max =SYNTHVID_HEIGHT_WIN8 ; + hv->preferred_width =SYNTHVID_WIDTH_WIN8 ; + hv->preferred_height =SYNTHVID_HEIGHT_WIN8 ; } hv->mmio_megabytes = hdev->channel->offermsg.offer.mmio_megabytes;
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [PATCH v5 1/2] drm/hyperv: validate resolution_count and fix WIN8 fallback
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] State Machine Desynchronization and Type Confusion due to missing reinit_completion() --
commit 75355159caa20ef4cf6b36266c2aa7dcbac0177d
Author: Berkant Koc <[email protected]>
drm/hyperv: validate resolution_count and fix WIN8 fallback
This commit bounds the resolution_count in SYNTHVID_RESOLUTION_RESPONSE to avoid out-of-bounds array access. It also adds a unified fallback for WIN8 default resolutions when the probe fails or max dimensions are left unpopulated.
drivers/gpu/drm/hyperv/hyperv_drm_proto.c static int hyperv_get_supported_resolution(struct hv_device *hdev) · line 392
Quoted 7 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +0
-0@@ -391,8 +391,11 @@ static int hyperv_get_supported_resolution(struct hv_device *hdev)return -ETIMEDOUT ; }
[Severity: High]
This is a pre-existing issue, but does reusing hv->wait without calling reinit_completion() risk state machine desynchronization?
If a prior request like hyperv_negotiate_version() times out, a delayed response could trigger complete(&hv->wait) and leave the completion signaled. When wait_for_completion_timeout() is called in this function, would it return immediately without waiting for the actual response?
If a WIN10 version negotiation times out and falls back to WIN8, could a delayed WIN10 response satisfy the subsequent WIN8 wait, leading the driver to mistakenly assume the hypervisor agreed to the WIN8 protocol?
Since there is no validation of msg->vid_hdr.type after waking up, could this lead to type confusion where a delayed response from an earlier request is incorrectly interpreted as the current response?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
RE: [PATCH v5 1/2] drm/hyperv: validate resolution_count and fix WIN8 fallback
In reply to Berkant KocFrom: Berkant Koc <[email protected]> Sent: Tuesday, May 19, 2026 1:08 PM
Quoted 15 lines
A
SYNTHVID_RESOLUTION_RESPONSEwithresolution_count> 64 walks past the supported_resolution[SYNTHVID_MAX_RESOLUTION_COUNT] array in the parse loop. Boundresolution_countagainst the array size, folded into the existing zero-check.When the WIN10 resolution probe fails, the caller in
hyperv_connect_vsp()left hv->screen_*_max / preferred_* unpopulated, which sets mode_config.max_width /max_heightto 0 and makesdrm_internal_framebuffer_create()reject every userspace framebuffer with-EINVAL. The pre-WIN10 branch had the same gap forpreferred_width/preferred_height. Use a single post-probe fallback guarded byscreen_width_max== 0 so both paths converge on the WIN8 defaults.
Quoted 5 lines
Signed-off-by Berkant Koc <[email protected]>Assisted-by Claude:claude-opus-4-7 berkoc-pipelineFixes 76c56a5affeb ("drm/hyperv: Add DRM driver for hyperv synthetic video device")Cc [email protected] # 5.14+Reviewed-by Michael Kelley <[email protected]>
I ran a basic smoke-test on my local Hyper-V instance. I can confirm that no error messages or failure were generated in the "good" case where the resolution_count from Hyper-V is valid. I did not simulate a bogus resolution_count and ensure it is detected.
Quoted 41 lines
--- drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)drivers/gpu/drm/hyperv/hyperv_drm_proto.c +10
-4@@ -391,8 +391,11 @@ static int hyperv_get_supported_resolution(struct hv_device *hdev)return -ETIMEDOUT ; } -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 ; }@@ -508,9 +511,13 @@ int hyperv_connect_vsp(struct hv_device *hdev) ret =hyperv_get_supported_resolution (hdev);if (ret)drm_err (dev,"Failed to get supported resolution from host, use default\n" ); - }else { + } + +if (!hv->screen_width_max) { hv->screen_width_max =SYNTHVID_WIDTH_WIN8 ; hv->screen_height_max =SYNTHVID_HEIGHT_WIN8 ; + hv->preferred_width =SYNTHVID_WIDTH_WIN8 ; + hv->preferred_height =SYNTHVID_HEIGHT_WIN8 ; } hv->mmio_megabytes = hdev->channel->offermsg.offer.mmio_megabytes; --2.47.3
PATCH v5 2/2 drm/hyperv: validate VMBus packet size in receive callback
In reply to Berkant Kochyperv_receive_sub() reads msg->vid_hdr.type and dispatches into one of four message-type branches without knowing how many bytes the host wrote into hv->recv_buf. The completion path then runs memcpy(hv->init_buf, msg, VMBUS_MAX_PACKET_SIZE), so the consumer that wakes on wait_for_completion_timeout() can read up to 16 KiB of residue from a prior message as if it were the response payload.
Pass bytes_recvd into hyperv_receive_sub() and reject any packet that does not cover the pipe + synthvid header. A single switch on msg->vid_hdr.type then computes the type-specific payload size: the three completion-driving types (SYNTHVID_VERSION_RESPONSE, SYNTHVID_RESOLUTION_RESPONSE, SYNTHVID_VRAM_LOCATION_ACK) fall through to a shared exit that requires that size before memcpy/complete, while SYNTHVID_FEATURE_CHANGE validates its own payload and returns before reading is_dirt_needed. Unknown types are dropped.
SYNTHVID_RESOLUTION_RESPONSE is variable length: the host fills resolution_count entries, not the full SYNTHVID_MAX_RESOLUTION_COUNT array. Validate the fixed prefix first so resolution_count can be read, bound it against the array, then require only the count-sized array, so the shorter responses the host actually sends are accepted.
Only run the sub-handler when vmbus_recvpacket() returned success. The memcpy length is bytes_recvd, which is bounded by VMBUS_MAX_PACKET_SIZE only on a successful receive; on -ENOBUFS vmbus_recvpacket() instead reports the required length, which can exceed hv->recv_buf, so copying bytes_recvd would read and write past the 16 KiB buffers. Gating on the success return keeps the copy bounded. The nonzero-return path is itself a malformed-message case and is now logged rather than silently skipped; channel recovery is not attempted.
Rejected packets are reported via drm_err_ratelimited() rather than silently dropped, matching the CoCo-hardened pattern in hv_kvp_onchannelcallback().
--- drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 100 +++++++++++++++++++--- 1 file changed, 87 insertions(+), 13 deletions(-)
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +87 -13
@@ -420,30 +420,92 @@ static int hyperv_get_supported_resolution(struct hv_device *hdev)return 0 ; } -static void hyperv_receive_sub (struct hv_device *hdev) +static void hyperv_receive_sub (struct hv_device *hdev,u32 bytes_recvd) {struct hyperv_drm_device *hv =hv_get_drvdata (hdev);struct synthvid_msg *msg; +size_t hdr_size; +size_t need;if (!hv)return ; - msg = (struct synthvid_msg *)hv->recv_buf; - -/* 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 ); -complete (&hv->wait); + hdr_size =sizeof (struct pipe_msg_hdr ) + +sizeof (struct synthvid_msg_hdr ); +if (bytes_recvd < hdr_size) { +drm_err_ratelimited (&hv->dev, +"synthvid packet too small for header: %u\n" , + bytes_recvd);return ; } -if (msg->vid_hdr.type ==SYNTHVID_FEATURE_CHANGE ) { + msg = (struct synthvid_msg *)hv->recv_buf; + need = hdr_size; + +switch (msg->vid_hdr.type) { +case SYNTHVID_VERSION_RESPONSE : + need +=sizeof (struct synthvid_version_resp ); +break ; +case SYNTHVID_RESOLUTION_RESPONSE : +/* +* The resolution response is variable length: the host +* fills resolution_count entries, not the full +* SYNTHVID_MAX_RESOLUTION_COUNT array. Require the fixed +* prefix first so resolution_count can be read, then +* demand exactly the count-sized array. +*/ + need += offsetof(struct synthvid_supported_resolution_resp , + supported_resolution); +if (bytes_recvd < need) +break ; +if (msg->resolution_resp.resolution_count > +SYNTHVID_MAX_RESOLUTION_COUNT ) { +drm_err_ratelimited (&hv->dev, +"synthvid resolution count too large: %u\n" , + msg->resolution_resp.resolution_count); +return ; + } + need += msg->resolution_resp.resolution_count * +sizeof (struct hvd_screen_info ); +break ; +case SYNTHVID_VRAM_LOCATION_ACK : + need +=sizeof (struct synthvid_vram_location_ack ); +break ; +case SYNTHVID_FEATURE_CHANGE : +/* +* Not a completion-driving message: validate its own payload +* and consume it here rather than falling through to the +* memcpy/complete shared by the wait-event responses. +*/ +if (bytes_recvd < need + +sizeof (struct synthvid_feature_change )) { +drm_err_ratelimited (&hv->dev, +"synthvid feature change packet too small: %u\n" , + bytes_recvd); +return ; + } hv->dirt_needed = msg->feature_chg.is_dirt_needed;if (hv->dirt_needed)hyperv_hide_hw_ptr (hv->hdev); +return ; +default : +return ; + } + +/* +* Shared completion path for the wait-event responses +* (VERSION_RESPONSE, RESOLUTION_RESPONSE, VRAM_LOCATION_ACK): +* require the type-specific payload before handing the buffer to +* the waiter. +*/ +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); +complete (&hv->wait); }static void hyperv_receive (void *ctx)
@@ -464,9 +526,21 @@ 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 && - recv_buf->pipe_hdr.type ==PIPE_MSG_DATA ) -hyperv_receive_sub (hdev); +if (ret) { +/* +* A nonzero return (e.g. -ENOBUFS for an oversized +* packet) is itself a malformed message: bytes_recvd +* then reports the required length rather than a copied +* payload, so it must not be forwarded to the +* sub-handler. Channel recovery is not attempted. +*/ +drm_err_ratelimited (&hv->dev, +"vmbus_recvpacket failed: %d (need %u)\n" , + ret, bytes_recvd); + }else if (bytes_recvd >0 && + recv_buf->pipe_hdr.type ==PIPE_MSG_DATA ) { +hyperv_receive_sub (hdev, bytes_recvd); + } }while (bytes_recvd >0 && ret ==0 ); }
Signature · 1 line
2.47.3
[email protected]
AI reviewRe: [PATCH v5 2/2] drm/hyperv: validate VMBus packet size in receive callback
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing completion reinitialization allows unsolicited host responses to prematurely satisfy future waits.
- [High] Lack of message type verification after waiting causes type confusion on mismatched responses.
- [High] Concurrent overwrites of the shared hv->init_buf by the receive handler cause a TOCTOU data race and data corruption. --
commit 3d83b43936914ea3bbc3740ccb46ab0c33af0c7a
Author: Berkant Koc <[email protected]>
drm/hyperv: validate VMBus packet size in receive callback
This patch adds validation for the received VMBus packet size to ensure it covers the pipe and synthvid headers. It also computes the type-specific payload size and ensures the copy length is properly bounded.
Quoted 4 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +0
-0
[ ... ]
Quoted 7 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +1
-1@@ -420,30 +420,92 @@ static int hyperv_get_supported_resolution(struct hv_device *hdev)return 0 ; } -static void hyperv_receive_sub (struct hv_device *hdev) +static void hyperv_receive_sub (struct hv_device *hdev,u32 bytes_recvd) {
[ ... ]
drivers/gpu/drm/hyperv/hyperv_drm_proto.c static int hyperv_get_supported_resolution(struct hv_device *hdev) · line 507
Quoted 13 lines
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +12
-0+/* +* Shared completion path for the wait-event responses +* (VERSION_RESPONSE, RESOLUTION_RESPONSE, VRAM_LOCATION_ACK): +* require the type-specific payload before handing the buffer to +* the waiter. +*/ +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 is a pre-existing issue, but does the shared use of hv->init_buf create a data race?
The driver uses the lockless hv->init_buf for both preparing outgoing requests and receiving incoming responses.
If a malicious host sends an unsolicited response packet while the driver thread is actively reading from or writing to hv->init_buf, could this Time-of-Check to Time-of-Use (TOCTOU) overwrite cause the host to bypass validations?
For example, could the host change resolution_count to exceed bounds after it was checked in hyperv_get_supported_resolution()?
[Severity: High]
This isn't a bug introduced by this patch, but does the driver verify the message type after waiting for the response?
Because hyperv_receive_sub() copies any of the three wait-driving message types into hv->init_buf and completes the wait, a malicious host could send a different response type (like SYNTHVID_VERSION_RESPONSE) than the one requested.
The consumer (like hyperv_get_supported_resolution()) will then parse the buffer as the expected type without checking msg->vid_hdr.type, reading mismatched fields and bypassing the newly added length validations.
drivers/gpu/drm/hyperv/hyperv_drm_proto.c static int hyperv_get_supported_resolution(struct hv_device *hdev) · line 508
Quoted 1 line
drivers/gpu/drm/hyperv/hyperv_drm_proto.c +1
-0+complete (&hv->wait);
[Severity: High]
This isn't a bug introduced by this patch, but does the driver fail to reinitialize the completion before waiting?
If a compromised or misbehaving host sends an unsolicited completion-driving message, complete() will increment the internal counter.
When the driver subsequently sends its next request and calls wait_for_completion_timeout(), it will return immediately without waiting for the actual response, allowing the driver to proceed with mismatched or leftover data.
Quoted 1 line
}
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
RE: [PATCH v5 2/2] drm/hyperv: validate VMBus packet size in receive callback
In reply to Berkant KocFrom: Berkant Koc <[email protected]> Sent: Saturday, May 23, 2026 6:28 AM
Quoted 32 lines
hyperv_receive_sub()readsmsg->vid_hdr.typeand dispatches into one of four message-type branches without knowing how many bytes the host wrote intohv->recv_buf. The completion path then runsmemcpy(hv->init_buf, msg, VMBUS_MAX_PACKET_SIZE), so the consumer that wakes onwait_for_completion_timeout()can read up to 16 KiB of residue from a prior message as if it were the response payload.Pass
bytes_recvdintohyperv_receive_sub()and reject any packet that does not cover the pipe + synthvid header. A single switch onmsg->vid_hdr.typethen computes the type-specific payload size: the three completion-driving types (SYNTHVID_VERSION_RESPONSE,SYNTHVID_RESOLUTION_RESPONSE,SYNTHVID_VRAM_LOCATION_ACK) fall through to a shared exit that requires that size before memcpy/complete, whileSYNTHVID_FEATURE_CHANGEvalidates its own payload and returns before readingis_dirt_needed. Unknown types are dropped.
SYNTHVID_RESOLUTION_RESPONSEis variable length: the host fillsresolution_countentries, not the fullSYNTHVID_MAX_RESOLUTION_COUNTarray. Validate the fixed prefix first soresolution_countcan be read, bound it against the array, then require only the count-sized array, so the shorter responses the host actually sends are accepted.Only run the sub-handler when
vmbus_recvpacket()returned success. The memcpy length isbytes_recvd, which is bounded byVMBUS_MAX_PACKET_SIZEonly on a successful receive; on-ENOBUFSvmbus_recvpacket()instead reports the required length, which can exceedhv->recv_buf, so copyingbytes_recvdwould read and write past the 16 KiB buffers. Gating on the success return keeps the copy bounded. The nonzero-return path is itself a malformed-message case and is now logged rather than silently skipped; channel recovery is not attempted.
Quoted 8 lines
Rejected packets are reported via
drm_err_ratelimited()rather than silently dropped, matching the CoCo-hardened pattern inhv_kvp_onchannelcallback().Fixes 76c56a5affeb ("drm/hyperv: Add DRM driver for hyperv synthetic video device")Cc [email protected] # 5.14+Signed-off-by Berkant Koc <[email protected]>Assisted-by Claude:claude-opus-4-7 berkoc-pipeline
This looks good now. The error checking and reporting is robust and the code is well-structured. Thanks for putting up with my sometimes picky feedback. :-)
I also ran a basic smoke-test on my local Hyper-V instance. I can confirm that no error messages or failure were generated in the "good" case where the messages from Hyper-V are properly formatted and sized. I did not simulate bad messages and ensure they are detected.
Quoted 140 lines
--- drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 100 +++++++++++++++++++--- 1 file changed, 87 insertions(+), 13 deletions(-)drivers/gpu/drm/hyperv/hyperv_drm_proto.c +87
-14@@ -420,30 +420,92 @@ static int hyperv_get_supported_resolution(struct hv_device *hdev)return 0 ; } -static void hyperv_receive_sub (struct hv_device *hdev) +static void hyperv_receive_sub (struct hv_device *hdev,u32 bytes_recvd) {struct hyperv_drm_device *hv =hv_get_drvdata (hdev);struct synthvid_msg *msg; +size_t hdr_size; +size_t need;if (!hv)return ; - msg = (struct synthvid_msg *)hv->recv_buf; - -/* 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 ); -complete (&hv->wait); + hdr_size =sizeof (struct pipe_msg_hdr ) + +sizeof (struct synthvid_msg_hdr ); +if (bytes_recvd < hdr_size) { +drm_err_ratelimited (&hv->dev, +"synthvid packet too small for header: %u\n" , + bytes_recvd);return ; } -if (msg->vid_hdr.type ==SYNTHVID_FEATURE_CHANGE ) { + msg = (struct synthvid_msg *)hv->recv_buf; + need = hdr_size; + +switch (msg->vid_hdr.type) { +case SYNTHVID_VERSION_RESPONSE : + need +=sizeof (struct synthvid_version_resp ); +break ; +case SYNTHVID_RESOLUTION_RESPONSE : +/* +* The resolution response is variable length: the host +* fills resolution_count entries, not the full +* SYNTHVID_MAX_RESOLUTION_COUNT array. Require the fixed +* prefix first so resolution_count can be read, then +* demand exactly the count-sized array. +*/ + need += offsetof(struct synthvid_supported_resolution_resp , + supported_resolution); +if (bytes_recvd < need) +break ; +if (msg->resolution_resp.resolution_count > +SYNTHVID_MAX_RESOLUTION_COUNT ) { +drm_err_ratelimited (&hv->dev, +"synthvid resolution count too large: %u\n" , + msg->resolution_resp.resolution_count); +return ; + } + need += msg->resolution_resp.resolution_count * +sizeof (struct hvd_screen_info ); +break ; +case SYNTHVID_VRAM_LOCATION_ACK : + need +=sizeof (struct synthvid_vram_location_ack ); +break ; +case SYNTHVID_FEATURE_CHANGE : +/* +* Not a completion-driving message: validate its own payload +* and consume it here rather than falling through to the +* memcpy/complete shared by the wait-event responses. +*/ +if (bytes_recvd < need + +sizeof (struct synthvid_feature_change )) { +drm_err_ratelimited (&hv->dev, +"synthvid feature change packet too small: %u\n" , + bytes_recvd); +return ; + } hv->dirt_needed = msg->feature_chg.is_dirt_needed;if (hv->dirt_needed)hyperv_hide_hw_ptr (hv->hdev); +return ; +default : +return ; + } + +/* +* Shared completion path for the wait-event responses +* (VERSION_RESPONSE, RESOLUTION_RESPONSE, VRAM_LOCATION_ACK): +* require the type-specific payload before handing the buffer to +* the waiter. +*/ +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); +complete (&hv->wait); }static void hyperv_receive (void *ctx)@@ -464,9 +526,21 @@ 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 && - recv_buf->pipe_hdr.type ==PIPE_MSG_DATA ) -hyperv_receive_sub (hdev); +if (ret) { +/* +* A nonzero return (e.g. -ENOBUFS for an oversized +* packet) is itself a malformed message: bytes_recvd +* then reports the required length rather than a copied +* payload, so it must not be forwarded to the +* sub-handler. Channel recovery is not attempted. +*/ +drm_err_ratelimited (&hv->dev, +"vmbus_recvpacket failed: %d (need %u)\n" , + ret, bytes_recvd); + }else if (bytes_recvd >0 && + recv_buf->pipe_hdr.type ==PIPE_MSG_DATA ) { +hyperv_receive_sub (hdev, bytes_recvd); + } }while (bytes_recvd >0 && ret ==0 ); } --2.47.3
Hamza Mahfooz
AppliedQuoted 98 lines · Berkant Koc
Two independent issues in the synthetic video driver that both stem from trusting unvalidated host data.
1/2 bounds
resolution_countfromSYNTHVID_RESOLUTION_RESPONSEagainst 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 havingdrm_internal_framebuffer_create()reject every userspace framebuffer with-EINVAL.2/2 forwards
bytes_recvdfromvmbus_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 viadrm_err_ratelimited()instead of being silently dropped, matching the CoCo-hardened pattern inhv_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 onmsg->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_CHANGEis 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.-ENOBUFSfor a
too-big packet) is itself a malformed-message case. It is now logged viadrm_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_RESPONSEagainst its actual
variable length. The response carriesresolution_countentries, not the fullSYNTHVID_MAX_RESOLUTION_COUNTarray, so requiringsizeof(struct synthvid_supported_resolution_resp)rejected the shorter responses the host legitimately sends and broke resolution probing. Require the fixed prefix, read and boundresolution_count, then require only the count-sized array.2/2: only run
hyperv_receive_sub()whenvmbus_recvpacket()returned
success. v3 dropped thebytes_recvdupper bound as redundant, which holds only on a successful receive: on-ENOBUFSvmbus_recvpacket()reports the required length inbytes_recvd, which can exceed the 16 KiBhv->recv_buf, and the subsequentmemcpy(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 inhyperv_vmbus_resume()after aget_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 singlescreen_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 emitsdrm_err_ratelimited()rather than returning silently.Changes since v1:
1/2: bound
resolution_countcheck folded into the existing zero check;
populate WIN8 defaults whenhyperv_get_supported_resolution()fails.
2/2: forwardbytes_recvdintohyperv_receive_sub(); enforce the pipe +
synthvid header minimum; checksynthvid_feature_changepayload size before readingis_dirt_needed.The shared
init_bufreuse (a duplicate or late host response can overwriteinit_bufbetween 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.
Quoted 8 lines
Berkant Koc (2):
drm/hyperv: validateresolution_countand fix WIN8 fallback drm/hyperv: validate VMBus packet size in receive callbackdrivers/gpu/drm/hyperv/hyperv_drm_proto.c | 113 +++++++++++++++++++--- 1 file changed, 97 insertions(+), 16 deletions(-)
Applied, thanks!
Quoted 4 lines
base-commit:
4bf5d3da79c48e1df4bab82c9680c53adeff7820Signature · 1 line
2.47.3
From: Hamza Mahfooz <[email protected]> Sent: Monday, May 25, 2026 4:36 AM
Quoted 21 lines · Berkant Koc
Quoted 19 lines
Two independent issues in the synthetic video driver that both stem from trusting unvalidated host data.
1/2 bounds
resolution_countfromSYNTHVID_RESOLUTION_RESPONSEagainst 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 havingdrm_internal_framebuffer_create()reject every userspace framebuffer with-EINVAL.2/2 forwards
bytes_recvdfromvmbus_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 viadrm_err_ratelimited()instead of being silently dropped, matching the CoCo-hardened pattern inhv_kvp_onchannelcallback().1/2 is unchanged from v3/v4 and carries Michael Kelley's Reviewed-by.
[snip]
Quoted 11 lines
Quoted 9 lines
Berkant Koc (2):
drm/hyperv: validateresolution_countand fix WIN8 fallback drm/hyperv: validate VMBus packet size in receive callbackdrivers/gpu/drm/hyperv/hyperv_drm_proto.c | 113 +++++++++++++++++++--- 1 file changed, 97 insertions(+), 16 deletions(-)Applied, thanks!
Hamza -- which tree was this applied to?
Michael
Quoted 4 lines · Michael Kelley
From: Hamza Mahfooz <[email protected]> Sent: Monday, May 25, 2026 4:36 AM
Quoted 1 line
Applied, thanks!
Hamza -- which tree was this applied to?
drm-misc-fixes
Quoted 2 lines
Michael