On Tue, Jul 11, 2017 at 02:48:50PM +0300, Victor Krapivensky wrote:
> This patch implies that we have process_vm_writev if and only if we have
Rather, the check. However, I'd prefer that process_vm_write's
availability is checked separately.

> process_vm_readv, and that they are either both supported or both
> unsupported.
> 
> * defs.h: Add declaration for upoken.
> * luajit.h (func_upoke): New wrapper function.
> (init_luajit): Expose it as strace.upoke.
> * luajit_lib.lua (write_obj): New function.
> * strace.1 (LUAJIT SCRIPTING): Describe changes.
> * util.c (strace_process_vm_writev): New function.
> (vm_write_mem): Likewise.
> (partial_poke): Likewise.
> (upoken): Likewise.
> ---
>  defs.h         |   3 +
>  luajit.h       |   8 +++
>  luajit_lib.lua |  12 ++++
>  strace.1       |  15 +++++
>  util.c         | 172 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 210 insertions(+)
> 
> diff --git a/defs.h b/defs.h
> index bae22cd1..37cbe9a2 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -439,6 +439,9 @@ umoven_or_printaddr_ignore_syserror(struct tcb *, 
> kernel_ulong_t addr,
>  extern int
>  umovestr(struct tcb *, kernel_ulong_t addr, unsigned int len, char *laddr);
>  
> +extern int
> +upoken(struct tcb *, kernel_ulong_t addr, unsigned int len, const void 
> *laddr);
> +
>  extern int upeek(int pid, unsigned long, kernel_ulong_t *);
>  extern int upoke(int pid, unsigned long, kernel_ulong_t);
>  
> diff --git a/luajit.h b/luajit.h
> index 231466af..c305c369 100644
> --- a/luajit.h
> +++ b/luajit.h
> @@ -149,6 +149,12 @@ func_umove_str(kernel_ulong_t addr, size_t len, char 
> *laddr)
>       return current_tcp ? umovestr(current_tcp, addr, len, laddr) : -1;
>  }
>  
> +static int
> +func_upoke(kernel_ulong_t addr, size_t len, const void *laddr)
> +{
> +     return current_tcp ? upoken(current_tcp, addr, len, laddr) : -1;
> +}
> +
>  static bool
>  func_path_match_arr(const char **set, size_t nset)
>  {
> @@ -264,6 +270,8 @@ init_luajit(const char *scriptfile)
>               kernel_ulong_t, size_t, void *);
>       EXPOSE_FUNC(int, func_umove_str, "umove_str",
>               kernel_ulong_t, size_t, char *);
> +     EXPOSE_FUNC(int, func_upoke, "upoke",
> +             kernel_ulong_t, size_t, const void *);
>       EXPOSE_FUNC(bool, func_path_match_arr, "path_match_arr",
>               const char **, size_t);
>  
> diff --git a/luajit_lib.lua b/luajit_lib.lua
> index 9c35c0fa..ee330f0b 100644
> --- a/luajit_lib.lua
> +++ b/luajit_lib.lua
> @@ -235,6 +235,18 @@ function strace.read_path(addr)
>       return strace.read_str(addr, strace.path_max, strace.path_max + 1)
>  end
>  
> +function strace.write_obj(addr, obj)
> +     local n = ffi.sizeof(obj)
> +     -- work around FFI pointer semantics
> +     if n == ptr_size then
> +             -- it may be a pointer, and it is cheap to create a local copy
> +             obj = ffi.typeof('$ [1]', ffi.typeof(obj))(obj)
> +     end
> +     if strace.upoke(addr, n, obj) ~= 0 then
> +             error('cannot write')
> +     end
> +end
> +
>  local function register_hook(scno, pers, on_entry, on_exit, cb)
>       assert(not not strace.monitor(scno, pers, on_entry, on_exit))
>       pers = tonumber(pers)
> diff --git a/strace.1 b/strace.1
> index 1e12e13b..cb221609 100644
> --- a/strace.1
> +++ b/strace.1
> @@ -911,6 +911,15 @@ if \fIlen\fR byes were read but no NUL seen.
>  Note: there is no guarantee it won't overwrite some bytes in \fIladdr\fR 
> after
>  terminating NUL (but, of course, it never writes past \fIladdr[len-1]\fR).
>  .TP
> +\fIret\fR = \fBstrace.upoke\fR(\fIaddr\fR, \fIlen\fR, \fIladdr\fR)
> +C type:
> +.B int (*)(kernel_ulong_t, size_t, const void *)
> +.IP
> +Copies ("pokes") \fIlen\fR bytes of data from the local address \fIladdr\fR 
> to
> +the address \fIaddr\fR of the current tracee process' address space.
> +.IP
> +Returns 0 on success and \-1 on failure.
> +.TP
>  \fIstatus\fR = \fBstrace.path_match_arr\fR(\fIset\fR, \fInset\fR)
>  C type:
>  .B bool (*)(const char **, size_t)
> @@ -1044,6 +1053,12 @@ Reads a path C string from the current tracee process 
> at address \fIaddr\fR.
>  Returns a Lua string on success, \fBnil, "readerr"\fR on read error, and
>  \fBnil, "toolong"\fR if the \fBPATH_MAX\fR limit was exceeded.
>  .TP
> +\fBstrace.write_obj\fR(\fIaddr\fR, \fIobj\fR)
> +Writes a FFI cdata object \fIobj\fR to the current tracee process' address 
> space
> +at address \fIaddr\fR.
> +.IP
> +Raises an error on failure.
> +.TP
>  \fBstrace.monitor_all\fR()
>  Marks all syscalls on all personalities as to be returned from
>  \fBstrace.next_sc\fR both on entry and on exit.
> diff --git a/util.c b/util.c
> index 2ccfe4fe..007db3a2 100644
> --- a/util.c
> +++ b/util.c
> @@ -946,6 +946,20 @@ static ssize_t strace_process_vm_readv(pid_t pid,
>       return syscall(__NR_process_vm_readv, (long)pid, lvec, liovcnt, rvec, 
> riovcnt, flags);
>  }
>  # define process_vm_readv strace_process_vm_readv
> +
> +/*
> + * The same goes for process_vm_writev().
> + */
> +static ssize_t strace_process_vm_writev(pid_t pid,
> +              const struct iovec *lvec,
> +              unsigned long liovcnt,
> +              const struct iovec *rvec,
> +              unsigned long riovcnt,
> +              unsigned long flags)
Strange indentation.

> +{
> +     return syscall(__NR_process_vm_writev, (long)pid, lvec, liovcnt, rvec, 
> riovcnt, flags);
Overly long line.

> +}
> +# define process_vm_writev strace_process_vm_writev
>  #endif /* !HAVE_PROCESS_VM_READV */
>  
>  static ssize_t
> @@ -971,6 +985,29 @@ vm_read_mem(const pid_t pid, void *const laddr,
>       return process_vm_readv(pid, &local, 1, &remote, 1, 0);
>  }
>  
> +static ssize_t
> +vm_write_mem(const pid_t pid, const void *const laddr,
> +         const kernel_ulong_t raddr, const size_t len)
> +{
> +     const unsigned long truncated_raddr = raddr;
> +
> +     if (raddr != (kernel_ulong_t) truncated_raddr) {
> +             errno = EIO;
> +             return -1;
> +     }
> +
> +     const struct iovec local = {
> +             .iov_base = (void *) laddr,
> +             .iov_len = len
> +     };
> +     const struct iovec remote = {
> +             .iov_base = (void *) truncated_raddr,
> +             .iov_len = len
> +     };
> +
> +     return process_vm_writev(pid, &local, 1, &remote, 1, 0);
> +}
> +
>  /*
>   * move `len' bytes of data from process `pid'
>   * at address `addr' to our space at `our_addr'
> @@ -1267,6 +1304,141 @@ umovestr(struct tcb *const tcp, kernel_ulong_t addr, 
> unsigned int len, char *lad
>       return 0;
>  }
>  
> +static bool
> +partial_poke(int pid, kernel_ulong_t addr, unsigned int len, const void 
> *laddr,
> +          bool from_left)
> +{
> +     union {
> +             long val;
> +             char x[sizeof(long)];
> +     } u;
> +     errno = 0;
> +     u.val = ptrace(PTRACE_PEEKDATA, pid, addr, 0);
> +     switch (errno) {
> +             case 0:
"case" should be indented at the same level as corresponding "switch".

> +                     break;
> +             case ESRCH: case EINVAL:
> +                     /* these could be seen if the process is gone */
> +                     return false;
> +             case EFAULT: case EIO: case EPERM:
> +                     /* address space is inaccessible */
> +                     return false;
> +             default:
> +                     /* all the rest is strange and should be reported */
> +                     perror_msg("partial_poke: PTRACE_PEEKDATA pid:%d @0x%" 
> PRI_klx,
> +                                 pid, addr);
> +                     return false;
> +     }
I think, at this point we definitely need some wrappers for the
PEEKDATA/POKEDATA calls which contain all this error-handling code.

> +
> +     memcpy(from_left ? u.x : u.x + sizeof(long) - len, laddr, len);
So, if I want to copy 1 byte to the address 0xabcdef02 on x86_64 (and call
partial_poke with 0xabcdef00 and len 1, according to the code in upken), this
code will update the last byte?

> +
> +     /* now write it back */
> +     if (ptrace(PTRACE_POKEDATA, pid, addr, u.val) < 0) {
> +             switch (errno) {
> +                     case ESRCH: case EINVAL:
"case" should be indented at the same level as corresponding "switch".

> +                             /* these could be seen if the process is gone */
> +                             return -1;
> +                     case EFAULT: case EIO: case EPERM:
> +                             /* address space is inaccessible, or write is 
> prohibited */
Overly long line.

> +                             return -1;
> +                     default:
> +                             /* all the rest is strange and should be 
> reported */
> +                             perror_msg("partial_poke: PTRACE_POKEDATA 
> pid:%d @0x%" PRI_klx,
Overly long lines.

> +                                         pid, addr);
> +                             return -1;
> +             }
> +     }
> +
> +     return true;
> +}
> +
> +int
> +upoken(struct tcb *tcp, kernel_ulong_t addr, unsigned int len,
> +       const void *const our_addr)
> +{
> +     if (!len)
> +             return 0;
> +
> +     const char *laddr = our_addr;
> +     int pid = tcp->pid;
> +
> +#if ANY_WORDSIZE_LESS_THAN_KERNEL_LONG
> +     if (current_wordsize < sizeof(addr)
> +         && (addr & (~(kernel_ulong_t) -1U))) {
> +             return -1;
> +     }
> +#endif
> +
> +     if (!process_vm_readv_not_supported) {
> +             int r = vm_write_mem(pid, laddr, addr, len);
> +             if ((unsigned int) r == len)
> +                     return 0;
> +             if (r >= 0) {
> +                     error_msg("upoken: short write (%u < %u) @0x%" PRI_klx,
> +                               (unsigned int) r, len, addr);
> +                     return -1;
> +             }
> +             switch (errno) {
> +                     case ENOSYS:
"case" should be indented at the same level as corresponding "switch".

> +                             process_vm_readv_not_supported = 1;
> +                             break;
> +                     case EPERM:
> +                             /* operation not permitted, try PTRACE_PEEKDATA 
> */
PTRACE_POKEDATA? Also, overly long line.

> +                             break;
> +                     case ESRCH:
> +                             /* the process is gone */
> +                             return -1;
> +                     case EFAULT: case EIO:
> +                             /* address space is inaccessible */
> +                             return -1;
> +                     default:
> +                             /* all the rest is strange and should be 
> reported */
Overly long line.

> +                             perror_msg("process_vm_writev");
> +                             return -1;
> +             }
> +     }
> +
> +     if (addr & (sizeof(long) - 1)) {
> +             /* addr not a multiple of sizeof(long) */
> +             unsigned n = addr & (sizeof(long) - 1); /* residue */
> +             addr &= -sizeof(long);                  /* aligned address */
> +             unsigned m = MIN(sizeof(long) - n, len);
> +             if (!partial_poke(pid, addr, m, laddr, false))
> +                     return -1;
> +             addr += sizeof(long);
> +             laddr += m;
> +             len -= m;
> +     }
> +
> +     while (len >= sizeof(long)) {
> +             if (ptrace(PTRACE_POKEDATA, pid, addr, * (const long *) laddr) 
> < 0) {
Overly long line.

> +                     switch (errno) {
> +                     case ESRCH: case EINVAL:
> +                             /* these could be seen if the process is gone */
> +                             return -1;
> +                     case EFAULT: case EIO: case EPERM:
> +                             /* address space is inaccessible, or write is 
> prohibited */
Overly long line.

> +                             return -1;
> +                     default:
> +                             /* all the rest is strange and should be 
> reported */
> +                             perror_msg("upoken: PTRACE_POKEDATA pid:%d 
> @0x%" PRI_klx,
Overly long lines.

> +                                         pid, addr);
> +                             return -1;
> +                     }
> +             }
> +             addr += sizeof(long);
> +             laddr += sizeof(long);
> +             len -= sizeof(long);
> +     }
> +
> +     if (len) {
> +             if (!partial_poke(pid, addr, len, laddr, true))
> +                     return -1;
> +     }
if len && !partial_poke...

> +
> +     return 0;
> +}
> +
>  /*
>   * Iteratively fetch and print up to nmemb elements of elem_size size
>   * from the array that starts at tracee's address start_addr.
> -- 
> 2.11.0

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel

Reply via email to