Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues: - [High] Intentional breakage of core driver functionality, boot crashes, and UAPI violation via hardcoded error stubs. Pre-existing issues: - [Critical] A partial failure during the HVCALL_MAP_GPA_PAGES batch processing fails to unmap successfully mapped pages, leaving the guest with valid GPA mappings to host pages that are subsequently unpinned and reallocated. - [Critical] A partial failure during HVCALL_RELEASE_SPARSE_SPA_PAGE_HOST_ACCESS causes the host to permanently lose access to pinned memory, leading to a host kernel crash when the pages are unpinned and reused. -- --- Patch [1]: [PATCH V0 1/2] mshv: Stub out deposit memory functions --- commit 31673114cbc4abf8a6e554e641203a81af3ed70c Author: Mukesh R <[email protected]> mshv: Stub out deposit memory functions This commit replaces the implementations of hv_call_deposit_pages() and hv_deposit_memory_node() with stubs that unconditionally return -ENOTSUPP. The commit message states this is to make adding new redesigned functions easier to review. > diff --git a/drivers/hv/hv_proc.c b/drivers/hv/hv_proc.c > index 57b2c64197cb9..d5ce9a032e9c7 100644 > --- a/drivers/hv/hv_proc.c > +++ b/drivers/hv/hv_proc.c > @@ -9,135 +9,15 @@ > #include <linux/export.h> > #include <asm/mshyperv.h> > [ ... ] > int hv_call_deposit_pages(int node, u64 partition_id, u32 num_pages) > { [ ... ] > -free_buf: > - free_page((unsigned long)pages); > - kfree(counts); > - return ret; > + return -ENOTSUPP; > } [Severity: High] Does unconditionally returning -ENOTSUPP here break core driver functionality and cause boot failures? If hv_smp_prepare_cpus() in arch/x86/kernel/cpu/mshyperv.c requires memory during early CPU setup, the -ENOTSUPP return will trigger a BUG_ON(ret), resulting in a fatal early boot crash. Additionally, hv_call_initialize_partition() in drivers/hv/mshv_root_hv_call.c unconditionally calls this function. Returning -ENOTSUPP (which is internal to the kernel) will propagate up to the mshv_partition_ioctl return value and leak to userspace as an unrecognized error, while guaranteeing partition creation always fails. [Severity: Critical] This is a pre-existing issue, but I noticed a potential problem in drivers/hv/mshv_root_hv_call.c:hv_do_map_gpa_hcall(): } else if (!hv_result_success(status)) { ret = hv_result_to_errno(status); break; } done += completed; If a partial failure occurs during the HVCALL_MAP_GPA_PAGES batch processing, the break statement skips the done += completed update. Could this permanently lose track of the successfully mapped pages in this batch? If the stale done value is used during cleanup, successfully mapped pages would not be unmapped, leaving the guest with valid GPA mappings to host pages that are subsequently unpinned and reallocated. [Severity: Critical] This is a pre-existing issue, but there appears to be another error handling concern in drivers/hv/mshv_root_hv_call.c:hv_call_modify_spa_host_access(): completed = hv_repcomp(status); if (!hv_result_success(status)) return hv_result_to_errno(status); done += completed; If this hypercall partially succeeds but returns an error for a subsequent page, the function returns immediately and discards the completed count. Can this prevent the caller from knowing which pages need their host access restored? If the caller treats the region as failed and unpins the pages without restoring host access, the host kernel allocator could reuse pages that lack host access. This could lead to a hardware-level access violation and a fatal kernel panic when the host accesses them. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
