x86/fpu: Restore and reinforce signal frame portability

index c3ec2512f2bb..20b638c507ca 100644
@@ -27,14 +27,19 @@
 static inline bool check_xstate_in_sigframe(struct fxregs_state __user *fxbuf,
 					    struct _fpx_sw_bytes *fx_sw)
 {
+	int min_xstate_size = sizeof(struct fxregs_state) +
+			      sizeof(struct xstate_header);
 	void __user *fpstate = fxbuf;
 	unsigned int magic2;
 
 	if (__copy_from_user(fx_sw, &fxbuf->sw_reserved[0], sizeof(*fx_sw)))
 		return false;
 
-	/* Check for the first magic field */
-	if (fx_sw->magic1 != FP_XSTATE_MAGIC1)
+	/* Check for the first magic field and other error scenarios. */
+	if (fx_sw->magic1 != FP_XSTATE_MAGIC1 ||
+	    fx_sw->xstate_size < min_xstate_size ||
+	    fx_sw->xstate_size > x86_task_fpu(current)->fpstate->user_size ||
+	    fx_sw->xstate_size > fx_sw->extended_size)
 		goto setfx;
 
 	/*

@@ -43,7 +48,7 @@ static inline bool check_xstate_in_sigframe(struct fxregs_state __user *fxbuf,
 	 * fpstate layout with out copying the extended state information
 	 * in the memory layout.
 	 */
-	if (__get_user(magic2, (__u32 __user *)(fpstate + x86_task_fpu(current)->fpstate->user_size)))
+	if (__get_user(magic2, (__u32 __user *)(fpstate + fx_sw->xstate_size)))
 		return false;
 
 	if (likely(magic2 == FP_XSTATE_MAGIC2))

index d0d9b331d3a1..f3d10f804f42 100644
@@ -31,8 +31,17 @@
  * If sw_reserved.magic1 == FP_XSTATE_MAGIC1 then there's a
  * sw_reserved.extended_size bytes large extended context area present. (The
  * last 32-bit word of this extended area (at the
- * fpstate+extended_size-FP_XSTATE_MAGIC2_SIZE address) is set to
- * FP_XSTATE_MAGIC2 so that you can sanity check your size calculations.)
+ * fpstate+xstate_size address) is set to FP_XSTATE_MAGIC2 so that you
+ * can sanity check your size calculations.)
+ *
+ * The xstate_size field indicates the actual size of the xstate context
+ * (including the fxregs_state and xstate_header). This size is used to
+ * locate FP_XSTATE_MAGIC2. This makes the signal frame self-describing
+ * and portable: a signal frame created on a machine with a certain set
+ * of xstate features can be restored on a machine with a different (larger)
+ * set of features, as long as the latter supports all features present in
+ * the frame. Note that this portability is generally limited to CPUs of the
+ * same vendor, as different vendors may use different xstate layouts.
  *
  * This extended area typically grows with newer CPUs that have larger and
  * larger XSAVE areas.

143 lines not on this page · raw message

index 97fe4bd8bc77..40062b28c001 100644
@@ -42,11 +42,6 @@ static inline uint64_t xgetbv(uint32_t index)
 	return eax + ((uint64_t)edx << 32);
 }
 
-static inline uint64_t get_xstatebv(struct xsave_buffer *xbuf)
-{
-	return *(uint64_t *)(&xbuf->header);
-}
-
 static struct xstate_info xstate;
 
 struct futex_info {

index 6ee816e7625a..b24501b621f3 100644
@@ -3,6 +3,8 @@
 #define __SELFTESTS_X86_XSTATE_H
 
 #include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
 
 #include "kselftest.h"
 

@@ -160,6 +162,11 @@ static inline void set_xstatebv(struct xsave_buffer *xbuf, uint64_t bv)
 	*(uint64_t *)(&xbuf->header) = bv;
 }
 
+static inline uint64_t get_xstatebv(struct xsave_buffer *xbuf)
+{
+	return *(uint64_t *)(&xbuf->header);
+}
+
 /* See 'struct _fpx_sw_bytes' at sigcontext.h */
 #define SW_BYTES_OFFSET		464
 /* N.B. The struct's field name varies so read from the offset. */

index 20b638c507ca..5baba4101f1d 100644
@@ -29,7 +29,8 @@ static inline bool check_xstate_in_sigframe(struct fxregs_state __user *fxbuf,
 {
 	int min_xstate_size = sizeof(struct fxregs_state) +
 			      sizeof(struct xstate_header);
-	void __user *fpstate = fxbuf;
+	struct fpstate *fpstate = x86_task_fpu(current)->fpstate;
+	void __user *sig_fpstate = fxbuf;
 	unsigned int magic2;
 
 	if (__copy_from_user(fx_sw, &fxbuf->sw_reserved[0], sizeof(*fx_sw)))

@@ -37,8 +38,9 @@ static inline bool check_xstate_in_sigframe(struct fxregs_state __user *fxbuf,
 
 	/* Check for the first magic field and other error scenarios. */
 	if (fx_sw->magic1 != FP_XSTATE_MAGIC1 ||
+	    (fx_sw->xfeatures & ~fpstate->user_xfeatures) ||
 	    fx_sw->xstate_size < min_xstate_size ||
-	    fx_sw->xstate_size > x86_task_fpu(current)->fpstate->user_size ||
+	    fx_sw->xstate_size > fpstate->user_size ||
 	    fx_sw->xstate_size > fx_sw->extended_size)
 		goto setfx;
 

@@ -48,9 +50,17 @@ static inline bool check_xstate_in_sigframe(struct fxregs_state __user *fxbuf,
 	 * fpstate layout with out copying the extended state information
 	 * in the memory layout.
 	 */
-	if (__get_user(magic2, (__u32 __user *)(fpstate + fx_sw->xstate_size)))
+	if (__get_user(magic2, (__u32 __user *)(sig_fpstate + fx_sw->xstate_size)))
 		return false;
 
+	if (fx_sw->xstate_size != fpstate->user_size) {
+		unsigned int calculated_size;
+
+		calculated_size = xstate_calculate_size(fx_sw->xfeatures, false);
+		if (fx_sw->xstate_size < calculated_size)
+			goto setfx;
+	}
+
 	if (likely(magic2 == FP_XSTATE_MAGIC2))
 		return true;
 setfx:

@@ -266,7 +276,8 @@ static int __restore_fpregs_from_user(void __user *buf, u64 ufeatures,
  * Attempt to restore the FPU registers directly from user memory.
  * Pagefaults are handled and any errors returned are fatal.
  */
-static bool restore_fpregs_from_user(void __user *buf, u64 xrestore, bool fx_only)
+static bool restore_fpregs_from_user(void __user *buf, u64 xrestore,
+				     bool fx_only, size_t xstate_size)
 {
 	struct fpu *fpu = x86_task_fpu(current);
 	int ret;

@@ -302,7 +313,7 @@ static bool restore_fpregs_from_user(void __user *buf, u64 xrestore, bool fx_onl
 		if (ret != X86_TRAP_PF)
 			return false;
 
-		if (!fault_in_readable(buf, fpu->fpstate->user_size))
+		if (!fault_in_readable(buf, xstate_size))
 			goto retry;
 		return false;
 	}

@@ -333,6 +344,7 @@ static bool __fpu_restore_sig(void __user *buf, void __user *buf_fx,
 	bool success, fx_only = false;
 	union fpregs_state *fpregs;
 	u64 user_xfeatures = 0;
+	size_t xstate_size;
 
 	if (use_xsave()) {
 		struct _fpx_sw_bytes fx_sw_user;

@@ -342,13 +354,15 @@ static bool __fpu_restore_sig(void __user *buf, void __user *buf_fx,
 
 		fx_only = !fx_sw_user.magic1;
 		user_xfeatures = fx_sw_user.xfeatures;
+		xstate_size = fx_sw_user.xstate_size;
 	} else {
 		user_xfeatures = XFEATURE_MASK_FPSSE;
+		xstate_size = sizeof(struct fxregs_state);
 	}
 
 	if (likely(!ia32_fxstate)) {
 		/* Restore the FPU registers directly from user memory. */
-		return restore_fpregs_from_user(buf_fx, user_xfeatures, fx_only);
+		return restore_fpregs_from_user(buf_fx, user_xfeatures, fx_only, xstate_size);
 	}
 
 	/*

index a7b6524a9dea..371a3a4a4b4e 100644
@@ -587,7 +587,7 @@ static bool __init check_xstate_against_struct(int nr)
 	return true;
 }
 
-static unsigned int xstate_calculate_size(u64 xfeatures, bool compacted)
+unsigned int xstate_calculate_size(u64 xfeatures, bool compacted)
 {
 	unsigned int topmost = fls64(xfeatures) -  1;
 	unsigned int offset, i;

index 38a2862f09d3..c73cf2444de6 100644
@@ -55,6 +55,8 @@ extern int copy_sigframe_from_user_to_xstate(struct task_struct *tsk, const void
 extern void fpu__init_cpu_xstate(void);
 extern void fpu__init_system_xstate(unsigned int legacy_size);
 
+extern unsigned int xstate_calculate_size(u64 xfeatures, bool compacted);
+
 extern void __user *get_xsave_addr_user(struct xregs_state __user *xsave, int xfeature_nr);
 
 static inline u64 xfeatures_mask_supervisor(void)

index 8888079a153a..a60aa4d20dd1 100644
@@ -17,13 +17,14 @@
 #include "xstate.h"
 
 /*
- * This test verifies the portability of the signal frame.
- * It simulates a scenario where a signal frame is created on a system with
- * fewer xstate features and restored on a system with more features.
+ * This test verifies the portability and consistency of the signal frame.
+ * Portability: A frame created on a system with fewer features can be
+ *              restored on a system with more features.
+ * Consistency: The kernel rejects frames where xstate_size is insufficient
+ *              for the features enabled in xfeatures.
  */
 
 #define SIGFRAME_XSTATE_HDR_OFFSET	512
-
 #define XSTATE_SSE_ONLY_SIZE	(SIGFRAME_XSTATE_HDR_OFFSET + XSAVE_HDR_SIZE)
 #define XFEATURE_MASK_FPSSE	((1 << XFEATURE_FP) | (1 << XFEATURE_SSE))
 

@@ -55,7 +56,17 @@ static void check_avx_support(void)
 
 #define TEST_YMMH_VAL (0x5656565656565656UL)
 
-static void handle_signal(int sig, siginfo_t *si, void *ucp)
+static void read_ymm0(uint64_t *v)
+{
+	asm volatile ("vmovdqu %%ymm0, %0" : "=m"  (*(char (*)[32])v));
+}
+
+static void write_ymm0(uint64_t *v)
+{
+	asm volatile ("vmovdqu %0, %%ymm0" : : "m"  (*(char (*)[32])v));
+}
+
+static void handle_portability(int sig, siginfo_t *si, void *ucp)
 {
 	ucontext_t *uc = ucp;
 	void *fp = uc->uc_mcontext.fpregs;

@@ -72,18 +83,14 @@ static void handle_signal(int sig, siginfo_t *si, void *ucp)
 
 	xbuf = (struct xsave_buffer *)fp;
 
-	if (sw->xstate_size < xstate_size_ymm) {
-		snprintf(sig_err_buf, SIGNAL_BUF_LEN,
-			 "xstate size is too small: %d (expected %d or greater)",
-			 sw->xstate_size, xstate_size_ymm);
-		return;
-	}
-
+	/* Shrink the frame to just YMM size */
 	sw->xstate_size = xstate_size_ymm;
 
 	xfeatures = get_xstatebv(xbuf);
 	xfeatures &= XFEATURE_MASK_FPSSE | (1 << XFEATURE_YMM);
 	set_xstatebv(xbuf, xfeatures);
+	/* Also update sw->xfeatures as the kernel relies on it */
+	set_fpx_sw_bytes_features(fp, xfeatures);
 
 	*(uint32_t *)(fp + sw->xstate_size) = FP_XSTATE_MAGIC2;
 

@@ -96,26 +103,12 @@ static void handle_signal(int sig, siginfo_t *si, void *ucp)
 		memset(fp + sw->xstate_size + 4, 0, sw->extended_size - sw->xstate_size - 4);
 }
 
-static void read_ymm0(uint64_t *v)
-{
-	asm volatile ("vmovdqu %%ymm0, %0" : "=m"  (*(char (*)[32])v));
-}
-
-static void write_ymm0(uint64_t *v)
-{
-	asm volatile ("vmovdqu %0, %%ymm0" : : "m"  (*(char (*)[32])v));
-}
-
-int main(void)
+static void test_portability(void)
 {
 	uint64_t v[4] = {0, 0, 0, 0};
 
-	ksft_print_header();
-	ksft_set_plan(1);
-
-	check_avx_support();
-
-	sethandler(SIGUSR1, handle_signal, 0);
+	sig_err_buf[0] = 0;
+	sethandler(SIGUSR1, handle_portability, 0);
 
 	v[0] = 0x1111111111111111ULL;
 	v[1] = 0x2222222222222222ULL;

@@ -130,13 +123,66 @@ int main(void)
 	if (sig_err_buf[0])
 		ksft_test_result_fail("%s\n", sig_err_buf);
 	else if (v[2] == TEST_YMMH_VAL && v[3] == (TEST_YMMH_VAL + 1))
-		ksft_test_result_pass("YMM state restored correctly\n");
+		ksft_test_result_pass("YMM state restored correctly from shrunk frame\n");
 	else
 		ksft_test_result_fail(
 				"Got upper bits: 0x%lx 0x%lx (expected %lx %lx)\n",
 			       v[2], v[3], TEST_YMMH_VAL, TEST_YMMH_VAL + 1);
 
 	clearhandler(SIGUSR1);
+}
+
+static void handle_consistency(int sig, siginfo_t *si, void *ucp)
+{
+	ucontext_t *uc = ucp;
+	void *fp = uc->uc_mcontext.fpregs;
+	struct _fpx_sw_bytes *sw = get_fpx_sw_bytes(fp);
+
+	/* The origin frame contains an AVX state. */
+	sw->xstate_size = XSTATE_SSE_ONLY_SIZE;
+
+	*(uint32_t *)(fp + sw->xstate_size) = FP_XSTATE_MAGIC2;
+}
+
+static void test_consistency(void)
+{
+	uint64_t v[4] = {0, 0, 0, 0};
+
+	sig_err_buf[0] = 0;
+	sethandler(SIGUSR2, handle_consistency, 0);
+
+	v[0] = 0x1111111111111111ULL;
+	v[1] = 0x2222222222222222ULL;
+	v[2] = 0x3333333333333333ULL;
+	v[3] = 0x4444444444444444ULL;
+	write_ymm0(v);
+
+	raise(SIGUSR2);
+	v[0] = v[1] = v[2] = v[3] = 0;
+	read_ymm0(v);
+
+	/*
+	 * When inconsistent, the kernel should have fallen back to
+	 * FX-only mode, so YMM upper bits should be zero (init state).
+	 */
+	if (v[2] == 0 && v[3] == 0)
+		ksft_test_result_pass("Inconsistent size correctly rejected\n");
+	else
+		ksft_test_result_fail("Inconsistent size was NOT rejected: 0x%lx 0x%lx\n",
+				      v[2], v[3]);
+
+	clearhandler(SIGUSR2);
+}
+
+int main(void)
+{
+	ksft_print_header();
+	ksft_set_plan(2);
+
+	check_avx_support();
+
+	test_portability();
+	test_consistency();
 
 	ksft_finished();
 	return 0;

index b24501b621f3..c531667b66ad 100644
@@ -182,6 +182,11 @@ static inline uint64_t get_fpx_sw_bytes_features(void *buffer)
 	return *(uint64_t *)(buffer + SW_BYTES_BV_OFFSET);
 }
 
+static inline void set_fpx_sw_bytes_features(void *buffer, uint64_t features)
+{
+	*(uint64_t *)(buffer + SW_BYTES_BV_OFFSET) = features;
+}
+
 static inline void set_rand_data(struct xstate_info *xstate, struct xsave_buffer *xbuf)
 {
 	int *ptr = (int *)&xbuf->bytes[xstate->xbuf_offset];