Page MenuHomePhabricator

[NativeProcessLinux] Reuse memory read by process_vm_readv before calling ptrace
Needs ReviewPublic

Authored by aadsm on May 30 2019, 7:44 PM.

Details

Summary

I'm putting this up as discussed per D62503.
Today when process_vm_readv fails to read the entire range we fallback to ptrace and try to read the same range.
However, process_vm_readv does partial reads so there's no need to read again what has already been read by process_vm_readv.

Event Timeline

aadsm created this revision.May 30 2019, 7:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 30 2019, 7:44 PM

Thinking about tests, do you know of any way to create memory which is readable by ptrace, but it is not accessible via process_vm_readv ? I know that latest android phones have memory like that, but I believe this depends on selinux or some other mechanism which cannot be reproduced in a test.

If we can't do anything like that, we should at least have an lldb-server test that performs a read ($m packet) crossing page boundaries, where one of the pages is unmapped. Probably the easiest way to guarantee that a particular page will be unmapped is to first mmap 2 pages worth of memory, and then munmap one of them.

lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
1457

llvm's policy is to *not* "almost always use auto". And it's nice to know whether this returns size_t, ssize_t, or something else without looking up the documentation.

1467

addr += vm_bytes_read ?
Since this is talking about addresses in the other process, I can't imagine that casting to a char * is going to make this more "correct" in any way.

aadsm marked 2 inline comments as done.May 31 2019, 7:26 AM
aadsm added inline comments.
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
1457

Makes sense. I'll make sure to take a more careful read at https://llvm.org/docs/CodingStandards.html.

1467

For some reason I thought that would increase addr by vm_bytes_read*4 or 8 but it's really just a uint64_t not a pointer. I'll also update ReadCStringFromMemory function on my other diff then, I believe I make the same logic there.

aadsm added a comment.May 31 2019, 7:27 AM

That's a good idea for the tests, will look into that.

That's a good idea for the tests, will look into that.

Actually, it looks like getting process_vm_readv to fail is as simple as running mprotect(..., PROT_NONE) on the piece of memory. Here's my test program, in case you find it useful for anything:

#include <cassert>
#include <cerrno>
#include <cstdio>
#include <cstring>
#include <sys/mman.h>
#include <sys/prctl.h>
#include <sys/ptrace.h>
#include <sys/uio.h>
#include <sys/wait.h>
#include <unistd.h>

long *mem;

void child() {
  assert(ptrace(PTRACE_TRACEME, 0, 0, 0) == 0);
  raise(SIGSTOP);
  _exit(0);
}

void parent(pid_t child) {
  int status;
  assert(waitpid(child, &status, __WALL) == child);
  assert(WIFSTOPPED(status));
  assert(WSTOPSIG(status) == SIGSTOP);
  unsigned long myx;
  struct iovec local;
  local.iov_base = &myx;
  local.iov_len = sizeof myx;

  struct iovec remote;
  remote.iov_base = mem;
  remote.iov_len = sizeof myx;

  ssize_t s = process_vm_readv(child, &local, 1, &remote, 1, 0);
  fprintf(stderr, "readv: %zd (%d - %s)\n", s, errno, strerror(errno));
  myx = ptrace(PTRACE_PEEKDATA, child, mem, 0);
  fprintf(stderr, "peek: %lx (%d - %s)\n", myx, errno, strerror(errno));
  _exit(0);
}

int main() {
  mem = reinterpret_cast<long *>(mmap(nullptr, 0x1000, PROT_READ | PROT_WRITE,
                                      MAP_PRIVATE | MAP_ANONYMOUS, 0, 0));
  assert(mem != MAP_FAILED);
  *mem = 0x424344454647;
  assert(mprotect(mem, 0x1000, PROT_NONE) == 0);

  pid_t pid = fork();
  assert(pid != -1);
  if (pid)
    parent(pid);
  else
    child();
}


$ g++ a.cc
$ ./a.out 
readv: -1 (14 - Bad address)
peek: 424344454647 (0 - Success)