Skip to content

Commit 27526ef

Browse files
committed
KVM: x86: Constrain guest-supported xfeatures only at KVM_GET_XSAVE{2}
Mask off xfeatures that aren't exposed to the guest only when saving guest state via KVM_GET_XSAVE{2} instead of modifying user_xfeatures directly. Preserving the maximal set of xfeatures in user_xfeatures restores KVM's ABI for KVM_SET_XSAVE, which prior to commit ad85628 ("x86/kvm/fpu: Limit guest user_xfeatures to supported bits of XCR0") allowed userspace to load xfeatures that are supported by the host, irrespective of what xfeatures are exposed to the guest. There is no known use case where userspace *intentionally* loads xfeatures that aren't exposed to the guest, but the bug fixed by commit ad85628 was specifically that KVM_GET_SAVE{2} would save xfeatures that weren't exposed to the guest, e.g. would lead to userspace unintentionally loading guest-unsupported xfeatures when live migrating a VM. Restricting KVM_SET_XSAVE to guest-supported xfeatures is especially problematic for QEMU-based setups, as QEMU has a bug where instead of terminating the VM if KVM_SET_XSAVE fails, QEMU instead simply stops loading guest state, i.e. resumes the guest after live migration with incomplete guest state, and ultimately results in guest data corruption. Note, letting userspace restore all host-supported xfeatures does not fix setups where a VM is migrated from a host *without* commit ad85628, to a target with a subset of host-supported xfeatures. However there is no way to safely address that scenario, e.g. KVM could silently drop the unsupported features, but that would be a clear violation of KVM's ABI and so would require userspace to opt-in, at which point userspace could simply be updated to sanitize the to-be-loaded XSAVE state. Reported-by: Tyler Stachecki <stachecki.tyler@gmail.com> Closes: https://lore.kernel.org/all/20230914010003.358162-1-tstachecki@bloomberg.net Fixes: ad85628 ("x86/kvm/fpu: Limit guest user_xfeatures to supported bits of XCR0") Cc: stable@vger.kernel.org Cc: Leonardo Bras <leobras@redhat.com> Acked-by: Dave Hansen <dave.hansen@linux.intel.com> Link: https://lore.kernel.org/r/20230928001956.924301-3-seanjc@google.com Signed-off-by: Sean Christopherson <seanjc@google.com>
1 parent 2d287ec commit 27526ef

File tree

3 files changed

+17
-14
lines changed

3 files changed

+17
-14
lines changed

arch/x86/kernel/fpu/xstate.c

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1539,10 +1539,7 @@ static int fpstate_realloc(u64 xfeatures, unsigned int ksize,
15391539
fpregs_restore_userregs();
15401540

15411541
newfps->xfeatures = curfps->xfeatures | xfeatures;
1542-
1543-
if (!guest_fpu)
1544-
newfps->user_xfeatures = curfps->user_xfeatures | xfeatures;
1545-
1542+
newfps->user_xfeatures = curfps->user_xfeatures | xfeatures;
15461543
newfps->xfd = curfps->xfd & ~xfeatures;
15471544

15481545
/* Do the final updates within the locked region */

arch/x86/kvm/cpuid.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -360,14 +360,6 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
360360
vcpu->arch.guest_supported_xcr0 =
361361
cpuid_get_supported_xcr0(vcpu->arch.cpuid_entries, vcpu->arch.cpuid_nent);
362362

363-
/*
364-
* FP+SSE can always be saved/restored via KVM_{G,S}ET_XSAVE, even if
365-
* XSAVE/XCRO are not exposed to the guest, and even if XSAVE isn't
366-
* supported by the host.
367-
*/
368-
vcpu->arch.guest_fpu.fpstate->user_xfeatures = vcpu->arch.guest_supported_xcr0 |
369-
XFEATURE_MASK_FPSSE;
370-
371363
kvm_update_pv_runtime(vcpu);
372364

373365
vcpu->arch.maxphyaddr = cpuid_query_maxphyaddr(vcpu);

arch/x86/kvm/x86.c

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5386,12 +5386,26 @@ static int kvm_vcpu_ioctl_x86_set_debugregs(struct kvm_vcpu *vcpu,
53865386
static void kvm_vcpu_ioctl_x86_get_xsave2(struct kvm_vcpu *vcpu,
53875387
u8 *state, unsigned int size)
53885388
{
5389+
/*
5390+
* Only copy state for features that are enabled for the guest. The
5391+
* state itself isn't problematic, but setting bits in the header for
5392+
* features that are supported in *this* host but not exposed to the
5393+
* guest can result in KVM_SET_XSAVE failing when live migrating to a
5394+
* compatible host without the features that are NOT exposed to the
5395+
* guest.
5396+
*
5397+
* FP+SSE can always be saved/restored via KVM_{G,S}ET_XSAVE, even if
5398+
* XSAVE/XCRO are not exposed to the guest, and even if XSAVE isn't
5399+
* supported by the host.
5400+
*/
5401+
u64 supported_xcr0 = vcpu->arch.guest_supported_xcr0 |
5402+
XFEATURE_MASK_FPSSE;
5403+
53895404
if (fpstate_is_confidential(&vcpu->arch.guest_fpu))
53905405
return;
53915406

53925407
fpu_copy_guest_fpstate_to_uabi(&vcpu->arch.guest_fpu, state, size,
5393-
vcpu->arch.guest_fpu.fpstate->user_xfeatures,
5394-
vcpu->arch.pkru);
5408+
supported_xcr0, vcpu->arch.pkru);
53955409
}
53965410

53975411
static void kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu,

0 commit comments

Comments
 (0)