* [PATCH] mshv_vtl: Check per-CPU register page before mmap
@ 2026-09-11 17:51 Hardik Garg
2026-09-11 18:05 ` sashiko-bot
0 siblings, 1 reply; 3+ messages in thread
From: Hardik Garg @ 2026-09-11 17:51 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui
Cc: mhklinux, ssengar, namjain, romank, linux-hyperv, linux-kernel,
stable
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
Signed-off-by: Hardik Garg <hargar@linux.microsoft.com>
---
drivers/hv/mshv_vtl_main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
index fc993560a45c..3296ab5c0243 100644
--- a/drivers/hv/mshv_vtl_main.c
+++ b/drivers/hv/mshv_vtl_main.c
@@ -912,6 +912,9 @@ static vm_fault_t mshv_vtl_fault(struct vm_fault *vmf)
return VM_FAULT_NOPAGE;
}
+ if (!page)
+ return VM_FAULT_SIGBUS;
+
get_page(page);
vmf->page = page;
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] mshv_vtl: Check per-CPU register page before mmap 2026-09-11 17:51 [PATCH] mshv_vtl: Check per-CPU register page before mmap Hardik Garg @ 2026-09-11 18:05 ` sashiko-bot 2026-09-11 18:19 ` Hardik Garg 0 siblings, 1 reply; 3+ messages in thread From: sashiko-bot @ 2026-09-11 18:05 UTC (permalink / raw) To: Hardik Garg; +Cc: linux-hyperv 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mshv_vtl: Check per-CPU register page before mmap 2026-09-11 18:05 ` sashiko-bot @ 2026-09-11 18:19 ` Hardik Garg 0 siblings, 0 replies; 3+ messages in thread From: Hardik Garg @ 2026-09-11 18:19 UTC (permalink / raw) To: sashiko-reviews; +Cc: linux-hyperv On 9/11/2026 11:05 AM, sashiko-bot@kernel.org wrote: > 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. > -- All the reported issues are pre-existing and not related to this patch. I will review the issues and fix them in a separate patch. > 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? > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-11 18:19 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-11 17:51 [PATCH] mshv_vtl: Check per-CPU register page before mmap Hardik Garg 2026-09-11 18:05 ` sashiko-bot 2026-09-11 18:19 ` Hardik Garg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox