Cache g_pat from vmcb12 in vmcb_save_area_cached to avoid TOCTTOU issues,
and add a validity check so that when nested paging is enabled for vmcb12,
an invalid g_pat at emulated VMRUN causes an immediate VMEXIT with exit
code VMEXIT_INVALID, as specified in the APM, volume 2: "Nested Paging and
VMRUN/VMEXIT."
Fixes: 3d6368ef580a ("KVM: SVM: Add VMRUN handler")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
arch/x86/kvm/svm/nested.c | 17 +++++++++++++----
arch/x86/kvm/svm/svm.h | 1 +
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index b72a1f3c4144..91b35adb83f8 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -426,7 +426,8 @@ static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
/* Common checks that apply to both L1 and L2 state. */
static bool nested_vmcb_check_save(struct kvm_vcpu *vcpu,
- struct vmcb_save_area_cached *save)
+ struct vmcb_save_area_cached *save,
+ bool check_gpat)
{
if (CC(!(save->efer & EFER_SVME)))
return false;
@@ -462,6 +463,9 @@ static bool nested_vmcb_check_save(struct kvm_vcpu *vcpu,
if (CC(!kvm_valid_efer(vcpu, save->efer)))
return false;
+ if (check_gpat && CC(!kvm_pat_valid(save->g_pat)))
+ return false;
+
return true;
}
@@ -573,6 +577,7 @@ static void __nested_copy_vmcb_save_to_cache(struct vmcb_save_area_cached *to,
to->rax = from->rax;
to->cr2 = from->cr2;
+ to->g_pat = from->g_pat;
svm_copy_lbrs(to, from);
}
@@ -1036,7 +1041,8 @@ int enter_svm_guest_mode(struct kvm_vcpu *vcpu, u64 vmcb12_gpa, bool from_vmrun)
enter_guest_mode(vcpu);
- if (!nested_vmcb_check_save(vcpu, &svm->nested.save) ||
+ if (!nested_vmcb_check_save(vcpu, &svm->nested.save,
+ nested_npt_enabled(svm)) ||
!nested_vmcb_check_controls(vcpu, &svm->nested.ctl,
svm->vmcb01.ptr->save.cr0))
return -EINVAL;
@@ -2006,13 +2012,16 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
/*
* Validate host state saved from before VMRUN (see
- * nested_svm_check_permissions).
+ * nested_svm_check_permissions). Note that the g_pat field is not
+ * validated, because (a) it may have been clobbered by SMM before
+ * KVM_GET_NESTED_STATE, and (b) it is not loaded at emulated
+ * #VMEXIT.
*/
__nested_copy_vmcb_save_to_cache(&save_cached, save);
if (!(save->cr0 & X86_CR0_PG) ||
!(save->cr0 & X86_CR0_PE) ||
(save->rflags & X86_EFLAGS_VM) ||
- !nested_vmcb_check_save(vcpu, &save_cached))
+ !nested_vmcb_check_save(vcpu, &save_cached, false))
goto out_free;
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 9850ed01e16e..a49c48459e0b 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -161,6 +161,7 @@ struct vmcb_save_area_cached {
u64 isst_addr;
u64 rax;
u64 cr2;
+ u64 g_pat;
u64 dbgctl;
u64 br_from;
u64 br_to;
--
2.53.0.239.g8d8fc8a987-googOn Thu, Feb 12, 2026 at 07:58:50AM -0800, Jim Mattson wrote:
> Cache g_pat from vmcb12 in vmcb_save_area_cached to avoid TOCTTOU issues,
> and add a validity check so that when nested paging is enabled for vmcb12,
> an invalid g_pat at emulated VMRUN causes an immediate VMEXIT with exit
> code VMEXIT_INVALID, as specified in the APM, volume 2: "Nested Paging and
> VMRUN/VMEXIT."
>
> Fixes: 3d6368ef580a ("KVM: SVM: Add VMRUN handler")
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
> arch/x86/kvm/svm/nested.c | 17 +++++++++++++----
> arch/x86/kvm/svm/svm.h | 1 +
> 2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index b72a1f3c4144..91b35adb83f8 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -426,7 +426,8 @@ static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
>
> /* Common checks that apply to both L1 and L2 state. */
> static bool nested_vmcb_check_save(struct kvm_vcpu *vcpu,
> - struct vmcb_save_area_cached *save)
> + struct vmcb_save_area_cached *save,
> + bool check_gpat)
> {
> if (CC(!(save->efer & EFER_SVME)))
> return false;
> @@ -462,6 +463,9 @@ static bool nested_vmcb_check_save(struct kvm_vcpu *vcpu,
> if (CC(!kvm_valid_efer(vcpu, save->efer)))
> return false;
>
> + if (check_gpat && CC(!kvm_pat_valid(save->g_pat)))
> + return false;
> +
> return true;
> }
>
> @@ -573,6 +577,7 @@ static void __nested_copy_vmcb_save_to_cache(struct vmcb_save_area_cached *to,
>
> to->rax = from->rax;
> to->cr2 = from->cr2;
> + to->g_pat = from->g_pat;
>
> svm_copy_lbrs(to, from);
> }
> @@ -1036,7 +1041,8 @@ int enter_svm_guest_mode(struct kvm_vcpu *vcpu, u64 vmcb12_gpa, bool from_vmrun)
>
> enter_guest_mode(vcpu);
>
> - if (!nested_vmcb_check_save(vcpu, &svm->nested.save) ||
> + if (!nested_vmcb_check_save(vcpu, &svm->nested.save,
> + nested_npt_enabled(svm)) ||
> !nested_vmcb_check_controls(vcpu, &svm->nested.ctl,
> svm->vmcb01.ptr->save.cr0))
> return -EINVAL;
> @@ -2006,13 +2012,16 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
>
> /*
> * Validate host state saved from before VMRUN (see
> - * nested_svm_check_permissions).
> + * nested_svm_check_permissions). Note that the g_pat field is not
> + * validated, because (a) it may have been clobbered by SMM before
> + * KVM_GET_NESTED_STATE, and (b) it is not loaded at emulated
> + * #VMEXIT.
(b) here means that svm_copy_vmrun_state() does not copy it to vmcb01,
and the value is restored by KVM_SET_MSRS, right?
If my understanding is correct:
Reviewed-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> */
> __nested_copy_vmcb_save_to_cache(&save_cached, save);
> if (!(save->cr0 & X86_CR0_PG) ||
> !(save->cr0 & X86_CR0_PE) ||
> (save->rflags & X86_EFLAGS_VM) ||
> - !nested_vmcb_check_save(vcpu, &save_cached))
> + !nested_vmcb_check_save(vcpu, &save_cached, false))
> goto out_free;
>
>
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 9850ed01e16e..a49c48459e0b 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -161,6 +161,7 @@ struct vmcb_save_area_cached {
> u64 isst_addr;
> u64 rax;
> u64 cr2;
> + u64 g_pat;
> u64 dbgctl;
> u64 br_from;
> u64 br_to;
> --
> 2.53.0.239.g8d8fc8a987-goog
>
On Thu, Feb 12, 2026 at 4:22 PM Yosry Ahmed <yosry.ahmed@linux.dev> wrote:
> > @@ -2006,13 +2012,16 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
> >
> > /*
> > * Validate host state saved from before VMRUN (see
> > - * nested_svm_check_permissions).
> > + * nested_svm_check_permissions). Note that the g_pat field is not
> > + * validated, because (a) it may have been clobbered by SMM before
> > + * KVM_GET_NESTED_STATE, and (b) it is not loaded at emulated
> > + * #VMEXIT.
>
> (b) here means that svm_copy_vmrun_state() does not copy it to vmcb01,
> and the value is restored by KVM_SET_MSRS, right?
Actually, (b) refers to the open-coded block of assignments in
nested_svm_vmexit() under the comment:
/*
* Restore processor state that had been saved in vmcb01
*/
> If my understanding is correct:
>
> Reviewed-by: Yosry Ahmed <yosry.ahmed@linux.dev>
February 21, 2026 at 12:26 AM, "Jim Mattson" <jmattson@google.com> wrote: > > On Thu, Feb 12, 2026 at 4:22 PM Yosry Ahmed <> wrote: > > > > > @@ -2006,13 +2012,16 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu, > > > > /* > > * Validate host state saved from before VMRUN (see > > - * nested_svm_check_permissions). > > + * nested_svm_check_permissions). Note that the g_pat field is not > > + * validated, because (a) it may have been clobbered by SMM before > > + * KVM_GET_NESTED_STATE, and (b) it is not loaded at emulated > > + * #VMEXIT. > > > > (b) here means that svm_copy_vmrun_state() does not copy it to vmcb01, > > and the value is restored by KVM_SET_MSRS, right? > > > Actually, (b) refers to the open-coded block of assignments in > nested_svm_vmexit() under the comment: > > /* > * Restore processor state that had been saved in vmcb01 > */ > Yeah IIUC it's the same thing, we migrate them and copy them here to vmcb01 so that we can restore them in nested_svm_vmexit().
© 2016 - 2026 Red Hat, Inc.