Skip to content

Commit

Permalink
Set GPRs through PTRACE_SETREGSET.
Browse files Browse the repository at this point in the history
When the GPRs are manipulated through PTRACE_GETREGS/PTRACE_SETREGS, whether the register set is the 32 bit user_regs_struct or the 64 bit user_regs_struct depends on whether rr is a 32 bit or 64 bit program. But when they're manipulated through PTRACE_GETREGSET/PTRACE_SETREGSET with the NT_PRSTATUS regset, it depends on whether the *tracee* is a 32 bit or 64 bit program.

Starting with kernel 5.9, if a 64 bit rr PTRACE_SETREGS a 32 bit tracee the fs/gsbase values in that user_regs_struct are used. This is a problem because we don't track them and thus they're always zero, regardless of what the correct value is. See LKML "x86/cpu fsgsbase breaks TLS in 32 bit rr tracees on a 64 bit system" for more discussion.

If we use PTRACE_SETREGSET we can pass in the 32 bit version of user_regs_struct and the kernel will figure out the correct fs/gsbase as before.

It's still more convenient to use PTRACE_GETREGS rather than PTRACE_GETREGSET so we can be sure where the CS register is in memory for testing whether we have a 64/32 bit process, so that and the conversion code stick around.

Fixes #2642 (again).
  • Loading branch information
khuey committed Aug 25, 2020
1 parent 2600795 commit 5c12d5f
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 10 deletions.
11 changes: 11 additions & 0 deletions src/Registers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,17 @@ NativeArch::user_regs_struct Registers::get_ptrace() const {
return result.linux_api;
}

iovec Registers::get_ptrace_iovec() {
if (arch() == NativeArch::arch()) {
iovec iov = { &u, sizeof(NativeArch::user_regs_struct) };
return iov;
}

DEBUG_ASSERT(arch() == x86 && NativeArch::arch() == x86_64);
iovec iov = { &u.x86regs, sizeof(u.x86regs) };
return iov;
}

Registers::InternalData Registers::get_ptrace_for_self_arch() const {
switch (arch_) {
case x86:
Expand Down
3 changes: 3 additions & 0 deletions src/Registers.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
#include "remote_code_ptr.h"
#include "remote_ptr.h"

struct iovec;

namespace rr {

class ReplayTask;
Expand Down Expand Up @@ -86,6 +88,7 @@ class Registers {
* to the rr build (e.g. ARM vs x86).
*/
NativeArch::user_regs_struct get_ptrace() const;
iovec get_ptrace_iovec();

/**
* Get a user_regs_struct for a particular Arch from these Registers.
Expand Down
12 changes: 6 additions & 6 deletions src/Task.cc
Original file line number Diff line number Diff line change
Expand Up @@ -673,17 +673,17 @@ void Task::on_syscall_exit_arch(int syscallno, const Registers& regs) {
switch ((int)regs.arg3()) {
case NT_PRSTATUS: {
auto set = ptrace_get_regs_set<Arch>(
this, regs, sizeof(typename Arch::user_regs_struct));
this, regs, user_regs_struct_size(tracee->arch()));
Registers r = tracee->regs();
r.set_from_ptrace_for_arch(Arch::arch(), set.data(), set.size());
r.set_from_ptrace_for_arch(tracee->arch(), set.data(), set.size());
tracee->set_regs(r);
break;
}
case NT_FPREGSET: {
auto set = ptrace_get_regs_set<Arch>(
this, regs, sizeof(typename Arch::user_fpregs_struct));
this, regs, user_fpregs_struct_size(tracee->arch()));
ExtraRegisters r = tracee->extra_regs();
r.set_user_fpregs_struct(this, Arch::arch(), set.data(),
r.set_user_fpregs_struct(this, tracee->arch(), set.data(),
set.size());
tracee->set_extra_regs(r);
break;
Expand Down Expand Up @@ -1442,9 +1442,9 @@ void Task::set_regs(const Registers& regs) {
void Task::flush_regs() {
if (registers_dirty) {
LOG(debug) << "Flushing registers for tid " << tid << " " << registers;
auto ptrace_regs = registers.get_ptrace();
auto ptrace_regs = registers.get_ptrace_iovec();
#if defined(__i386__) || defined(__x86_64__)
if (ptrace_if_alive(PTRACE_SETREGS, nullptr, &ptrace_regs)) {
if (ptrace_if_alive(PTRACE_SETREGSET, NT_PRSTATUS, &ptrace_regs)) {
/* It's ok for flush regs to fail, e.g. if the task got killed underneath
* us - we just need to remember not to trust any value we would load
* from ptrace otherwise */
Expand Down
16 changes: 16 additions & 0 deletions src/kernel_abi.cc
Original file line number Diff line number Diff line change
Expand Up @@ -335,4 +335,20 @@ template <typename Arch> static size_t sigaction_sigset_size_arch() {
size_t sigaction_sigset_size(SupportedArch arch) {
RR_ARCH_FUNCTION(sigaction_sigset_size_arch, arch);
}

template <typename Arch> static size_t user_regs_struct_size_arch() {
return sizeof(typename Arch::user_regs_struct);
}

size_t user_regs_struct_size(SupportedArch arch) {
RR_ARCH_FUNCTION(user_regs_struct_size_arch, arch)
}

template <typename Arch> static size_t user_fpregs_struct_size_arch() {
return sizeof(typename Arch::user_fpregs_struct);
}

size_t user_fpregs_struct_size(SupportedArch arch) {
RR_ARCH_FUNCTION(user_fpregs_struct_size_arch, arch)
}
}
3 changes: 3 additions & 0 deletions src/kernel_abi.h
Original file line number Diff line number Diff line change
Expand Up @@ -2224,6 +2224,9 @@ void set_arch_siginfo(const siginfo_t& siginfo, SupportedArch a, void* dest,

size_t sigaction_sigset_size(SupportedArch arch);

size_t user_regs_struct_size(SupportedArch arch);
size_t user_fpregs_struct_size(SupportedArch arch);

#if defined(__i386__)
typedef X86Arch NativeArch;
#elif defined(__x86_64__)
Expand Down
8 changes: 4 additions & 4 deletions src/record_syscall.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2585,7 +2585,7 @@ static Switchable prepare_ptrace(RecordTask* t,
case NT_PRSTATUS: {
RecordTask* tracee = verify_ptrace_target(t, syscall_state, pid);
if (tracee) {
auto regs = tracee->regs().get_ptrace_for_arch(Arch::arch());
auto regs = tracee->regs().get_ptrace_for_arch(tracee->arch());
ptrace_get_reg_set<Arch>(t, syscall_state, regs);
}
break;
Expand All @@ -2594,7 +2594,7 @@ static Switchable prepare_ptrace(RecordTask* t,
RecordTask* tracee = verify_ptrace_target(t, syscall_state, pid);
if (tracee) {
auto regs =
tracee->extra_regs().get_user_fpregs_struct(Arch::arch());
tracee->extra_regs().get_user_fpregs_struct(tracee->arch());
ptrace_get_reg_set<Arch>(t, syscall_state, regs);
}
break;
Expand Down Expand Up @@ -2667,15 +2667,15 @@ static Switchable prepare_ptrace(RecordTask* t,
RecordTask* tracee = verify_ptrace_target(t, syscall_state, pid);
if (tracee) {
ptrace_verify_set_reg_set<Arch>(
t, sizeof(typename Arch::user_regs_struct), syscall_state);
t, user_regs_struct_size(tracee->arch()), syscall_state);
}
break;
}
case NT_FPREGSET: {
RecordTask* tracee = verify_ptrace_target(t, syscall_state, pid);
if (tracee) {
ptrace_verify_set_reg_set<Arch>(
t, sizeof(typename Arch::user_fpregs_struct), syscall_state);
t, user_fpregs_struct_size(tracee->arch()), syscall_state);
}
break;
}
Expand Down

0 comments on commit 5c12d5f

Please sign in to comment.