From: Yousef Alhouseen <[email protected]> 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/[email protected]/ > > 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 <[email protected]> > --- > 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 >
