From aeabb7852781e764655feeca8f67c46be14b0bc0 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Tue, 24 Jan 2023 17:12:43 -0800 Subject: [PATCH] Allow to return an error from PullFullState. PiperOrigin-RevId: 504415294 --- pkg/sentry/kernel/task_clone.go | 7 ++++- pkg/sentry/kernel/task_run.go | 5 +++- pkg/sentry/kernel/task_signals.go | 36 +++++++++++++++++++++++-- pkg/sentry/platform/kvm/context.go | 2 +- pkg/sentry/platform/platform.go | 2 +- pkg/sentry/platform/ptrace/ptrace.go | 2 +- pkg/sentry/syscalls/linux/sys_signal.go | 22 ++------------- 7 files changed, 49 insertions(+), 27 deletions(-) diff --git a/pkg/sentry/kernel/task_clone.go b/pkg/sentry/kernel/task_clone.go index 91815c398..70540f44b 100644 --- a/pkg/sentry/kernel/task_clone.go +++ b/pkg/sentry/kernel/task_clone.go @@ -65,7 +65,12 @@ func (t *Task) Clone(args *linux.CloneArgs) (ThreadID, *SyscallControl, error) { // Pull task registers and FPU state, a cloned task will inherit the // state of the current task. - t.p.PullFullState(t.MemoryManager().AddressSpace(), t.Arch()) + if err := t.p.PullFullState(t.MemoryManager().AddressSpace(), t.Arch()); err != nil { + t.Warningf("Unable to pull a full state: %v", err) + t.forceSignal(linux.SIGILL, true /* unconditional */) + t.SendSignal(SignalInfoPriv(linux.SIGILL)) + return 0, nil, linuxerr.EFAULT + } // "If CLONE_NEWUSER is specified along with other CLONE_NEW* flags in a // single clone(2) or unshare(2) call, the user namespace is guaranteed to diff --git a/pkg/sentry/kernel/task_run.go b/pkg/sentry/kernel/task_run.go index d01ad67c1..34bc0d33a 100644 --- a/pkg/sentry/kernel/task_run.go +++ b/pkg/sentry/kernel/task_run.go @@ -247,7 +247,10 @@ func (app *runApp) execute(t *Task) taskRunState { t.Arch().ClearSingleStep() } if t.hasTracer() { - t.p.PullFullState(t.MemoryManager().AddressSpace(), t.Arch()) + if e := t.p.PullFullState(t.MemoryManager().AddressSpace(), t.Arch()); e != nil { + t.Warningf("Unable to pull a full state: %v", e) + err = e + } } switch err { diff --git a/pkg/sentry/kernel/task_signals.go b/pkg/sentry/kernel/task_signals.go index d63c9e98c..241548bc2 100644 --- a/pkg/sentry/kernel/task_signals.go +++ b/pkg/sentry/kernel/task_signals.go @@ -644,8 +644,9 @@ func (t *Task) SetSavedSignalMask(mask linux.SignalSet) { } // SignalStack returns the task-private signal stack. +// +// By precondition, a full state has to be pulled. func (t *Task) SignalStack() linux.SignalStack { - t.p.PullFullState(t.MemoryManager().AddressSpace(), t.Arch()) alt := t.signalStack if t.onSignalStack(alt) { alt.Flags |= linux.SS_ONSTACK @@ -653,6 +654,34 @@ func (t *Task) SignalStack() linux.SignalStack { return alt } +// SigaltStack implements the sigaltstack syscall. +func (t *Task) SigaltStack(setaddr hostarch.Addr, oldaddr hostarch.Addr) (*SyscallControl, error) { + if err := t.p.PullFullState(t.MemoryManager().AddressSpace(), t.Arch()); err != nil { + t.PrepareGroupExit(linux.WaitStatusTerminationSignal(linux.SIGILL)) + return CtrlDoExit, linuxerr.EFAULT + } + + alt := t.SignalStack() + if oldaddr != 0 { + if _, err := alt.CopyOut(t, oldaddr); err != nil { + return nil, err + } + } + if setaddr != 0 { + if _, err := alt.CopyIn(t, setaddr); err != nil { + return nil, err + } + // The signal stack cannot be changed if the task is currently + // on the stack. This is enforced at the lowest level because + // these semantics apply to changing the signal stack via a + // ucontext during a signal handler. + if !t.SetSignalStack(alt) { + return nil, linuxerr.EPERM + } + } + return nil, nil +} + // onSignalStack returns true if the task is executing on the given signal stack. func (t *Task) onSignalStack(alt linux.SignalStack) bool { sp := hostarch.Addr(t.Arch().Stack()) @@ -1018,7 +1047,10 @@ func (*runInterrupt) execute(t *Task) taskRunState { // Are there signals pending? if info := t.dequeueSignalLocked(linux.SignalSet(t.signalMask.RacyLoad())); info != nil { - t.p.PullFullState(t.MemoryManager().AddressSpace(), t.Arch()) + if err := t.p.PullFullState(t.MemoryManager().AddressSpace(), t.Arch()); err != nil { + t.PrepareGroupExit(linux.WaitStatusTerminationSignal(linux.SIGILL)) + return (*runExit)(nil) + } if linux.SignalSetOf(linux.Signal(info.Signo))&StopSignals != 0 { // Indicate that we've dequeued a stop signal before unlocking the diff --git a/pkg/sentry/platform/kvm/context.go b/pkg/sentry/platform/kvm/context.go index 81904e874..4c7ba7004 100644 --- a/pkg/sentry/platform/kvm/context.go +++ b/pkg/sentry/platform/kvm/context.go @@ -125,7 +125,7 @@ func (c *context) Release() {} func (c *context) FullStateChanged() {} // PullFullState implements platform.Context.PullFullState. -func (c *context) PullFullState(as platform.AddressSpace, ac *arch.Context64) {} +func (c *context) PullFullState(as platform.AddressSpace, ac *arch.Context64) error { return nil } // PrepareSleep implements platform.Context.platform.Context. func (*context) PrepareSleep() {} diff --git a/pkg/sentry/platform/platform.go b/pkg/sentry/platform/platform.go index d776ca3af..2971dc569 100644 --- a/pkg/sentry/platform/platform.go +++ b/pkg/sentry/platform/platform.go @@ -221,7 +221,7 @@ type Context interface { // PullFullState() to load all registers and FPU state. // // Preconditions: The caller must be running on the task goroutine. - PullFullState(as AddressSpace, ac *arch.Context64) + PullFullState(as AddressSpace, ac *arch.Context64) error // FullStateChanged() indicates that a thread state has been changed by // the Sentry. This happens in case of the rt_sigreturn, execve, etc. diff --git a/pkg/sentry/platform/ptrace/ptrace.go b/pkg/sentry/platform/ptrace/ptrace.go index 7e6c2fbb1..3c045c414 100644 --- a/pkg/sentry/platform/ptrace/ptrace.go +++ b/pkg/sentry/platform/ptrace/ptrace.go @@ -197,7 +197,7 @@ func (c *context) Release() {} func (c *context) FullStateChanged() {} // PullFullState implements platform.Context.PullFullState. -func (c *context) PullFullState(as platform.AddressSpace, ac *arch.Context64) {} +func (c *context) PullFullState(as platform.AddressSpace, ac *arch.Context64) error { return nil } // PrepareSleep implements platform.Context.platform.PrepareSleep. func (*context) PrepareSleep() {} diff --git a/pkg/sentry/syscalls/linux/sys_signal.go b/pkg/sentry/syscalls/linux/sys_signal.go index 80ea72d5d..9306d68e1 100644 --- a/pkg/sentry/syscalls/linux/sys_signal.go +++ b/pkg/sentry/syscalls/linux/sys_signal.go @@ -323,26 +323,8 @@ func Sigaltstack(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.S setaddr := args[0].Pointer() oldaddr := args[1].Pointer() - alt := t.SignalStack() - if oldaddr != 0 { - if _, err := alt.CopyOut(t, oldaddr); err != nil { - return 0, nil, err - } - } - if setaddr != 0 { - if _, err := alt.CopyIn(t, setaddr); err != nil { - return 0, nil, err - } - // The signal stack cannot be changed if the task is currently - // on the stack. This is enforced at the lowest level because - // these semantics apply to changing the signal stack via a - // ucontext during a signal handler. - if !t.SetSignalStack(alt) { - return 0, nil, linuxerr.EPERM - } - } - - return 0, nil, nil + ctrl, err := t.SigaltStack(setaddr, oldaddr) + return 0, ctrl, err } // Pause implements linux syscall pause(2).