[PATCH] KVM: clear map->gfn in kvm_vcpu_unmap() to prevent stale validity checks

[PATCH] KVM: clear map->gfn in kvm_vcpu_unmap() to prevent stale validity checks

[Taeyang Lee]

kvm_vcpu_unmap() clears map->hva and map->page but leaves map->gfn with
its previous value. This creates an inconsistent state: callers that
check gfn != 0 as a proxy for map validity will believe the map is still
valid when hva is already NULL.

This pattern caused a null pointer dereference in the 6.1.x LTS branch,
where vmx_guest_apic_has_interrupt() checked virtual_apic_map.gfn but
dereferenced virtual_apic_map.hva unconditionally. That specific call
site no longer exists in mainline due to the gfn_to_pfn_cache
refactoring, but the inconsistency in kvm_vcpu_unmap() remains and could
affect future kvm_host_map users that rely on gfn for validity.

Similarly, kvm_vcpu_map() does not modify the map struct on failure, so
stale gfn values from a previous successful mapping survive a failed
remap attempt. Clearing gfn in kvm_vcpu_unmap() ensures that after an
unmap-then-failed-remap sequence, gfn correctly reflects that no valid
mapping exists.

Clear map->gfn in kvm_vcpu_unmap().

Reported-by: Taeyang Lee <0wn@theori.io>
Fixes: e45adf665a53 ("KVM: Introduce a new guest mapping API")
Signed-off-by: Taeyang Lee <0wn@theori.io>
---
 virt/kvm/kvm_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 7a4fd1dbe0d7..88fc8b20aa8f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2887,6 +2887,7 @@ void kvm_vcpu_unmap(struct kvm_vcpu *vcpu, struct kvm_host_map *map, bool dirty)
 
 	map->hva = NULL;
 	map->page = NULL;
+	map->gfn = 0;
 }
 EXPORT_SYMBOL_GPL(kvm_vcpu_unmap);
 
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] KVM: clear map->gfn in kvm_vcpu_unmap() to prevent stale validity checks

[Sean Christopherson]

On Wed, Mar 11, 2026, Taeyang Lee wrote:
> kvm_vcpu_unmap() clears map->hva and map->page but leaves map->gfn with
> its previous value. This creates an inconsistent state: callers that
> check gfn != 0 as a proxy for map validity will believe the map is still

But that was and still is flat out wrong, '0' is a perfectly legal gfn.

> valid when hva is already NULL.
> 
> This pattern caused a null pointer dereference in the 6.1.x LTS branch,
> where vmx_guest_apic_has_interrupt() checked virtual_apic_map.gfn but
> dereferenced virtual_apic_map.hva unconditionally. That specific call
> site no longer exists in mainline due to the gfn_to_pfn_cache
> refactoring, but the inconsistency in kvm_vcpu_unmap() remains and could
> affect future kvm_host_map users that rely on gfn for validity.
> 
> Similarly, kvm_vcpu_map() does not modify the map struct on failure, so
> stale gfn values from a previous successful mapping survive a failed
> remap attempt. Clearing gfn in kvm_vcpu_unmap() ensures that after an
> unmap-then-failed-remap sequence, gfn correctly reflects that no valid
> mapping exists.
> 
> Clear map->gfn in kvm_vcpu_unmap().
> 
> Reported-by: Taeyang Lee <0wn@theori.io>
> Fixes: e45adf665a53 ("KVM: Introduce a new guest mapping API")

This is not the correct fixes, the blame lies squarely on 96c66e87deee ("KVM/nVMX:
Use kvm_vcpu_map when mapping the virtual APIC page").  The API even provided
kvm_vcpu_mapped() from the get-go; why commit e45adf665a53 checked the gfn, I have
no idea.

> Signed-off-by: Taeyang Lee <0wn@theori.io>
> ---
>  virt/kvm/kvm_main.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 7a4fd1dbe0d7..88fc8b20aa8f 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2887,6 +2887,7 @@ void kvm_vcpu_unmap(struct kvm_vcpu *vcpu, struct kvm_host_map *map, bool dirty)
>  
>  	map->hva = NULL;
>  	map->page = NULL;
> +	map->gfn = 0;

As above, this is at best misleading.  If we wanted to harden this, we should do

	map->gfn = INVALID_GPA;

but I honestly don't see the point.  Yes, we had terrible code in the past, but
the above doesn't prevent terrible code.  E.g. the correct invalidation wouldn't
actually do anything to make the terrible code less broken.

We even (somewhat unintentionally) removed that terrible code in commit 321ef62b0c5f
("KVM: nVMX: Fold requested virtual interrupt check into has_nested_events()").
And that commit was tagged for stable@, it just didn't apply.  If we want to do
anything for 6.1.y (and 6.6.y?), we should backport that entire series:

  https://lore.kernel.org/all/2024072925-straw-mashing-54f6@gregkh

Re: [PATCH] KVM: clear map->gfn in kvm_vcpu_unmap() to prevent stale validity checks

[Taeyang Lee]

Hi Sean,

Thanks for the detailed review.

On Wed, Mar 11, 2026 at 10:53 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Wed, Mar 11, 2026, Taeyang Lee wrote:
> > kvm_vcpu_unmap() clears map->hva and map->page but leaves map->gfn with
> > its previous value. This creates an inconsistent state: callers that
> > check gfn != 0 as a proxy for map validity will believe the map is still
>
> But that was and still is flat out wrong, '0' is a perfectly legal gfn.

You're right, I hadn't considered that. Setting gfn = 0 as a sentinel
is fundamentally incorrect.

>
> > valid when hva is already NULL.
> >
> > This pattern caused a null pointer dereference in the 6.1.x LTS branch,
> > where vmx_guest_apic_has_interrupt() checked virtual_apic_map.gfn but
> > dereferenced virtual_apic_map.hva unconditionally. That specific call
> > site no longer exists in mainline due to the gfn_to_pfn_cache
> > refactoring, but the inconsistency in kvm_vcpu_unmap() remains and could
> > affect future kvm_host_map users that rely on gfn for validity.
> >
> > Similarly, kvm_vcpu_map() does not modify the map struct on failure, so
> > stale gfn values from a previous successful mapping survive a failed
> > remap attempt. Clearing gfn in kvm_vcpu_unmap() ensures that after an
> > unmap-then-failed-remap sequence, gfn correctly reflects that no valid
> > mapping exists.
> >
> > Clear map->gfn in kvm_vcpu_unmap().
> >
> > Reported-by: Taeyang Lee <0wn@theori.io>
> > Fixes: e45adf665a53 ("KVM: Introduce a new guest mapping API")
>
> This is not the correct fixes, the blame lies squarely on 96c66e87deee ("KVM/nVMX:
> Use kvm_vcpu_map when mapping the virtual APIC page").  The API even provided
> kvm_vcpu_mapped() from the get-go; why commit e45adf665a53 checked the gfn, I have
> no idea.
>

Agreed — the bug is in the caller misusing gfn instead of
kvm_vcpu_mapped(), not in the unmap API itself.

> > Signed-off-by: Taeyang Lee <0wn@theori.io>
> > ---
> >  virt/kvm/kvm_main.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 7a4fd1dbe0d7..88fc8b20aa8f 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -2887,6 +2887,7 @@ void kvm_vcpu_unmap(struct kvm_vcpu *vcpu, struct kvm_host_map *map, bool dirty)
> >
> >       map->hva = NULL;
> >       map->page = NULL;
> > +     map->gfn = 0;
>
> As above, this is at best misleading.  If we wanted to harden this, we should do
>
>         map->gfn = INVALID_GPA;
>
> but I honestly don't see the point.  Yes, we had terrible code in the past, but
> the above doesn't prevent terrible code.  E.g. the correct invalidation wouldn't
> actually do anything to make the terrible code less broken.
>
> We even (somewhat unintentionally) removed that terrible code in commit 321ef62b0c5f
> ("KVM: nVMX: Fold requested virtual interrupt check into has_nested_events()").
> And that commit was tagged for stable@, it just didn't apply.  If we want to do
> anything for 6.1.y (and 6.6.y?), we should backport that entire series:
>
>   https://lore.kernel.org/all/2024072925-straw-mashing-54f6@gregkh

I see — so the proper LTS fix would be to backport that series rather
than adding band-aid patches. That makes sense.

For context, I sent the patch because the NULL dereference appears
to be guest-triggerable on 6.1.y, and I wanted to suggest something
minimal for the LTS branches. But based on your feedback, backporting
the series that includes 321ef62b0c5f sounds like the correct fix.

I also sent a minimal NULL check patch for vmx_guest_apic_has_interrupt()
to stable@vger.kernel.org. Would that still be reasonable as an interim
fix until the full series is backported, or would you prefer to just
backport the series directly?

Thanks

Re: [PATCH] KVM: clear map->gfn in kvm_vcpu_unmap() to prevent stale validity checks

[Taeyang Lee]

Hello,

Gentle ping on this. Based on your suggestion, it sounds like the right
approach is to backport the 321ef62b0c5f series to the affected LTS
branches.

However, looking at the lore link you shared, it seems like the backport
didn't apply cleanly due to SEV-SNP dependencies. Do you have any
thoughts on how best to handle this for the LTS branches?

Thanks


On Thu, Mar 12, 2026 at 4:00 PM Taeyang Lee <0wn@theori.io> wrote:
>
> Hi Sean,
>
> Thanks for the detailed review.
>
> On Wed, Mar 11, 2026 at 10:53 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Wed, Mar 11, 2026, Taeyang Lee wrote:
> > > kvm_vcpu_unmap() clears map->hva and map->page but leaves map->gfn with
> > > its previous value. This creates an inconsistent state: callers that
> > > check gfn != 0 as a proxy for map validity will believe the map is still
> >
> > But that was and still is flat out wrong, '0' is a perfectly legal gfn.
>
> You're right, I hadn't considered that. Setting gfn = 0 as a sentinel
> is fundamentally incorrect.
>
> >
> > > valid when hva is already NULL.
> > >
> > > This pattern caused a null pointer dereference in the 6.1.x LTS branch,
> > > where vmx_guest_apic_has_interrupt() checked virtual_apic_map.gfn but
> > > dereferenced virtual_apic_map.hva unconditionally. That specific call
> > > site no longer exists in mainline due to the gfn_to_pfn_cache
> > > refactoring, but the inconsistency in kvm_vcpu_unmap() remains and could
> > > affect future kvm_host_map users that rely on gfn for validity.
> > >
> > > Similarly, kvm_vcpu_map() does not modify the map struct on failure, so
> > > stale gfn values from a previous successful mapping survive a failed
> > > remap attempt. Clearing gfn in kvm_vcpu_unmap() ensures that after an
> > > unmap-then-failed-remap sequence, gfn correctly reflects that no valid
> > > mapping exists.
> > >
> > > Clear map->gfn in kvm_vcpu_unmap().
> > >
> > > Reported-by: Taeyang Lee <0wn@theori.io>
> > > Fixes: e45adf665a53 ("KVM: Introduce a new guest mapping API")
> >
> > This is not the correct fixes, the blame lies squarely on 96c66e87deee ("KVM/nVMX:
> > Use kvm_vcpu_map when mapping the virtual APIC page").  The API even provided
> > kvm_vcpu_mapped() from the get-go; why commit e45adf665a53 checked the gfn, I have
> > no idea.
> >
>
> Agreed — the bug is in the caller misusing gfn instead of
> kvm_vcpu_mapped(), not in the unmap API itself.
>
> > > Signed-off-by: Taeyang Lee <0wn@theori.io>
> > > ---
> > >  virt/kvm/kvm_main.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > > index 7a4fd1dbe0d7..88fc8b20aa8f 100644
> > > --- a/virt/kvm/kvm_main.c
> > > +++ b/virt/kvm/kvm_main.c
> > > @@ -2887,6 +2887,7 @@ void kvm_vcpu_unmap(struct kvm_vcpu *vcpu, struct kvm_host_map *map, bool dirty)
> > >
> > >       map->hva = NULL;
> > >       map->page = NULL;
> > > +     map->gfn = 0;
> >
> > As above, this is at best misleading.  If we wanted to harden this, we should do
> >
> >         map->gfn = INVALID_GPA;
> >
> > but I honestly don't see the point.  Yes, we had terrible code in the past, but
> > the above doesn't prevent terrible code.  E.g. the correct invalidation wouldn't
> > actually do anything to make the terrible code less broken.
> >
> > We even (somewhat unintentionally) removed that terrible code in commit 321ef62b0c5f
> > ("KVM: nVMX: Fold requested virtual interrupt check into has_nested_events()").
> > And that commit was tagged for stable@, it just didn't apply.  If we want to do
> > anything for 6.1.y (and 6.6.y?), we should backport that entire series:
> >
> >   https://lore.kernel.org/all/2024072925-straw-mashing-54f6@gregkh
>
> I see — so the proper LTS fix would be to backport that series rather
> than adding band-aid patches. That makes sense.
>
> For context, I sent the patch because the NULL dereference appears
> to be guest-triggerable on 6.1.y, and I wanted to suggest something
> minimal for the LTS branches. But based on your feedback, backporting
> the series that includes 321ef62b0c5f sounds like the correct fix.
>
> I also sent a minimal NULL check patch for vmx_guest_apic_has_interrupt()
> to stable@vger.kernel.org. Would that still be reasonable as an interim
> fix until the full series is backported, or would you prefer to just
> backport the series directly?
>
> Thanks