Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues: - [Low] Unreachable version negotiation fallback logic left in hv_balloon.c. - [Low] Embedded newline and multi-line string literal continuation breaks kernel log formatting. - [Low] The patch series missed removing legacy VMBus version checks for VERSION_WIN8 and VERSION_WIN8_1 in other Hyper-V subsystem drivers. Pre-existing issues: - [High] The error path in vmbus_connect() and vmbus_exit() bypasses sending the VMBus disconnect message, risking guest memory corruption. - [High] The error paths in balloon_probe() and balloon_resume() fail to cancel scheduled work items, leading to use-after-free. -- --- Patch [1]: [PATCH v2 1/6] Drivers: hv: Remove support for WS2012/2012R2 & Win8/8.1 version of Hyper-V --- commit 764adf4bdf08e43be943c35b8558657a532f1362 Author: Michael Kelley <[email protected]> Subject: Drivers: hv: Remove support for WS2012/2012R2 & Win8/8.1 version of Hyper-V This commit simplifies Linux code by removing special cases and VMBus protocol version negotiations for running on older versions of Hyper-V (WS2012/2012R2 and Windows 8/8.1), as their official support ended. Note: The format of this report is altered due to recitation restrictions. The original patch code is not quoted directly, but rather a free-form summary of the issues is provided without inline quoting. [Severity: Low] In drivers/hv/hv_balloon.c, within the version_resp() function, the patch hardcodes dm->next_version to 0. Is it necessary to leave the check "if (dm->next_version == 0) goto version_error;" and the subsequent version request packet sending logic intact? Since the downgrade path is removed, this makes the remainder of the function mathematically unreachable dead code. [Severity: Low] In drivers/hv/connection.c, within the vmbus_connect() function, the patch adds a pr_err() call for unsupported VMBus versions. Does the embedded newline and multi-line string literal continuation break kernel log formatting? This can cause the second line to lack a severity prefix and interleave with other logs. [Severity: High] This is a pre-existing issue, but in drivers/hv/connection.c during the vmbus_connect() error path (cleanup label), the vmbus_connection.conn_state is explicitly set to DISCONNECTED before calling vmbus_disconnect(). In drivers/hv/channel_mgmt.c:vmbus_initiate_unload(), the code returns early if the old state is DISCONNECTED or CONNECTING, thereby skipping the vital UNLOAD message. Can this bypass of the VMBus disconnect message cause guest memory corruption? Since vmbus_disconnect() frees int_page and monitor_pages without the host being notified of the teardown, the host might continue to DMA incoming messages to these freed guest physical pages. [Severity: High] This is a pre-existing issue, but in drivers/hv/hv_balloon.c:balloon_probe(), if the kthread_run() fails, the probe function jumps to the probe_error label. This cleans up the VMBus channel but fails to call cancel_work_sync(). Since the host can send a DM_BALLOON_REQUEST which queues balloon_wrk right after balloon_connect_vsp() finishes, can these orphaned background work items execute later and dereference the freed device, leading to a use-after-free? [Severity: Low] The patch series removed legacy VMBus version checks in the core, but did it miss removing them in other Hyper-V subsystem drivers? Drivers like drivers/gpu/drm/hyperv/hyperv_drm_proto.c (in hv_drm_connect_vsp()) and drivers/scsi/storvsc_drv.c (in storvsc_sdev_configure()) still contain switch cases for VERSION_WIN8 and VERSION_WIN8_1 (or VMSTOR_PROTO_VERSION_WIN8). Since vmbus_connect() now rejects hosts not supporting at least VERSION_WIN10, isn't this leftover dead code? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
