[PATCH] mshv: bounds-check cpu index in vtl mmap fault handler

[PATCH] mshv: bounds-check cpu index in vtl mmap fault handler

[Yi Xie]

cpu is taken from pgoff & 0xffff.  cpu_online() does not reject cpu >=
nr_cpu_ids, and per_cpu_ptr() can then walk off __per_cpu_offset.

Signed-off-by: Yi Xie <xieyi@kylinos.cn>
---
 drivers/hv/mshv_vtl_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
index 0d3d4161974f..fc50c44ac1bd 100644
--- a/drivers/hv/mshv_vtl_main.c
+++ b/drivers/hv/mshv_vtl_main.c
@@ -801,7 +801,7 @@ static vm_fault_t mshv_vtl_fault(struct vm_fault *vmf)
 	int cpu = vmf->pgoff & MSHV_PG_OFF_CPU_MASK;
 	int real_off = vmf->pgoff >> MSHV_REAL_OFF_SHIFT;
 
-	if (!cpu_online(cpu))
+	if (cpu >= nr_cpu_ids || !cpu_online(cpu))
 		return VM_FAULT_SIGBUS;
 	/*
 	 * CPU Hotplug is not supported in VTL2 in OpenHCL, where this kernel driver exists.
-- 
2.34.1

Re: [PATCH] mshv: bounds-check cpu index in vtl mmap fault handler

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

Pre-existing issues:
- [High] Returning VM_FAULT_NOPAGE for unmapped/invalid offsets in mshv_vtl_fault creates an infinite page fault loop.
- [High] A partial allocation failure of `reg_page` on a secondary CPU leads to a NULL pointer dereference upon mmap fault, due to global state mismatch.
- [High] Registering the character device before CPU contexts are initialized exposes uninitialized memory or causes kernel panics.
- [High] The module leaks an eventfd wait queue entry, leading to a guaranteed Use-After-Free (UAF) upon module unload.
- [High] Missing allocation failure checks lead to a NULL pointer dereference in `mshv_vtl_hvcall_call`.
- [Medium] The CPU bounds check in `mshv_vtl_ioctl_set_poll_file` uses `num_possible_cpus()` instead of `nr_cpu_ids`, potentially rejecting valid CPUs.
--

--- Patch [1]: [PATCH] mshv: bounds-check cpu index in vtl mmap fault handler ---
commit 8ac0cc752b2b67def52fafd91c16c26937bc5c16
Author: Yi Xie <xieyi@kylinos.cn>

mshv: bounds-check cpu index in vtl mmap fault handler

cpu is taken from pgoff & 0xffff.  cpu_online() does not reject cpu >=
nr_cpu_ids, and per_cpu_ptr() can then walk off __per_cpu_offset.

> diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
> index 0d3d4161974f8..fc50c44ac1bda 100644
> --- a/drivers/hv/mshv_vtl_main.c
> +++ b/drivers/hv/mshv_vtl_main.c
> @@ -801,7 +801,7 @@ static vm_fault_t mshv_vtl_fault(struct vm_fault *vmf)
>  	int cpu = vmf->pgoff & MSHV_PG_OFF_CPU_MASK;
>  	int real_off = vmf->pgoff >> MSHV_REAL_OFF_SHIFT;
>  
> -	if (!cpu_online(cpu))
> +	if (cpu >= nr_cpu_ids || !cpu_online(cpu))
>  		return VM_FAULT_SIGBUS;

[Severity: High]
This is a pre-existing issue, but returning VM_FAULT_NOPAGE for unmapped or
invalid offsets in mshv_vtl_fault() can create an infinite page fault loop:

drivers/hv/mshv_vtl_main.c:mshv_vtl_fault() {
...
	if (real_off == MSHV_RUN_PAGE_OFFSET) {
		page = virt_to_page(mshv_vtl_cpu_run(cpu));
	} else if (real_off == MSHV_REG_PAGE_OFFSET) {
		if (!mshv_has_reg_page)
			return VM_FAULT_SIGBUS;
		page = mshv_vtl_cpu_reg_page(cpu);
	} else {
		return VM_FAULT_NOPAGE;
	}
...

Since VM_FAULT_NOPAGE indicates the fault was successfully handled without
installing a PTE, won't the CPU infinitely re-fault on the same instruction
and lock up the core if userspace accesses an unmapped offset? Should this
return VM_FAULT_SIGBUS instead?


[Severity: High]
This is a pre-existing issue, but does a partial allocation failure on a
secondary CPU lead to a NULL pointer dereference upon mmap fault?

If alloc_page() fails in mshv_vtl_configure_reg_page() for a specific CPU,
it returns without initializing the per-CPU reg_page:

drivers/hv/mshv_vtl_main.c:mshv_vtl_configure_reg_page() {
...
	reg_page = alloc_page(GFP_KERNEL | __GFP_ZERO | __GFP_RETRY_MAYFAIL);
	if (!reg_page) {
		WARN(1, "failed to allocate register page\n");
		return;
	}
...

However, the fault handler relies on the global mshv_has_reg_page flag
instead of checking if the per-CPU page is actually valid:

drivers/hv/mshv_vtl_main.c:mshv_vtl_fault() {
...
	} else if (real_off == MSHV_REG_PAGE_OFFSET) {
		if (!mshv_has_reg_page)
			return VM_FAULT_SIGBUS;
		page = mshv_vtl_cpu_reg_page(cpu);
	}
...
	get_page(page);

If the global flag is set by another CPU's successful allocation, won't
mshv_vtl_fault() bypass the safety check and call get_page(NULL) for the
failing CPU?


[Severity: High]
This is a pre-existing issue, but does registering the character device
before CPU contexts are initialized expose uninitialized memory?

drivers/hv/mshv_vtl_main.c:mshv_vtl_init() {
...
	ret = misc_register(&mshv_dev);
	if (ret) {
...
	ret = hv_vtl_setup_synic();
...

Since misc_register() exposes /dev/mshv to userspace before
hv_vtl_setup_synic() allocates the per-CPU run and reg_page pointers, can a
fast userspace process mmap the device and trigger a page fault resulting in
virt_to_page(NULL) or get_page(NULL)?


[Severity: Medium]
This is a pre-existing issue, but does the bounds check in
mshv_vtl_ioctl_set_poll_file() incorrectly reject valid CPUs on sparse
topologies?

drivers/hv/mshv_vtl_main.c:mshv_vtl_ioctl_set_poll_file() {
...
	if (input.cpu >= num_possible_cpus() || !cpu_online(input.cpu))
		return -EINVAL;
...

If the system has a sparse CPU possible mask, num_possible_cpus() could be
lower than a valid CPU ID. Should this use nr_cpu_ids instead?


[Severity: High]
This is a pre-existing issue, but does the module leak an eventfd wait queue
entry leading to a use-after-free upon module unload?

drivers/hv/mshv_vtl_main.c:mshv_vtl_ioctl_set_poll_file() {
...
	if (file) {
		init_waitqueue_func_entry(&poll_file->wait, mshv_vtl_poll_file_wake);
		init_poll_funcptr(&poll_file->pt, mshv_vtl_ptable_queue_proc);
		vfs_poll(file, &poll_file->pt);
	}
...

drivers/hv/mshv_vtl_main.c:mshv_vtl_exit() {
...
	device_del(mem_dev);
	kfree(mem_dev);
	misc_deregister(&mshv_vtl_low);
...

The MSHV_SET_POLL_FILE ioctl permanently adds the global per-CPU
poll_file->wait entry into the user-provided eventfd's wait queue. Since
mshv_vtl_exit() doesn't remove the wait queue entry or release the eventfd
reference, won't signaling the eventfd after module unload cause the kernel
to traverse the wait queue and dereference freed memory?


[Severity: High]
This is a pre-existing issue, but are missing allocation failure checks
causing a NULL pointer dereference in mshv_vtl_hvcall_call()?

drivers/hv/mshv_vtl_main.c:mshv_vtl_hvcall_call() {
...
	in = (void *)__get_free_page(GFP_KERNEL);
	out = (void *)__get_free_page(GFP_KERNEL);

	if (copy_from_user(in, (void __user *)hvcall.input_ptr, hvcall.input_size)) {
...

If __get_free_page() returns 0 (NULL) due to memory pressure, the
subsequent copy_from_user() will attempt to copy user-supplied data
to a NULL pointer. Shouldn't the return values of __get_free_page() be
validated?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709021947.49436-1-xieyi@kylinos.cn?part=1

Re: [PATCH] mshv: bounds-check cpu index in vtl mmap fault handler

[Wei Liu]

Naman, Saurabh, please review this.

On Thu, Jul 09, 2026 at 10:19:47AM +0800, Yi Xie wrote:
> cpu is taken from pgoff & 0xffff.  cpu_online() does not reject cpu >=
> nr_cpu_ids, and per_cpu_ptr() can then walk off __per_cpu_offset.
> 
> Signed-off-by: Yi Xie <xieyi@kylinos.cn>
> ---
>  drivers/hv/mshv_vtl_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
> index 0d3d4161974f..fc50c44ac1bd 100644
> --- a/drivers/hv/mshv_vtl_main.c
> +++ b/drivers/hv/mshv_vtl_main.c
> @@ -801,7 +801,7 @@ static vm_fault_t mshv_vtl_fault(struct vm_fault *vmf)
>  	int cpu = vmf->pgoff & MSHV_PG_OFF_CPU_MASK;
>  	int real_off = vmf->pgoff >> MSHV_REAL_OFF_SHIFT;
>  
> -	if (!cpu_online(cpu))
> +	if (cpu >= nr_cpu_ids || !cpu_online(cpu))
>  		return VM_FAULT_SIGBUS;
>  	/*
>  	 * CPU Hotplug is not supported in VTL2 in OpenHCL, where this kernel driver exists.
> -- 
> 2.34.1

Re: [PATCH] mshv: bounds-check cpu index in vtl mmap fault handler

[Naman Jain]

On 7/9/2026 7:49 AM, Yi Xie wrote:
> cpu is taken from pgoff & 0xffff.  cpu_online() does not reject cpu >=
> nr_cpu_ids, and per_cpu_ptr() can then walk off __per_cpu_offset.
> 
> Signed-off-by: Yi Xie <xieyi@kylinos.cn>
> ---
>   drivers/hv/mshv_vtl_main.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
> index 0d3d4161974f..fc50c44ac1bd 100644
> --- a/drivers/hv/mshv_vtl_main.c
> +++ b/drivers/hv/mshv_vtl_main.c
> @@ -801,7 +801,7 @@ static vm_fault_t mshv_vtl_fault(struct vm_fault *vmf)
>   	int cpu = vmf->pgoff & MSHV_PG_OFF_CPU_MASK;
>   	int real_off = vmf->pgoff >> MSHV_REAL_OFF_SHIFT;
>   
> -	if (!cpu_online(cpu))
> +	if (cpu >= nr_cpu_ids || !cpu_online(cpu))
>   		return VM_FAULT_SIGBUS;
>   	/*
>   	 * CPU Hotplug is not supported in VTL2 in OpenHCL, where this kernel driver exists.

The problem fixed by this patch generally does not happen in practice as 
the user space is trusted user space (OpenVMM). Nevertheless, it's good 
to have this check.

Nit: subject - s/"mshv:"/"mshv_vtl:"
as this was the agreed upon prefix for changes to mshv_vtl_main driver.

Reviewed-by: Naman Jain <namjain@linux.microsoft.com>