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

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

[Yousef Alhouseen]

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.

Allocate the output page zeroed before issuing the hypercall. 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@gmail.com>
---
 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);
 
 	return ret;
 }
-- 
2.54.0

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

[Michael Kelley]

From: Yousef Alhouseen <alhouseenyousef@gmail.com> 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@linux.microsoft.com/

> 
> 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@gmail.com>
> ---
>  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
> 

[PATCH v2] mshv_vtl: clear hypercall output before copyout

[Yousef Alhouseen]

mshv_vtl_hvcall_call() copies output_size bytes to userspace.

The output page is freshly allocated. Userspace chooses the copyout length.

If the hypercall writes less, the tail can contain stale page data.

Clear the copied range before issuing the hypercall.

Also check both bounce page allocations before either page is used.

Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
Changes in v2:
- Use the mshv_vtl subject prefix.
- Clear only the requested output byte range instead of the whole page.
- Add a comment explaining why the output range is cleared.
- Keep free_page() calls unconditional.
- v1: https://lore.kernel.org/r/20260624172157.2790-1-alhouseenyousef@gmail.com

 drivers/hv/mshv_vtl_main.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
index 0d3d41619..dbf03b667 100644
--- a/drivers/hv/mshv_vtl_main.c
+++ b/drivers/hv/mshv_vtl_main.c
@@ -1148,12 +1148,22 @@ static int mshv_vtl_hvcall_call(struct mshv_vtl_hvcall_fd *fd,
 	 */
 	in = (void *)__get_free_page(GFP_KERNEL);
 	out = (void *)__get_free_page(GFP_KERNEL);
+	if (!in || !out) {
+		ret = -ENOMEM;
+		goto free_pages;
+	}
 
 	if (copy_from_user(in, (void __user *)hvcall.input_ptr, hvcall.input_size)) {
 		ret = -EFAULT;
 		goto free_pages;
 	}
 
+	/*
+	 * The caller supplies output_size, so clear the range copied back to
+	 * userspace in case the hypercall writes fewer bytes than requested.
+	 */
+	memset(out, 0, hvcall.output_size);
+
 	hvcall.status = hv_do_hypercall(hvcall.control, in, out);
 
 	if (copy_to_user((void __user *)hvcall.output_ptr, out, hvcall.output_size)) {
-- 
2.54.0

RE: [PATCH v2] mshv_vtl: clear hypercall output before copyout

[Michael Kelley]

From: Yousef Alhouseen <alhouseenyousef@gmail.com> Sent: Thursday, June 25, 2026 11:13 AM
> 
> mshv_vtl_hvcall_call() copies output_size bytes to userspace.
> 
> The output page is freshly allocated. Userspace chooses the copyout length.
> 
> If the hypercall writes less, the tail can contain stale page data.
> 
> Clear the copied range before issuing the hypercall.
> 
> Also check both bounce page allocations before either page is used.
> 
> Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
> ---
> Changes in v2:
> - Use the mshv_vtl subject prefix.
> - Clear only the requested output byte range instead of the whole page.
> - Add a comment explaining why the output range is cleared.
> - Keep free_page() calls unconditional.
> - v1: https://lore.kernel.org/all/20260624172157.2790-1-alhouseenyousef@gmail.com/ 
> 
>  drivers/hv/mshv_vtl_main.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
> index 0d3d41619..dbf03b667 100644
> --- a/drivers/hv/mshv_vtl_main.c
> +++ b/drivers/hv/mshv_vtl_main.c
> @@ -1148,12 +1148,22 @@ static int mshv_vtl_hvcall_call(struct mshv_vtl_hvcall_fd *fd,
>  	 */
>  	in = (void *)__get_free_page(GFP_KERNEL);
>  	out = (void *)__get_free_page(GFP_KERNEL);
> +	if (!in || !out) {
> +		ret = -ENOMEM;
> +		goto free_pages;
> +	}
> 
>  	if (copy_from_user(in, (void __user *)hvcall.input_ptr, hvcall.input_size)) {
>  		ret = -EFAULT;
>  		goto free_pages;
>  	}
> 
> +	/*
> +	 * The caller supplies output_size, so clear the range copied back to
> +	 * userspace in case the hypercall writes fewer bytes than requested.
> +	 */
> +	memset(out, 0, hvcall.output_size);
> +
>  	hvcall.status = hv_do_hypercall(hvcall.control, in, out);
> 
>  	if (copy_to_user((void __user *)hvcall.output_ptr, out, hvcall.output_size)) {
> --
> 2.54.0

Reviewed-by: Michael Kelley <mhklinux@outlook.com>