[PATCH 4/5] x86/fpu: Add consistency check between xstate_size and xfeatures

[Andrei Vagin]

The signal frame is designed to be self-describing, where xstate_size
indicates the actual size of the xstate context. The kernel previously
lacked a check to ensure that the provided xstate_size was sufficient
for the features enabled in the xfeatures mask. Additionally,
restore_fpregs_from_user() always used the default xstate_size to
fault in the xstate user buffer.

These consistency checks have been added:
* Validate that xfeatures is a subset of the features enabled for the
  task.
* Calculate the required size for the validated xfeatures mask.
* Ensure the provided xstate_size is sufficient.

These checks prevent the kernel from attempting to fault in memory past the
end of a frame.

Signed-off-by: Andrei Vagin <avagin@xxxxxxxxxx>
---
 arch/x86/kernel/fpu/signal.c | 26 ++++++++++++++++++++------
 arch/x86/kernel/fpu/xstate.c |  2 +-
 arch/x86/kernel/fpu/xstate.h |  2 ++
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 20b638c507ca..5baba4101f1d 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -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);
 	}
 
 	/*
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index a7b6524a9dea..371a3a4a4b4e 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -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;
diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h
index 38a2862f09d3..c73cf2444de6 100644
--- a/arch/x86/kernel/fpu/xstate.h
+++ b/arch/x86/kernel/fpu/xstate.h
@@ -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)
-- 
2.54.0.746.g67dd491aae-goog