Re: [PATCH V0 1/2] mshv: Stub out deposit memory functions

[sashiko-bot@kernel.org]

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 <mrathor@linux.microsoft.com>

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/20260812000507.3809046-1-mrathor@linux.microsoft.com?part=1