summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorMichael Pratt <mpratt@google.com>2018-12-10 12:36:27 -0800
committerShentubot <shentubot@google.com>2018-12-10 12:37:16 -0800
commit99d595869332f817de8f570fae184658c513a43c (patch)
tree0e1309f278df51d47a4059ab9c1ef5c3b37f14ca
parent25b8424d754bd659a0f976f82f7c8846dc2a194f (diff)
Validate FS_BASE in Task.Clone
arch_prctl already verified that the new FS_BASE was canonical, but Task.Clone did not. Centralize these checks in the arch packages. Failure to validate could cause an error in PTRACE_SET_REGS when we try to switch to the app. PiperOrigin-RevId: 224862398 Change-Id: Iefe63b3f9aa6c4810326b8936e501be3ec407f14
-rw-r--r--pkg/sentry/arch/arch.go6
-rw-r--r--pkg/sentry/arch/arch_amd64.go16
-rw-r--r--pkg/sentry/arch/arch_x86.go10
-rw-r--r--pkg/sentry/kernel/task_clone.go4
-rw-r--r--pkg/sentry/platform/ptrace/subprocess.go4
-rw-r--r--pkg/sentry/syscalls/linux/sys_tls.go9
6 files changed, 38 insertions, 11 deletions
diff --git a/pkg/sentry/arch/arch.go b/pkg/sentry/arch/arch.go
index 575b7ba66..4cd7a9af5 100644
--- a/pkg/sentry/arch/arch.go
+++ b/pkg/sentry/arch/arch.go
@@ -115,6 +115,12 @@ type Context interface {
// SetStack sets the current stack pointer.
SetStack(value uintptr)
+ // TLS returns the current TLS pointer.
+ TLS() uintptr
+
+ // SetTLS sets the current TLS pointer. Returns false if value is invalid.
+ SetTLS(value uintptr) bool
+
// SetRSEQInterruptedIP sets the register that contains the old IP when a
// restartable sequence is interrupted.
SetRSEQInterruptedIP(value uintptr)
diff --git a/pkg/sentry/arch/arch_amd64.go b/pkg/sentry/arch/arch_amd64.go
index bb80a7bed..2507774f7 100644
--- a/pkg/sentry/arch/arch_amd64.go
+++ b/pkg/sentry/arch/arch_amd64.go
@@ -158,6 +158,22 @@ func (c *context64) SetStack(value uintptr) {
c.Regs.Rsp = uint64(value)
}
+// TLS returns the current TLS pointer.
+func (c *context64) TLS() uintptr {
+ return uintptr(c.Regs.Fs_base)
+}
+
+// SetTLS sets the current TLS pointer. Returns false if value is invalid.
+func (c *context64) SetTLS(value uintptr) bool {
+ if !isValidSegmentBase(uint64(value)) {
+ return false
+ }
+
+ c.Regs.Fs = 0
+ c.Regs.Fs_base = uint64(value)
+ return true
+}
+
// SetRSEQInterruptedIP implements Context.SetRSEQInterruptedIP.
func (c *context64) SetRSEQInterruptedIP(value uintptr) {
c.Regs.R10 = uint64(value)
diff --git a/pkg/sentry/arch/arch_x86.go b/pkg/sentry/arch/arch_x86.go
index 59bf89d99..e50a76083 100644
--- a/pkg/sentry/arch/arch_x86.go
+++ b/pkg/sentry/arch/arch_x86.go
@@ -353,10 +353,10 @@ func (s *State) PtraceSetRegs(src io.Reader) (int, error) {
if !isUserSegmentSelector(regs.Ss) {
return 0, syscall.EIO
}
- if regs.Fs_base >= uint64(maxAddr64) {
+ if !isValidSegmentBase(regs.Fs_base) {
return 0, syscall.EIO
}
- if regs.Gs_base >= uint64(maxAddr64) {
+ if !isValidSegmentBase(regs.Gs_base) {
return 0, syscall.EIO
}
// CS and SS are validated, but changes to them are otherwise silently
@@ -389,6 +389,12 @@ func isUserSegmentSelector(reg uint64) bool {
return reg&3 == 3
}
+// isValidSegmentBase returns true if the given segment base specifies a
+// canonical user address.
+func isValidSegmentBase(reg uint64) bool {
+ return reg < uint64(maxAddr64)
+}
+
// ptraceFPRegsSize is the size in bytes of Linux's user_i387_struct, the type
// manipulated by PTRACE_GETFPREGS and PTRACE_SETFPREGS on x86. Equivalently,
// ptraceFPRegsSize is the size in bytes of the x86 FXSAVE area.
diff --git a/pkg/sentry/kernel/task_clone.go b/pkg/sentry/kernel/task_clone.go
index 755fe0370..b66fa34a9 100644
--- a/pkg/sentry/kernel/task_clone.go
+++ b/pkg/sentry/kernel/task_clone.go
@@ -210,7 +210,9 @@ func (t *Task) Clone(opts *CloneOptions) (ThreadID, *SyscallControl, error) {
tc.Arch.SetStack(uintptr(opts.Stack))
}
if opts.SetTLS {
- tc.Arch.StateData().Regs.Fs_base = uint64(opts.TLS)
+ if !tc.Arch.SetTLS(uintptr(opts.TLS)) {
+ return 0, nil, syserror.EPERM
+ }
}
var fsc *FSContext
diff --git a/pkg/sentry/platform/ptrace/subprocess.go b/pkg/sentry/platform/ptrace/subprocess.go
index 5e56a1514..a9d083f5a 100644
--- a/pkg/sentry/platform/ptrace/subprocess.go
+++ b/pkg/sentry/platform/ptrace/subprocess.go
@@ -480,10 +480,10 @@ func (s *subprocess) switchToApp(c *context, ac arch.Context) bool {
// Set registers.
if err := t.setRegs(regs); err != nil {
- panic(fmt.Sprintf("ptrace set regs failed: %v", err))
+ panic(fmt.Sprintf("ptrace set regs (%+v) failed: %v", regs, err))
}
if err := t.setFPRegs(fpState, uint64(fpLen), useXsave); err != nil {
- panic(fmt.Sprintf("ptrace set fpregs failed: %v", err))
+ panic(fmt.Sprintf("ptrace set fpregs (%+v) failed: %v", fpState, err))
}
for {
diff --git a/pkg/sentry/syscalls/linux/sys_tls.go b/pkg/sentry/syscalls/linux/sys_tls.go
index 40e84825b..8ea78093b 100644
--- a/pkg/sentry/syscalls/linux/sys_tls.go
+++ b/pkg/sentry/syscalls/linux/sys_tls.go
@@ -22,7 +22,6 @@ import (
"gvisor.googlesource.com/gvisor/pkg/abi/linux"
"gvisor.googlesource.com/gvisor/pkg/sentry/arch"
"gvisor.googlesource.com/gvisor/pkg/sentry/kernel"
- "gvisor.googlesource.com/gvisor/pkg/sentry/usermem"
)
// ArchPrctl implements linux syscall arch_prctl(2).
@@ -31,19 +30,17 @@ func ArchPrctl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Sys
switch args[0].Int() {
case linux.ARCH_GET_FS:
addr := args[1].Pointer()
- _, err := t.CopyOut(addr, &t.Arch().StateData().Regs.Fs_base)
+ fsbase := t.Arch().TLS()
+ _, err := t.CopyOut(addr, uint64(fsbase))
if err != nil {
return 0, nil, err
}
case linux.ARCH_SET_FS:
fsbase := args[1].Uint64()
- if _, ok := t.MemoryManager().CheckIORange(usermem.Addr(fsbase), 0); !ok {
+ if !t.Arch().SetTLS(uintptr(fsbase)) {
return 0, nil, syscall.EPERM
}
- regs := &t.Arch().StateData().Regs
- regs.Fs = 0
- regs.Fs_base = fsbase
case linux.ARCH_GET_GS, linux.ARCH_SET_GS:
t.Kernel().EmitUnimplementedEvent(t)