Skip to contentSkip to conversation

Patch "KVM, pkeys: do not use PKRU value in vcpu->arch.guest_fpu.state" has been added to the 4.9-stable tree

PATCH [email protected] 1 participant 2 messages Aug 27, 2017

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

    KVM, pkeys: do not use PKRU value in vcpu->arch.guest_fpu.state

to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
kvm-pkeys-do-not-use-pkru-value-in-vcpu-arch.guest_fpu.state.patch
and it can be found in the queue-4.9 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 38cfd5e3df9c4f88e76b547eee2087ee5c042ae2 Mon Sep 17 00:00:00 2001

From: Paolo Bonzini <[email protected]>
Date: Wed, 23 Aug 2017 23:16:29 +0200
Subject: KVM, pkeys: do not use PKRU value in vcpu->arch.guest_fpu.state

From: Paolo Bonzini <[email protected]>

commit 38cfd5e3df9c4f88e76b547eee2087ee5c042ae2 upstream.

The host pkru is restored right after vcpu exit (commit 1be0e61), so KVM_GET_XSAVE will return the host PKRU value instead. Fix this by using the guest PKRU explicitly in fill_xsave and load_xsave. This part is based on a patch by Junkang Fu.

The host PKRU data may also not match the value in vcpu->arch.guest_fpu.state, because it could have been changed by userspace since the last time it was saved, so skip loading it in kvm_load_guest_fpu.

Cc Yang Zhang <[email protected]>
Fixes 1be0e61c1f255faaeab04a390e00c8b9b9042870
---
 arch/x86/include/asm/fpu/internal.h |    6 +++---
 arch/x86/kvm/x86.c                  |   17 ++++++++++++++---
 2 files changed, 17 insertions(+), 6 deletions(-)
arch/x86/include/asm/fpu/internal.h +3 -3
@@ -445,10 +445,10 @@ static inline int copy_fpregs_to_fpstate
 	return 0;
 }
 
-static inline void __copy_kernel_to_fpregs(union fpregs_state *fpstate)
+static inline void __copy_kernel_to_fpregs(union fpregs_state *fpstate, u64 mask)
 {
 	if (use_xsave()) {
-		copy_kernel_to_xregs(&fpstate->xsave, -1);
+		copy_kernel_to_xregs(&fpstate->xsave, mask);
 	} else {
 		if (use_fxsr())
 			copy_kernel_to_fxregs(&fpstate->fxsave);
@@ -472,7 +472,7 @@ static inline void copy_kernel_to_fpregs
 			: : [addr] "m" (fpstate));
 	}
 
-	__copy_kernel_to_fpregs(fpstate);
+	__copy_kernel_to_fpregs(fpstate, -1);
 }
 
 extern int copy_fpstate_to_sigframe(void __user *buf, void __user *fp, int size);
arch/x86/kvm/x86.c +14 -3
@@ -3179,7 +3179,12 @@ static void fill_xsave(u8 *dest, struct
 			u32 size, offset, ecx, edx;
 			cpuid_count(XSTATE_CPUID, index,
 				    &size, &offset, &ecx, &edx);
-			memcpy(dest + offset, src, size);
+			if (feature == XFEATURE_MASK_PKRU)
+				memcpy(dest + offset, &vcpu->arch.pkru,
+				       sizeof(vcpu->arch.pkru));
+			else
+				memcpy(dest + offset, src, size);
+
 		}
 
 		valid -= feature;
@@ -3217,7 +3222,11 @@ static void load_xsave(struct kvm_vcpu *
 			u32 size, offset, ecx, edx;
 			cpuid_count(XSTATE_CPUID, index,
 				    &size, &offset, &ecx, &edx);
-			memcpy(dest, src + offset, size);
+			if (feature == XFEATURE_MASK_PKRU)
+				memcpy(&vcpu->arch.pkru, src + offset,
+				       sizeof(vcpu->arch.pkru));
+			else
+				memcpy(dest, src + offset, size);
 		}
 
 		valid -= feature;
@@ -7453,7 +7462,9 @@ void kvm_load_guest_fpu(struct kvm_vcpu
 	 */
 	vcpu->guest_fpu_loaded = 1;
 	__kernel_fpu_begin();
-	__copy_kernel_to_fpregs(&vcpu->arch.guest_fpu.state);
+	/* PKRU is separately restored in kvm_x86_ops->run.  */
+	__copy_kernel_to_fpregs(&vcpu->arch.guest_fpu.state,
+				~XFEATURE_MASK_PKRU);
 	trace_kvm_fpu(1);
 }
 

Patches currently in stable-queue which might be from [email protected] are

queue-4.9/kvm-x86-block-guest-protection-keys-unless-the-host-has-them-enabled.patch queue-4.9/kvm-pkeys-do-not-use-pkru-value-in-vcpu-arch.guest_fpu.state.patch

author maintainer
#1 lore raw

Oops, no, this didn't build properly, can someone please provide a working version for 4.9 so that I can apply it?

thanks,

greg k-h

Quoted 115 lines · On Sun, Aug 27, 2017 at 09:48:45AM +0200, [email protected] wrote:

This is a note to let you know that I've just added the patch titled

    KVM, pkeys: do not use PKRU value in vcpu->arch.guest_fpu.state

to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
kvm-pkeys-do-not-use-pkru-value-in-vcpu-arch.guest_fpu.state.patch
and it can be found in the queue-4.9 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 38cfd5e3df9c4f88e76b547eee2087ee5c042ae2 Mon Sep 17 00:00:00 2001

From: Paolo Bonzini <[email protected]>
Date: Wed, 23 Aug 2017 23:16:29 +0200
Subject: KVM, pkeys: do not use PKRU value in vcpu->arch.guest_fpu.state

From: Paolo Bonzini <[email protected]>

commit 38cfd5e3df9c4f88e76b547eee2087ee5c042ae2 upstream.

The host pkru is restored right after vcpu exit (commit 1be0e61), so KVM_GET_XSAVE will return the host PKRU value instead. Fix this by using the guest PKRU explicitly in fill_xsave and load_xsave. This part is based on a patch by Junkang Fu.

The host PKRU data may also not match the value in vcpu->arch.guest_fpu.state, because it could have been changed by userspace since the last time it was saved, so skip loading it in kvm_load_guest_fpu.

Cc Yang Zhang <[email protected]>
Fixes 1be0e61c1f255faaeab04a390e00c8b9b9042870
---
 arch/x86/include/asm/fpu/internal.h |    6 +++---
 arch/x86/kvm/x86.c                  |   17 ++++++++++++++---
 2 files changed, 17 insertions(+), 6 deletions(-)
arch/x86/include/asm/fpu/internal.h +3 -3
@@ -445,10 +445,10 @@ static inline int copy_fpregs_to_fpstate
 	return 0;
 }
 
-static inline void __copy_kernel_to_fpregs(union fpregs_state *fpstate)
+static inline void __copy_kernel_to_fpregs(union fpregs_state *fpstate, u64 mask)
 {
 	if (use_xsave()) {
-		copy_kernel_to_xregs(&fpstate->xsave, -1);
+		copy_kernel_to_xregs(&fpstate->xsave, mask);
 	} else {
 		if (use_fxsr())
 			copy_kernel_to_fxregs(&fpstate->fxsave);
@@ -472,7 +472,7 @@ static inline void copy_kernel_to_fpregs
 			: : [addr] "m" (fpstate));
 	}
 
-	__copy_kernel_to_fpregs(fpstate);
+	__copy_kernel_to_fpregs(fpstate, -1);
 }
 
 extern int copy_fpstate_to_sigframe(void __user *buf, void __user *fp, int size);
arch/x86/kvm/x86.c +14 -3
@@ -3179,7 +3179,12 @@ static void fill_xsave(u8 *dest, struct
 			u32 size, offset, ecx, edx;
 			cpuid_count(XSTATE_CPUID, index,
 				    &size, &offset, &ecx, &edx);
-			memcpy(dest + offset, src, size);
+			if (feature == XFEATURE_MASK_PKRU)
+				memcpy(dest + offset, &vcpu->arch.pkru,
+				       sizeof(vcpu->arch.pkru));
+			else
+				memcpy(dest + offset, src, size);
+
 		}
 
 		valid -= feature;
@@ -3217,7 +3222,11 @@ static void load_xsave(struct kvm_vcpu *
 			u32 size, offset, ecx, edx;
 			cpuid_count(XSTATE_CPUID, index,
 				    &size, &offset, &ecx, &edx);
-			memcpy(dest, src + offset, size);
+			if (feature == XFEATURE_MASK_PKRU)
+				memcpy(&vcpu->arch.pkru, src + offset,
+				       sizeof(vcpu->arch.pkru));
+			else
+				memcpy(dest, src + offset, size);
 		}
 
 		valid -= feature;
@@ -7453,7 +7462,9 @@ void kvm_load_guest_fpu(struct kvm_vcpu
 	 */
 	vcpu->guest_fpu_loaded = 1;
 	__kernel_fpu_begin();
-	__copy_kernel_to_fpregs(&vcpu->arch.guest_fpu.state);
+	/* PKRU is separately restored in kvm_x86_ops->run.  */
+	__copy_kernel_to_fpregs(&vcpu->arch.guest_fpu.state,
+				~XFEATURE_MASK_PKRU);
 	trace_kvm_fpu(1);
 }
 

Patches currently in stable-queue which might be from [email protected] are

queue-4.9/kvm-x86-block-guest-protection-keys-unless-the-host-has-them-enabled.patch queue-4.9/kvm-pkeys-do-not-use-pkru-value-in-vcpu-arch.guest_fpu.state.patch