From 4adc33ad0d0b31fb66d6d97a7b86491bfef47537 Mon Sep 17 00:00:00 2001
From: Andrei Vagin <avagin@google.com>
Date: Mon, 10 Jul 2023 13:51:48 -0700
Subject: [PATCH] fpu: avoid user stack corruptions

When we reserve space for an FPU state in a signal frame, we need to use
the size of the current fpu state. The kernel cpuid can report a smaller size.

On restore, we need to read the size of an fpu state from a signal frame.
After S/R, it can't be different from the current fpu size.

PiperOrigin-RevId: 546972243
---
 pkg/sentry/arch/fpu/fpu_amd64.go | 12 ++++++++----
 pkg/sentry/arch/signal_amd64.go  | 31 ++++++++++++++++++++++++-------
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/pkg/sentry/arch/fpu/fpu_amd64.go b/pkg/sentry/arch/fpu/fpu_amd64.go
index c394b8cfe..57b26bc7e 100644
--- a/pkg/sentry/arch/fpu/fpu_amd64.go
+++ b/pkg/sentry/arch/fpu/fpu_amd64.go
@@ -61,6 +61,9 @@ const (
 	// XFEATURE_MASK_FPSSE is xsave features that are always enabled in
 	// signal frame fpstate.
 	XFEATURE_MASK_FPSSE = 0x3
+
+	// FXSAVE_AREA_SIZE is the size of the FXSAVE area.
+	FXSAVE_AREA_SIZE = 512
 )
 
 // initX86FPState (defined in asm files) sets up initial state.
@@ -323,7 +326,7 @@ const fxsaveBV uint64 = cpuid.XSAVEFeatureX87 | cpuid.XSAVEFeatureSSE
 // AfterLoad converts the loaded state to the format that compatible with the
 // current processor.
 func (s *State) AfterLoad() {
-	old := *s
+	old := s.Slice()
 
 	// Recreate the slice. This is done to ensure that it is aligned
 	// appropriately in memory, and large enough to accommodate any new
@@ -348,8 +351,9 @@ func (s *State) AfterLoad() {
 	// (according to XSTATE_BV) which we do not support.
 	// What do we support?
 	supportedBV := fxsaveBV
-	if fs := cpuid.HostFeatureSet(); fs.UseXsave() {
-		supportedBV = fs.ValidXCR0Mask()
+	hostFeatureSet := cpuid.HostFeatureSet()
+	if hostFeatureSet.UseXsave() {
+		supportedBV = hostFeatureSet.ValidXCR0Mask()
 	}
 
 	// What was in use?
@@ -372,7 +376,7 @@ func (s *State) AfterLoad() {
 	if mxcsrBefore != mxcsrAfter {
 		panic(fmt.Sprintf("incompatible mxcsr value: %x (%x)", mxcsrBefore, mxcsrAfter))
 	}
-	if fs := cpuid.HostFeatureSet(); fs.UseXsave() {
+	if hostFeatureSet.UseXsave() {
 		if err := safecopy.CheckXstate(s.BytePointer()); err != nil {
 			panic(fmt.Sprintf("incompatible state: %s (%#v)", err, *s))
 		}
diff --git a/pkg/sentry/arch/signal_amd64.go b/pkg/sentry/arch/signal_amd64.go
index e9eca2a70..4dd7e1332 100644
--- a/pkg/sentry/arch/signal_amd64.go
+++ b/pkg/sentry/arch/signal_amd64.go
@@ -24,6 +24,7 @@ import (
 	"gvisor.dev/gvisor/pkg/abi/linux"
 	"gvisor.dev/gvisor/pkg/context"
 	"gvisor.dev/gvisor/pkg/cpuid"
+	"gvisor.dev/gvisor/pkg/errors/linuxerr"
 	"gvisor.dev/gvisor/pkg/hostarch"
 	"gvisor.dev/gvisor/pkg/marshal/primitive"
 	"gvisor.dev/gvisor/pkg/sentry/arch/fpu"
@@ -101,12 +102,9 @@ func (c *Context64) SignalSetup(st *Stack, act *linux.SigAction, info *linux.Sig
 	}
 
 	// Allocate space for floating point state on the stack.
-	fpSize, fpAlign := featureSet.ExtendedStateSize()
-	if fpSize < 512 {
-		// We expect support for at least FXSAVE.
-		fpSize = 512
-	}
-	fpSize += fpu.FP_XSTATE_MAGIC2_SIZE
+	_, fpAlign := featureSet.ExtendedStateSize()
+	fpState := c.fpState.Slice()
+	fpSize := len(fpState) + fpu.FP_XSTATE_MAGIC2_SIZE
 	fpStart := (sp - hostarch.Addr(fpSize)) & ^hostarch.Addr(fpAlign-1)
 
 	// Construct the UContext64 now since we need its size.
@@ -167,7 +165,6 @@ func (c *Context64) SignalSetup(st *Stack, act *linux.SigAction, info *linux.Sig
 		return unix.EFAULT
 	}
 
-	fpState := c.fpState.Slice()
 	// Set up floating point state on the stack. Compare Linux's
 	// arch/x86/kernel/fpu/signal.c:copy_fpstate_to_sigframe().
 	if _, err := st.IO.CopyOut(context.Background(), fpStart, fpState[:fpu.FP_SW_FRAME_OFFSET], usermem.IOOpts{}); err != nil {
@@ -277,7 +274,27 @@ func (c *Context64) SignalRestore(st *Stack, rt bool, featureSet cpuid.FeatureSe
 	if uc.MContext.Fpstate == 0 {
 		c.fpState.Reset()
 	} else {
+		fpsw := fpu.FPSoftwareFrame{}
+		st.Bottom = hostarch.Addr(uc.MContext.Fpstate + fpu.FP_SW_FRAME_OFFSET)
+		if _, err := fpsw.CopyIn(st, StackBottomMagic); err != nil {
+			c.fpState.Reset()
+			return 0, linux.SignalStack{}, err
+		}
+		if fpsw.Magic1 != fpu.FP_XSTATE_MAGIC1 ||
+			fpsw.XstateSize < fpu.FXSAVE_AREA_SIZE ||
+			fpsw.XstateSize > fpsw.ExtendedSize {
+			c.fpState.Reset()
+			return 0, linux.SignalStack{}, linuxerr.EFAULT
+		}
+
 		fpState := c.fpState.Slice()
+		fpSize := fpsw.XstateSize
+		if int(fpSize) < len(fpState) {
+			// The signal frame FPU state is smaller than expected. This can happen after S/R.
+			c.fpState.Reset()
+			fpState = fpState[:fpSize]
+		}
+
 		if _, err := st.IO.CopyIn(context.Background(), hostarch.Addr(uc.MContext.Fpstate), fpState, usermem.IOOpts{}); err != nil {
 			c.fpState.Reset()
 			return 0, linux.SignalStack{}, err