On 9/11/2026 11:05 AM, [email protected] 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 <[email protected]> > > 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: [email protected] > >> 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? >
