* 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
2026-09-15 4:41 ` Naman Jain
2026-09-22 0:14 ` Michael Kelley
2 siblings, 1 reply; 7+ 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] 7+ 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; 7+ 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] 7+ 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-15 4:41 ` Naman Jain
2026-09-19 23:02 ` Wei Liu
2026-09-22 0:14 ` Michael Kelley
2 siblings, 1 reply; 7+ messages in thread
From: Naman Jain @ 2026-09-15 4:41 UTC (permalink / raw)
To: Hardik Garg, kys, haiyangz, wei.liu, decui
Cc: mhklinux, ssengar, romank, linux-hyperv, linux-kernel, stable
On 9/11/2026 11:21 PM, Hardik Garg wrote:
> 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;
>
LGTM. Thanks.
Reviewed-by: Naman Jain <namjain@linux.microsoft.com>
Regards,
Naman
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] mshv_vtl: Check per-CPU register page before mmap
2026-09-15 4:41 ` Naman Jain
@ 2026-09-19 23:02 ` Wei Liu
0 siblings, 0 replies; 7+ messages in thread
From: Wei Liu @ 2026-09-19 23:02 UTC (permalink / raw)
To: Naman Jain
Cc: Hardik Garg, kys, haiyangz, wei.liu, decui, mhklinux, ssengar,
romank, linux-hyperv, linux-kernel, stable
On Tue, Sep 15, 2026 at 10:11:36AM +0530, Naman Jain wrote:
>
>
> On 9/11/2026 11:21 PM, Hardik Garg wrote:
> > 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;
>
>
> LGTM. Thanks.
>
> Reviewed-by: Naman Jain <namjain@linux.microsoft.com>
Applied. Thanks.
^ permalink raw reply [flat|nested] 7+ 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-15 4:41 ` Naman Jain
@ 2026-09-22 0:14 ` Michael Kelley
2026-09-24 19:19 ` Hardik Garg
2 siblings, 1 reply; 7+ messages in thread
From: Michael Kelley @ 2026-09-22 0:14 UTC (permalink / raw)
To: Hardik Garg, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com
Cc: ssengar@linux.microsoft.com, namjain@linux.microsoft.com,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
From: Hardik Garg <hargar@linux.microsoft.com>
>
> 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.
I'm a bit late in reviewing this because I was travelling all last week.
This patch seems like it is just papering over the real problem, which
is that a global variable like mshv_has_reg_page can't represent the
status of an operation that may succeed or fail on a per-cpu basis.
I pointed this out (as did Sashiko) in review comments back in
April [1].
The only other place mshv_has_reg_page is used is in
mshv_ioctl_check_extensions() where its value is returned from an
ioctl() system call made by user space. That's a questionable practice
since ioctl() usually returns 0 on success, though the man page for
ioctl() does admit that some ioctls use the return value as an output
parameter and return a non-negative value on success. But even
then, a single value can't accurately reflect the status of an
operation that may succeed or fail on a per-cpu basis. User
space would presumably have a similar problem to what is
being fixed by this patch.
Is there any reason that the real problem couldn't be fixed
instead of doing this fix on top of something that is fundamentally
broken?
Michael
[1] https://lore.kernel.org/linux-hyperv/SN6PR02MB4157CF364DA2C0CC657A6DCBD450A@SN6PR02MB4157.namprd02.prod.outlook.com/
>
> 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;
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] mshv_vtl: Check per-CPU register page before mmap
2026-09-22 0:14 ` Michael Kelley
@ 2026-09-24 19:19 ` Hardik Garg
0 siblings, 0 replies; 7+ messages in thread
From: Hardik Garg @ 2026-09-24 19:19 UTC (permalink / raw)
To: Michael Kelley, kys@microsoft.com, haiyangz@microsoft.com,
wei.liu@kernel.org, decui@microsoft.com
Cc: ssengar@linux.microsoft.com, namjain@linux.microsoft.com,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
On 9/21/2026 5:14 PM, Michael Kelley wrote:
> From: Hardik Garg <hargar@linux.microsoft.com>
>> 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.
> I'm a bit late in reviewing this because I was travelling all last week.
>
> This patch seems like it is just papering over the real problem, which
> is that a global variable like mshv_has_reg_page can't represent the
> status of an operation that may succeed or fail on a per-cpu basis.
> I pointed this out (as did Sashiko) in review comments back in
> April [1].
>
> The only other place mshv_has_reg_page is used is in
> mshv_ioctl_check_extensions() where its value is returned from an
> ioctl() system call made by user space. That's a questionable practice
> since ioctl() usually returns 0 on success, though the man page for
> ioctl() does admit that some ioctls use the return value as an output
> parameter and return a non-negative value on success. But even
> then, a single value can't accurately reflect the status of an
> operation that may succeed or fail on a per-cpu basis. User
> space would presumably have a similar problem to what is
> being fixed by this patch.
>
> Is there any reason that the real problem couldn't be fixed
> instead of doing this fix on top of something that is fundamentally
> broken?
>
> Michael
>
> [1] https://lore.kernel.org/linux-hyperv/SN6PR02MB4157CF364DA2C0CC657A6DCBD450A@SN6PR02MB4157.namprd02.prod.outlook.com/
Hi Michael,
Thanks for the feedback and for pointing me to your April review.
After going through it, I understand that the NULL check only addresses
the immediate fault, while the global capability still misrepresents
per-CPU availability.
I'll work on addressing the underlying issue and send a proposed fix
soon.
Thanks,
Hardik
>> 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;
>>
^ permalink raw reply [flat|nested] 7+ messages in thread