Skip to content

Commit bd5f500

Browse files
committed
KVM: SEV: Read save fields from GHCB exactly once
Wrap all reads of GHCB save fields with READ_ONCE() via a KVM-specific GHCB get() utility to help guard against TOCTOU bugs. Using READ_ONCE() doesn't completely prevent such bugs, e.g. doesn't prevent KVM from redoing get() after checking the initial value, but at least addresses all potential TOCTOU issues in the current KVM code base. To prevent unintentional use of the generic helpers, take only @SVM for the kvm_ghcb_get_xxx() helpers and retrieve the ghcb instead of explicitly passing it in. Opportunistically reduce the indentation of the macro-defined helpers and clean up the alignment. Fixes: 4e15a0d ("KVM: SEV: snapshot the GHCB before accessing it") Reviewed-by: Tom Lendacky <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Sean Christopherson <[email protected]>
1 parent e0ff302 commit bd5f500

File tree

2 files changed

+26
-21
lines changed

2 files changed

+26
-21
lines changed

arch/x86/kvm/svm/sev.c

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3299,26 +3299,26 @@ static void sev_es_sync_from_ghcb(struct vcpu_svm *svm)
32993299
BUILD_BUG_ON(sizeof(svm->sev_es.valid_bitmap) != sizeof(ghcb->save.valid_bitmap));
33003300
memcpy(&svm->sev_es.valid_bitmap, &ghcb->save.valid_bitmap, sizeof(ghcb->save.valid_bitmap));
33013301

3302-
vcpu->arch.regs[VCPU_REGS_RAX] = kvm_ghcb_get_rax_if_valid(svm, ghcb);
3303-
vcpu->arch.regs[VCPU_REGS_RBX] = kvm_ghcb_get_rbx_if_valid(svm, ghcb);
3304-
vcpu->arch.regs[VCPU_REGS_RCX] = kvm_ghcb_get_rcx_if_valid(svm, ghcb);
3305-
vcpu->arch.regs[VCPU_REGS_RDX] = kvm_ghcb_get_rdx_if_valid(svm, ghcb);
3306-
vcpu->arch.regs[VCPU_REGS_RSI] = kvm_ghcb_get_rsi_if_valid(svm, ghcb);
3302+
vcpu->arch.regs[VCPU_REGS_RAX] = kvm_ghcb_get_rax_if_valid(svm);
3303+
vcpu->arch.regs[VCPU_REGS_RBX] = kvm_ghcb_get_rbx_if_valid(svm);
3304+
vcpu->arch.regs[VCPU_REGS_RCX] = kvm_ghcb_get_rcx_if_valid(svm);
3305+
vcpu->arch.regs[VCPU_REGS_RDX] = kvm_ghcb_get_rdx_if_valid(svm);
3306+
vcpu->arch.regs[VCPU_REGS_RSI] = kvm_ghcb_get_rsi_if_valid(svm);
33073307

3308-
svm->vmcb->save.cpl = kvm_ghcb_get_cpl_if_valid(svm, ghcb);
3308+
svm->vmcb->save.cpl = kvm_ghcb_get_cpl_if_valid(svm);
33093309

33103310
if (kvm_ghcb_xcr0_is_valid(svm)) {
3311-
vcpu->arch.xcr0 = ghcb_get_xcr0(ghcb);
3311+
vcpu->arch.xcr0 = kvm_ghcb_get_xcr0(svm);
33123312
vcpu->arch.cpuid_dynamic_bits_dirty = true;
33133313
}
33143314

33153315
/* Copy the GHCB exit information into the VMCB fields */
3316-
exit_code = ghcb_get_sw_exit_code(ghcb);
3316+
exit_code = kvm_ghcb_get_sw_exit_code(svm);
33173317
control->exit_code = lower_32_bits(exit_code);
33183318
control->exit_code_hi = upper_32_bits(exit_code);
3319-
control->exit_info_1 = ghcb_get_sw_exit_info_1(ghcb);
3320-
control->exit_info_2 = ghcb_get_sw_exit_info_2(ghcb);
3321-
svm->sev_es.sw_scratch = kvm_ghcb_get_sw_scratch_if_valid(svm, ghcb);
3319+
control->exit_info_1 = kvm_ghcb_get_sw_exit_info_1(svm);
3320+
control->exit_info_2 = kvm_ghcb_get_sw_exit_info_2(svm);
3321+
svm->sev_es.sw_scratch = kvm_ghcb_get_sw_scratch_if_valid(svm);
33223322

33233323
/* Clear the valid entries fields */
33243324
memset(ghcb->save.valid_bitmap, 0, sizeof(ghcb->save.valid_bitmap));

arch/x86/kvm/svm/svm.h

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -913,16 +913,21 @@ void __svm_sev_es_vcpu_run(struct vcpu_svm *svm, bool spec_ctrl_intercepted,
913913
void __svm_vcpu_run(struct vcpu_svm *svm, bool spec_ctrl_intercepted);
914914

915915
#define DEFINE_KVM_GHCB_ACCESSORS(field) \
916-
static __always_inline bool kvm_ghcb_##field##_is_valid(const struct vcpu_svm *svm) \
917-
{ \
918-
return test_bit(GHCB_BITMAP_IDX(field), \
919-
(unsigned long *)&svm->sev_es.valid_bitmap); \
920-
} \
921-
\
922-
static __always_inline u64 kvm_ghcb_get_##field##_if_valid(struct vcpu_svm *svm, struct ghcb *ghcb) \
923-
{ \
924-
return kvm_ghcb_##field##_is_valid(svm) ? ghcb->save.field : 0; \
925-
} \
916+
static __always_inline u64 kvm_ghcb_get_##field(struct vcpu_svm *svm) \
917+
{ \
918+
return READ_ONCE(svm->sev_es.ghcb->save.field); \
919+
} \
920+
\
921+
static __always_inline bool kvm_ghcb_##field##_is_valid(const struct vcpu_svm *svm) \
922+
{ \
923+
return test_bit(GHCB_BITMAP_IDX(field), \
924+
(unsigned long *)&svm->sev_es.valid_bitmap); \
925+
} \
926+
\
927+
static __always_inline u64 kvm_ghcb_get_##field##_if_valid(struct vcpu_svm *svm) \
928+
{ \
929+
return kvm_ghcb_##field##_is_valid(svm) ? kvm_ghcb_get_##field(svm) : 0; \
930+
}
926931

927932
DEFINE_KVM_GHCB_ACCESSORS(cpl)
928933
DEFINE_KVM_GHCB_ACCESSORS(rax)

0 commit comments

Comments
 (0)