diff options
author | Michael Pratt <mpratt@google.com> | 2018-12-10 12:36:27 -0800 |
---|---|---|
committer | Shentubot <shentubot@google.com> | 2018-12-10 12:37:16 -0800 |
commit | 99d595869332f817de8f570fae184658c513a43c (patch) | |
tree | 0e1309f278df51d47a4059ab9c1ef5c3b37f14ca | |
parent | 25b8424d754bd659a0f976f82f7c8846dc2a194f (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.go | 6 | ||||
-rw-r--r-- | pkg/sentry/arch/arch_amd64.go | 16 | ||||
-rw-r--r-- | pkg/sentry/arch/arch_x86.go | 10 | ||||
-rw-r--r-- | pkg/sentry/kernel/task_clone.go | 4 | ||||
-rw-r--r-- | pkg/sentry/platform/ptrace/subprocess.go | 4 | ||||
-rw-r--r-- | pkg/sentry/syscalls/linux/sys_tls.go | 9 |
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) |