RE: [PATCH] hyperv: mshv: zero VTL hypercall output page

[Michael Kelley]

From: Yousef Alhouseen <alhouseenyousef@xxxxxxxxx> Sent: Wednesday, June 24, 2026 10:22 AM

> Subject: [PATCH] hyperv: mshv: zero VTL hypercall output page

There was a recent discussion about what prefix to use in the patch
"Subject:" field for changes to MSHV VTL code. The agreement was to
use just "mshv_vtl:". See [1].

[1] https://lore.kernel.org/linux-hyperv/a0d271e3-ece8-45cf-9dbb-ced773d6f3f8@xxxxxxxxxxxxxxxxxxx/

> 
> mshv_vtl_hvcall_call() copies output_size bytes from a freshly allocated
> hypercall output page back to userspace. The page is currently allocated
> without __GFP_ZERO, so any bytes not written by the hypervisor are copied
> from stale page contents.

This is a good find! Even though the VTL user space code is somewhat trusted,
there should not be any circumstances where the kernel could copy random
garbage to user space.

> 
> Allocate the output page zeroed before issuing the hypercall.

Hypercall output is usually no more than a few tens of bytes. Zeroing
the entire page is a bit expensive. It would be sufficient to just zero
output_size bytes.

Standard practice is to *not* zero to the hypercall output area, since
the hypercall invoker knows exactly how many bytes Hyper-V will
return for a particular hypercall, and Hyper-V is responsible for not
leaving any garbage. So it would be good to leave a code comment
here about why the output area is being zero'ed contrary to that
standard practice.

I would note that many hypercalls don't return any output other
than the hypercall status. If output_size is zero, allocating the
output page could be skipped. But that's a further
optimization for another patch.

> Also check
> both bounce-page allocations before using them so memory pressure cannot
> turn the copy paths into NULL pointer dereferences.
> 
> Signed-off-by: Yousef Alhouseen <alhouseenyousef@xxxxxxxxx>
> ---
>  drivers/hv/mshv_vtl_main.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
> index 0d3d41619..0365d207c 100644
> --- a/drivers/hv/mshv_vtl_main.c
> +++ b/drivers/hv/mshv_vtl_main.c
> @@ -1147,7 +1147,11 @@ static int mshv_vtl_hvcall_call(struct mshv_vtl_hvcall_fd *fd,
>  	 * TODO: Take care of this when CVM support is added.
>  	 */
>  	in = (void *)__get_free_page(GFP_KERNEL);
> -	out = (void *)__get_free_page(GFP_KERNEL);
> +	out = (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO);
> +	if (!in || !out) {
> +		ret = -ENOMEM;
> +		goto free_pages;
> +	}
> 
>  	if (copy_from_user(in, (void __user *)hvcall.input_ptr, hvcall.input_size)) {
>  		ret = -EFAULT;
> @@ -1162,8 +1166,10 @@ static int mshv_vtl_hvcall_call(struct mshv_vtl_hvcall_fd *fd,
>  	}
>  	ret = put_user(hvcall.status, &hvcall_user->status);
>  free_pages:
> -	free_page((unsigned long)in);
> -	free_page((unsigned long)out);
> +	if (in)
> +		free_page((unsigned long)in);
> +	if (out)
> +		free_page((unsigned long)out);

Testing "in" and "out" here isn't necessary. free_page()
already has code to do nothing if its argument is zero.

Michael 

> 
>  	return ret;
>  }
> --
> 2.54.0
>