Skip to content

Commit 3376ca3

Browse files
minipli-osssean-jc
authored andcommitted
KVM: x86: Fix KVM_GET_MSRS stack info leak
Commit 6abe9c1 ("KVM: X86: Move ignore_msrs handling upper the stack") changed the 'ignore_msrs' handling, including sanitizing return values to the caller. This was fine until commit 12bc213 ("KVM: X86: Do the same ignore_msrs check for feature msrs") which allowed non-existing feature MSRs to be ignored, i.e. to not generate an error on the ioctl() level. It even tried to preserve the sanitization of the return value. However, the logic is flawed, as '*data' will be overwritten again with the uninitialized stack value of msr.data. Fix this by simplifying the logic and always initializing msr.data, vanishing the need for an additional error exit path. Fixes: 12bc213 ("KVM: X86: Do the same ignore_msrs check for feature msrs") Signed-off-by: Mathias Krause <minipli@grsecurity.net> Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com> Link: https://lore.kernel.org/r/20240203124522.592778-2-minipli@grsecurity.net Signed-off-by: Sean Christopherson <seanjc@google.com>
1 parent 05519c8 commit 3376ca3

File tree

1 file changed

+5
-10
lines changed

1 file changed

+5
-10
lines changed

arch/x86/kvm/x86.c

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1704,22 +1704,17 @@ static int do_get_msr_feature(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
17041704
struct kvm_msr_entry msr;
17051705
int r;
17061706

1707+
/* Unconditionally clear the output for simplicity */
1708+
msr.data = 0;
17071709
msr.index = index;
17081710
r = kvm_get_msr_feature(&msr);
17091711

1710-
if (r == KVM_MSR_RET_INVALID) {
1711-
/* Unconditionally clear the output for simplicity */
1712-
*data = 0;
1713-
if (kvm_msr_ignored_check(index, 0, false))
1714-
r = 0;
1715-
}
1716-
1717-
if (r)
1718-
return r;
1712+
if (r == KVM_MSR_RET_INVALID && kvm_msr_ignored_check(index, 0, false))
1713+
r = 0;
17191714

17201715
*data = msr.data;
17211716

1722-
return 0;
1717+
return r;
17231718
}
17241719

17251720
static bool __kvm_valid_efer(struct kvm_vcpu *vcpu, u64 efer)

0 commit comments

Comments
 (0)