KVM: x86: Fix and harden reg caching from !TASK context

[PATCH v4 0/4] KVM: x86: Fix and harden reg caching from !TASK context

Posted by Sean Christopherson 1 month, 2 weeks ago

Fix a (VMX only) bug reported by Maxim where KVM caches a stale SS.AR_BYTES
when involuntary preemption schedules out a vCPU during vmx_vcpu_rest(), and
ultimately clobbers the VMCS's SS.AR_BYTES if userspace does KVM_GET_SREGS
=> KVM_SET_SREGS, i.e. if userspace writes the stale value back into KVM.

v4, as this is a spiritual successor to Maxim's earlier series.

Patch 1 fixes the underlying problem by avoiding the cache in kvm_sched_out().

Patch 2 fixes vmx_vcpu_reset() to invalidate the cache _after_ writing the
VMCS, which also fixes the VMCS clobbering bug, but isn't as robust of a fix
for KVM as a whole, e.g. any other flow that invalidates the cache too "early"
would be susceptible to the bug, and on its own doesn't allow for the
hardening in patch 3.

Patch 3 hardens KVM against using the register caches from !TASK context.
Except for PMI callbacks, which are tightly bounded, i.e. can't run while
KVM is modifying segment information, using the register caches from IRQ/NMI
is unsafe.

Patch 4 is a tangentially related cleanup.

v3: https://lore.kernel.org/all/20240725175232.337266-1-mlevitsk@redhat.com

Maxim Levitsky (1):
  KVM: VMX: reset the segment cache after segment init in
    vmx_vcpu_reset()

Sean Christopherson (3):
  KVM: x86: Bypass register cache when querying CPL from kvm_sched_out()
  KVM: x86: Add lockdep-guarded asserts on register cache usage
  KVM: x86: Use '0' for guest RIP if PMI encounters protected guest
    state

 arch/x86/include/asm/kvm-x86-ops.h |  1 +
 arch/x86/include/asm/kvm_host.h    |  1 +
 arch/x86/kvm/kvm_cache_regs.h      | 17 +++++++++++++++++
 arch/x86/kvm/svm/svm.c             |  1 +
 arch/x86/kvm/vmx/main.c            |  1 +
 arch/x86/kvm/vmx/vmx.c             | 29 +++++++++++++++++++++--------
 arch/x86/kvm/vmx/vmx.h             |  1 +
 arch/x86/kvm/x86.c                 | 15 ++++++++++++++-
 8 files changed, 57 insertions(+), 9 deletions(-)


base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b
-- 
2.47.0.rc1.288.g06298d1525-goog

Re: [PATCH v4 0/4] KVM: x86: Fix and harden reg caching from !TASK context

Posted by Sean Christopherson 3 weeks, 5 days ago

On Wed, 09 Oct 2024 10:49:58 -0700, Sean Christopherson wrote:
> Fix a (VMX only) bug reported by Maxim where KVM caches a stale SS.AR_BYTES
> when involuntary preemption schedules out a vCPU during vmx_vcpu_rest(), and
> ultimately clobbers the VMCS's SS.AR_BYTES if userspace does KVM_GET_SREGS
> => KVM_SET_SREGS, i.e. if userspace writes the stale value back into KVM.
> 
> v4, as this is a spiritual successor to Maxim's earlier series.
> 
> [...]

Applied 1 and 3-4 to kvm-x86 misc.  Patch 2 went into 6.12.  Thanks!

[1/4] KVM: x86: Bypass register cache when querying CPL from kvm_sched_out()
      https://github.com/kvm-x86/linux/commit/8c8e90f79c56
[2/4] KVM: VMX: reset the segment cache after segment init in vmx_vcpu_reset()
      (no commit info)
[3/4] KVM: x86: Add lockdep-guarded asserts on register cache usage
      https://github.com/kvm-x86/linux/commit/21abefc6958d
[4/4] KVM: x86: Use '0' for guest RIP if PMI encounters protected guest state
      https://github.com/kvm-x86/linux/commit/a395d143ef40

--
https://github.com/kvm-x86/linux/tree/next

Re: [PATCH v4 0/4] KVM: x86: Fix and harden reg caching from !TASK context

Posted by Sean Christopherson 3 weeks, 4 days ago

On Thu, Oct 31, 2024, Sean Christopherson wrote:
> On Wed, 09 Oct 2024 10:49:58 -0700, Sean Christopherson wrote:
> > Fix a (VMX only) bug reported by Maxim where KVM caches a stale SS.AR_BYTES
> > when involuntary preemption schedules out a vCPU during vmx_vcpu_rest(), and
> > ultimately clobbers the VMCS's SS.AR_BYTES if userspace does KVM_GET_SREGS
> > => KVM_SET_SREGS, i.e. if userspace writes the stale value back into KVM.
> > 
> > v4, as this is a spiritual successor to Maxim's earlier series.
> > 
> > [...]
> 
> Applied 1 and 3-4 to kvm-x86 misc.  Patch 2 went into 6.12.  Thanks!
> 
> [1/4] KVM: x86: Bypass register cache when querying CPL from kvm_sched_out()
>       https://github.com/kvm-x86/linux/commit/8c8e90f79c56
> [2/4] KVM: VMX: reset the segment cache after segment init in vmx_vcpu_reset()
>       (no commit info)
> [3/4] KVM: x86: Add lockdep-guarded asserts on register cache usage
>       https://github.com/kvm-x86/linux/commit/21abefc6958d
> [4/4] KVM: x86: Use '0' for guest RIP if PMI encounters protected guest state
>       https://github.com/kvm-x86/linux/commit/a395d143ef40

FYI, I rebased misc to v6.12-rc5, as patches in another series had already been
taken through the tip tree.  New hashes:

[1/4] KVM: x86: Bypass register cache when querying CPL from kvm_sched_out()
      https://github.com/kvm-x86/linux/commit/f0e7012c4b93
...

[3/4] KVM: x86: Add lockdep-guarded asserts on register cache usage
      https://github.com/kvm-x86/linux/commit/1c932fc7620d
[4/4] KVM: x86: Use '0' for guest RIP if PMI encounters protected guest state
      https://github.com/kvm-x86/linux/commit/eecf3985459a

Re: [PATCH v4 0/4] KVM: x86: Fix and harden reg caching from !TASK context

Posted by Paolo Bonzini 1 month, 2 weeks ago

On 10/9/24 19:49, Sean Christopherson wrote:
> Fix a (VMX only) bug reported by Maxim where KVM caches a stale SS.AR_BYTES
> when involuntary preemption schedules out a vCPU during vmx_vcpu_rest(), and
> ultimately clobbers the VMCS's SS.AR_BYTES if userspace does KVM_GET_SREGS
> => KVM_SET_SREGS, i.e. if userspace writes the stale value back into KVM.
> 
> v4, as this is a spiritual successor to Maxim's earlier series.
> 
> Patch 1 fixes the underlying problem by avoiding the cache in kvm_sched_out().

I think we want this one in stable?

Thanks,

Paolo

> Patch 2 fixes vmx_vcpu_reset() to invalidate the cache _after_ writing the
> VMCS, which also fixes the VMCS clobbering bug, but isn't as robust of a fix
> for KVM as a whole, e.g. any other flow that invalidates the cache too "early"
> would be susceptible to the bug, and on its own doesn't allow for the
> hardening in patch 3.
> 
> Patch 3 hardens KVM against using the register caches from !TASK context.
> Except for PMI callbacks, which are tightly bounded, i.e. can't run while
> KVM is modifying segment information, using the register caches from IRQ/NMI
> is unsafe.
> 
> Patch 4 is a tangentially related cleanup.
> 
> v3: https://lore.kernel.org/all/20240725175232.337266-1-mlevitsk@redhat.com
> 
> Maxim Levitsky (1):
>    KVM: VMX: reset the segment cache after segment init in
>      vmx_vcpu_reset()
> 
> Sean Christopherson (3):
>    KVM: x86: Bypass register cache when querying CPL from kvm_sched_out()
>    KVM: x86: Add lockdep-guarded asserts on register cache usage
>    KVM: x86: Use '0' for guest RIP if PMI encounters protected guest
>      state
> 
>   arch/x86/include/asm/kvm-x86-ops.h |  1 +
>   arch/x86/include/asm/kvm_host.h    |  1 +
>   arch/x86/kvm/kvm_cache_regs.h      | 17 +++++++++++++++++
>   arch/x86/kvm/svm/svm.c             |  1 +
>   arch/x86/kvm/vmx/main.c            |  1 +
>   arch/x86/kvm/vmx/vmx.c             | 29 +++++++++++++++++++++--------
>   arch/x86/kvm/vmx/vmx.h             |  1 +
>   arch/x86/kvm/x86.c                 | 15 ++++++++++++++-
>   8 files changed, 57 insertions(+), 9 deletions(-)
> 
> 
> base-commit: 8cf0b93919e13d1e8d4466eb4080a4c4d9d66d7b

Re: [PATCH v4 0/4] KVM: x86: Fix and harden reg caching from !TASK context

Posted by Sean Christopherson 1 month, 2 weeks ago

On Thu, Oct 10, 2024, Paolo Bonzini wrote:
> On 10/9/24 19:49, Sean Christopherson wrote:
> > Fix a (VMX only) bug reported by Maxim where KVM caches a stale SS.AR_BYTES
> > when involuntary preemption schedules out a vCPU during vmx_vcpu_rest(), and
> > ultimately clobbers the VMCS's SS.AR_BYTES if userspace does KVM_GET_SREGS
> > => KVM_SET_SREGS, i.e. if userspace writes the stale value back into KVM.
> > 
> > v4, as this is a spiritual successor to Maxim's earlier series.
> > 
> > Patch 1 fixes the underlying problem by avoiding the cache in kvm_sched_out().
> 
> I think we want this one in stable?

If anything, we should send Maxim's patch to stable trees.  While not a complete
fix, it resolves the only known scenario where caching SS.AR_BYTES is truly
problematic, it's as low risk as patches get, and it's much more likely to backport
cleanly to older kernels.

> > Patch 2 fixes vmx_vcpu_reset() to invalidate the cache _after_ writing the
> > VMCS, which also fixes the VMCS clobbering bug, but isn't as robust of a fix
> > for KVM as a whole, e.g. any other flow that invalidates the cache too "early"
> > would be susceptible to the bug, and on its own doesn't allow for the
> > hardening in patch 3.

Re: [PATCH v4 0/4] KVM: x86: Fix and harden reg caching from !TASK context

Posted by Paolo Bonzini 1 month, 2 weeks ago

On Thu, Oct 10, 2024 at 6:17 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Oct 10, 2024, Paolo Bonzini wrote:
> > On 10/9/24 19:49, Sean Christopherson wrote:
> > > Fix a (VMX only) bug reported by Maxim where KVM caches a stale SS.AR_BYTES
> > > when involuntary preemption schedules out a vCPU during vmx_vcpu_rest(), and
> > > ultimately clobbers the VMCS's SS.AR_BYTES if userspace does KVM_GET_SREGS
> > > => KVM_SET_SREGS, i.e. if userspace writes the stale value back into KVM.
> > >
> > > v4, as this is a spiritual successor to Maxim's earlier series.
> > >
> > > Patch 1 fixes the underlying problem by avoiding the cache in kvm_sched_out().
> >
> > I think we want this one in stable?
>
> If anything, we should send Maxim's patch to stable trees.  While not a complete
> fix, it resolves the only known scenario where caching SS.AR_BYTES is truly
> problematic, it's as low risk as patches get, and it's much more likely to backport
> cleanly to older kernels.

Ok, this works for me.

Paolo