From 2a56495dfaef6add3fa81e9cccd5623bb4c40d80 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Wed, 18 Jan 2023 07:32:42 -0800 Subject: [PATCH] test: check that we can change registers via ptrace PiperOrigin-RevId: 502871694 --- pkg/sentry/kernel/ptrace.go | 9 +--- pkg/sentry/kernel/ptrace_amd64.go | 6 +++ pkg/sentry/kernel/task_run.go | 3 ++ test/syscalls/linux/ptrace.cc | 87 +++++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 7 deletions(-) diff --git a/pkg/sentry/kernel/ptrace.go b/pkg/sentry/kernel/ptrace.go index e603c283a..7e009daf6 100644 --- a/pkg/sentry/kernel/ptrace.go +++ b/pkg/sentry/kernel/ptrace.go @@ -1160,8 +1160,6 @@ func (t *Task) Ptrace(req int64, pid ThreadID, addr, data hostarch.Addr) error { return err } - t.p.PullFullState(t.MemoryManager().AddressSpace(), t.Arch()) - ar := ars.Head() n, err := target.Arch().PtraceGetRegSet(uintptr(addr), &usermem.IOReadWriter{ Ctx: t, @@ -1189,13 +1187,10 @@ func (t *Task) Ptrace(req int64, pid ThreadID, addr, data hostarch.Addr) error { return err } - mm := t.MemoryManager() - t.p.PullFullState(mm.AddressSpace(), t.Arch()) - ar := ars.Head() n, err := target.Arch().PtraceSetRegSet(uintptr(addr), &usermem.IOReadWriter{ Ctx: t, - IO: mm, + IO: t.MemoryManager(), Addr: ar.Start, Opts: usermem.IOOpts{ AddressSpaceActive: true, @@ -1204,7 +1199,7 @@ func (t *Task) Ptrace(req int64, pid ThreadID, addr, data hostarch.Addr) error { if err != nil { return err } - t.p.FullStateChanged() + target.p.FullStateChanged() ar.End -= hostarch.Addr(n) return t.CopyOutIovecs(data, hostarch.AddrRangeSeqOf(ar)) diff --git a/pkg/sentry/kernel/ptrace_amd64.go b/pkg/sentry/kernel/ptrace_amd64.go index 564add01b..38e97a167 100644 --- a/pkg/sentry/kernel/ptrace_amd64.go +++ b/pkg/sentry/kernel/ptrace_amd64.go @@ -73,6 +73,9 @@ func (t *Task) ptraceArch(target *Task, req int64, addr, data hostarch.Addr) err AddressSpaceActive: true, }, }) + if err == nil { + target.p.FullStateChanged() + } return err case linux.PTRACE_SETFPREGS: @@ -85,6 +88,9 @@ func (t *Task) ptraceArch(target *Task, req int64, addr, data hostarch.Addr) err AddressSpaceActive: true, }, }, len(*s)) + if err == nil { + target.p.FullStateChanged() + } return err default: diff --git a/pkg/sentry/kernel/task_run.go b/pkg/sentry/kernel/task_run.go index aaa0c74cf..d01ad67c1 100644 --- a/pkg/sentry/kernel/task_run.go +++ b/pkg/sentry/kernel/task_run.go @@ -246,6 +246,9 @@ func (app *runApp) execute(t *Task) taskRunState { if clearSinglestep { t.Arch().ClearSingleStep() } + if t.hasTracer() { + t.p.PullFullState(t.MemoryManager().AddressSpace(), t.Arch()) + } switch err { case nil: diff --git a/test/syscalls/linux/ptrace.cc b/test/syscalls/linux/ptrace.cc index ac4982e3e..ae911b598 100644 --- a/test/syscalls/linux/ptrace.cc +++ b/test/syscalls/linux/ptrace.cc @@ -1400,6 +1400,93 @@ TEST(PtraceTest, GetRegSet) { EXPECT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) == 0) << " status " << status; } +#if defined(__x86_64__) +#define SYSNO_STR1(x) #x +#define SYSNO_STR(x) SYSNO_STR1(x) + +// Check that ptrace works properly when a target process is stopped on a +// system call that is handled via the fast path. +TEST(PtraceTest, ChangeRegSetInOptSyscall) { + constexpr uint64_t kTestRet = 0x111; + constexpr uint64_t kTestRbx1 = 0x333; + constexpr uint64_t kTestRbx2 = 0x333; + constexpr uint64_t kTestRdi = 0x555; + + pid_t const child_pid = fork(); + if (child_pid == 0) { + // In child process. + uint64_t ret, rbx = 0, rdi = kTestRdi; + + // Enable tracing. + TEST_PCHECK(ptrace(PTRACE_TRACEME, 0, 0, 0) == 0); + MaybeSave(); + + // Use kill explicitly because we check the syscall argument register below. + kill(getpid(), SIGSTOP); + + // A tested syscall has to be triggered twice, because the first call + // doesn't trigger the fast path. + for (int i = 0; i < 2; i++) { + if (i == 1) rbx = kTestRbx1; + __asm__ __volatile__( + "movl $" SYSNO_STR(SYS_getpid) ", %%eax\n" + "syscall\n" + : "=a"(ret), "=b"(rbx) + : "b"(rbx), "D"(rdi) + : "rcx", "r11", "memory"); + } + + TEST_CHECK(ret == kTestRet); + TEST_CHECK(rbx == kTestRbx2); + + _exit(0); + } + // In parent process. + ASSERT_THAT(child_pid, SyscallSucceeds()); + + // Wait for the child to send itself SIGSTOP and enter signal-delivery-stop. + int status; + ASSERT_THAT(waitpid(child_pid, &status, 0), + SyscallSucceedsWithValue(child_pid)); + EXPECT_TRUE(WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP) + << " status " << status; + + // Stop the child in the second getpid syscall. + for (int i = 0; i < 2; i++) { + ASSERT_THAT(ptrace(PTRACE_SYSEMU, child_pid, 0, 0), SyscallSucceeds()); + ASSERT_THAT(waitpid(child_pid, &status, 0), + SyscallSucceedsWithValue(child_pid)); + } + + // Get the general registers. + struct user_regs_struct regs; + struct iovec iov; + iov.iov_base = ®s; + iov.iov_len = sizeof(regs); + EXPECT_THAT(ptrace(PTRACE_GETREGSET, child_pid, NT_PRSTATUS, &iov), + SyscallSucceeds()); + + // Read exactly the full register set. + EXPECT_EQ(iov.iov_len, sizeof(regs)); + + EXPECT_EQ(regs.rax, -ENOSYS); + EXPECT_EQ(regs.orig_rax, SYS_getpid); + EXPECT_EQ(regs.rdi, kTestRdi); + EXPECT_EQ(regs.rbx, kTestRbx1); + + regs.rbx = kTestRbx2; + regs.rax = kTestRet; + EXPECT_THAT(ptrace(PTRACE_SETREGSET, child_pid, NT_PRSTATUS, &iov), + SyscallSucceeds()); + + ASSERT_THAT(ptrace(PTRACE_CONT, child_pid, 0, 0), SyscallSucceeds()); + ASSERT_THAT(waitpid(child_pid, &status, 0), + SyscallSucceedsWithValue(child_pid)); + // Let's see that process exited normally. + EXPECT_TRUE(WIFEXITED(status) && WEXITSTATUS(status) == 0) + << " status " << status; +} +#endif TEST(PtraceTest, AttachingConvertsGroupStopToPtraceStop) { pid_t const child_pid = fork();