Re: [PATCH] mshv_vtl: Check per-CPU register page before mmap

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Returning VM_FAULT_NOPAGE for invalid memory offsets without installing a PTE causes an unkillable infinite page fault loop.
- [High] Per-CPU memory allocated for `run` and `reg_page` is leaked on driver module unload and upon CPU hotplug setup failures.
- [High] Premature character device registration allows userspace to trigger page faults and ioctls before per-CPU data is initialized, bypassing the newly added `if (!page)` check and causing kernel panics or UAF.
--

commit 33a60782bd60657bd70b537be47acdb4ffe78f38
Author: Hardik Garg <hargar@linux.microsoft.com>

mshv_vtl: Check per-CPU register page before mmap

Register-page setup is optional for each CPU. If allocation or
registration fails, mshv_vtl_configure_reg_page() warns and leaves that
CPU's reg_page NULL, but initial context setup continues successfully.
Meanwhile, successful setup on another CPU sets the global
mshv_has_reg_page flag.

mshv_vtl_fault() checks this global flag before selecting the requested
CPU's register page. With mixed setup results across online CPUs, the
check passes even for a CPU with no register page, reaching
get_page(NULL) when userspace faults in that mapping.

Check the selected per-CPU register page before taking its reference and
return VM_FAULT_SIGBUS if it is absent.

Fixes: 7bfe3b8ea6e3 ("Drivers: hv: Introduce mshv_vtl driver")
Cc: stable@vger.kernel.org

> diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
> index 6e3c11c681717..d8225242c73f8 100644
> --- a/drivers/hv/mshv_vtl_main.c
> +++ b/drivers/hv/mshv_vtl_main.c
> @@ -819,6 +819,9 @@ static vm_fault_t mshv_vtl_fault(struct vm_fault *vmf)
>  		return VM_FAULT_NOPAGE;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but returning VM_FAULT_NOPAGE
for an invalid memory offset without installing a PTE causes an unkillable
infinite page fault loop.

If a userspace process maps the /dev/mshv device and accesses an invalid page
offset (neither MSHV_RUN_PAGE_OFFSET nor MSHV_REG_PAGE_OFFSET), this else
branch is taken. Returning VM_FAULT_NOPAGE signals to the core MM subsystem
that the fault handler successfully installed the page table entry, so the
kernel returns to userspace without doing anything. Since no PTE was actually
installed, the CPU will immediately re-fault on the same instruction, creating
a 100% CPU hung task.

Could we return VM_FAULT_SIGBUS here instead?

>  
> +	if (!page)
> +		return VM_FAULT_SIGBUS;

[Severity: High]
This is a pre-existing issue, but premature character device registration
allows userspace to bypass this newly added check.

In mshv_vtl_init(), the character device is exposed to userspace early:

mshv_vtl_init() {
    ...
    ret = misc_register(&mshv_dev);
    ...
}

If a userspace process races the module initialization by opening /dev/mshv
and mapping it before hv_vtl_setup_synic() allocates the per-CPU run page,
mshv_vtl_cpu_run(cpu) will return NULL.

When this happens, virt_to_page(NULL) is called:

mshv_vtl_fault() {
    ...
    if (real_off == MSHV_RUN_PAGE_OFFSET) {
        page = virt_to_page(mshv_vtl_cpu_run(cpu));
    ...
}

This produces a garbage non-NULL pointer, causing the new if (!page) check
to pass, and the subsequent get_page() will dereference the garbage pointer
and panic.

If initialization subsequently fails, the module load aborts and .text is
freed while the racing userspace process retains an open file descriptor,
leading to a use-after-free.

Could the device registration be deferred until after the per-CPU data is
initialized?

> +
>  	get_page(page);
>  	vmf->page = page;
>  

[Severity: High]
This is a pre-existing issue, but does this code leak the per-CPU pages
allocated for run and reg_page on driver module unload and upon CPU hotplug
setup failures?

When setting up the hotplug state, the teardown callback is set to NULL:

hv_vtl_setup_synic() {
    ...
    ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "hyperv/vtl:online",
                            mshv_vtl_alloc_context, NULL);
    ...
}

When the module unloads, cpuhp_remove_state() is called, but without a
teardown callback, the memory permanently allocated via __get_free_page()
in mshv_vtl_alloc_context() is leaked:

mshv_vtl_alloc_context() {
    ...
    per_cpu->run = (struct mshv_vtl_run *)
        __get_free_page(GFP_KERNEL | __GFP_ZERO);
    ...
}

Additionally, if the hotplug state setup fails midway, the successfully
processed CPUs will not be cleaned up.

Could a teardown callback be provided to free these pages?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911175146.1370148-1-hargar@linux.microsoft.com?part=1