Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more ptrace_* wrappers #666

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,19 @@ This project adheres to [Semantic Versioning](http://semver.org/).
([#739](https://github.com/nix-rust/nix/pull/739))
- Expose `signalfd` module on Android as well.
([#739](https://github.com/nix-rust/nix/pull/739))
- Added nix::sys::ptrace::detach.
- Added nix::sys::ptrace::detach.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be removed, I assume it was a rebase error.

([#749](https://github.com/nix-rust/nix/pull/749))
- Added timestamp socket control message variant:
`nix::sys::socket::ControlMessage::ScmTimestamp`
([#663](https://github.com/nix-rust/nix/pull/663))
- Added socket option variant that enables the timestamp socket
control message: `nix::sys::socket::sockopt::ReceiveTimestamp`
([#663](https://github.com/nix-rust/nix/pull/663))

- Added specialized wrappers: `sys::ptrace::{peek, poke}{user, data}`
and macros: `syscall_arg`, `syscall_arg32` for register-to-argument
mappings. Using the matching routines
with `sys::ptrace::ptrace` is now deprecated.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's some weird indentation going on with this line.

([#666](https://github.com/nix-rust/nix/pull/666))
### Changed
- Renamed existing `ptrace` wrappers to encourage namespacing ([#692](https://github.com/nix-rust/nix/pull/692))
- Marked `sys::ptrace::ptrace` as `unsafe`.
Expand All @@ -55,7 +59,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
([#731](https://github.com/nix-rust/nix/pull/731))
- Marked `pty::ptsname` function as `unsafe`
([#744](https://github.com/nix-rust/nix/pull/744))
- Moved constants ptrace request, event and options to enums and updated ptrace functions and argument types accordingly.
- Moved constants ptrace request, event and options to enums and updated ptrace functions and argument types accordingly.
([#749](https://github.com/nix-rust/nix/pull/749))
- `AioCb::Drop` will now panic if the `AioCb` is still in-progress ([#715](https://github.com/nix-rust/nix/pull/715))

Expand Down
239 changes: 225 additions & 14 deletions src/sys/ptrace.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
//! For detailed description of the ptrace requests, consult `man ptrace`.
//! Interface for `ptrace`
//!
//! For detailed description of the ptrace requests, consult [`ptrace`(2)].
//! [`ptrace`(2)]: http://man7.org/linux/man-pages/man2/ptrace.2.html
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't render correctly on Rust 1.25.0.


use std::{mem, ptr};
use {Errno, Error, Result};
Expand Down Expand Up @@ -134,11 +137,20 @@ pub unsafe fn ptrace(request: Request, pid: Pid, addr: *mut c_void, data: *mut c
}
}

unsafe fn ptrace_peek(request: Request, pid: Pid, addr: *mut c_void, data: *mut c_void) -> Result<c_long> {
let ret = {
Errno::clear();
libc::ptrace(request as RequestType, libc::pid_t::from(pid), addr, data)
};
unsafe fn ptrace_peek(
request: Request,
pid: Pid,
addr: *mut c_void,
data: *mut c_void
) -> Result<c_long> {

Errno::clear();
let ret = libc::ptrace(
request as RequestType,
libc::pid_t::from(pid),
addr,
data
);
match Errno::result(ret) {
Ok(..) | Err(Error::Sys(Errno::UnknownErrno)) => Ok(ret),
err @ Err(..) => err,
Expand Down Expand Up @@ -168,8 +180,6 @@ unsafe fn ptrace_other(request: Request, pid: Pid, addr: *mut c_void, data: *mut

/// Set options, as with `ptrace(PTRACE_SETOPTIONS,...)`.
pub fn setoptions(pid: Pid, options: Options) -> Result<()> {
use std::ptr;

let res = unsafe {
libc::ptrace(Request::PTRACE_SETOPTIONS as RequestType,
libc::pid_t::from(pid),
Expand Down Expand Up @@ -238,12 +248,7 @@ pub fn syscall(pid: Pid) -> Result<()> {
/// Attaches to the process specified in pid, making it a tracee of the calling process.
pub fn attach(pid: Pid) -> Result<()> {
unsafe {
ptrace_other(
Request::PTRACE_ATTACH,
pid,
ptr::null_mut(),
ptr::null_mut(),
).map(|_| ()) // ignore the useless return value
ptrace_other(Request::PTRACE_ATTACH, pid, ptr::null_mut(), ptr::null_mut()).map(|_| ())
}
}

Expand Down Expand Up @@ -275,3 +280,209 @@ pub fn cont<T: Into<Option<Signal>>>(pid: Pid, sig: T) -> Result<()> {
}
}

/// Represents all possible ptrace-accessible registers on x86_64
#[cfg(target_arch = "x86_64")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is why your tests are failing, you need to either limit peekuser() to x86_64, since that's the only play this register is defined for, or expand Register to more platforms.

#[allow(non_camel_case_types)]
#[derive(Debug, PartialEq)]
pub enum Register {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs docs. Every variant should be documented as well.

Copy link
Contributor

@Susurrus Susurrus Sep 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a logical ordering to these registers? I would at least expect R10..R15 to be in order and they're not. I'm not certain how the docs for these are generally arranged, but maybe we can do a better grouping here?

Additionally is there no reference documentation we can link to here to give users more background on these registers? Some canonical reference? The kernel has some docs that touch on this here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's the order from user_regs_struct. Please, don't make nix an operating systems kindergarten. As said in another comment, there's a lot of other things that you need to know, to be able to use ptrace.

R15 = 8 * ::libc::R15 as isize,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to have all these registers documented, which I think I requested in my earlier review. Could you do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, R15 is just an... R15 register. If someone's using ptrace, it they have to know what the register does anyway.

I don't see any added value in documenting it as

R15 = 8 * ::libc::R15 as isize, /// the R15 register

People using ptrace are not kids.

R14 = 8 * ::libc::R14 as isize,
R13 = 8 * ::libc::R13 as isize,
R12 = 8 * ::libc::R12 as isize,
RBP = 8 * ::libc::RBP as isize,
RBX = 8 * ::libc::RBX as isize,
R11 = 8 * ::libc::R11 as isize,
R10 = 8 * ::libc::R10 as isize,
R9 = 8 * ::libc::R9 as isize,
R8 = 8 * ::libc::R8 as isize,
RAX = 8 * ::libc::RAX as isize,
RCX = 8 * ::libc::RCX as isize,
RDX = 8 * ::libc::RDX as isize,
RSI = 8 * ::libc::RSI as isize,
RDI = 8 * ::libc::RDI as isize,

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

ORIG_RAX = 8 * ::libc::ORIG_RAX as isize,
RIP = 8 * ::libc::RIP as isize,
CS = 8 * ::libc::CS as isize,
EFLAGS = 8 * ::libc::EFLAGS as isize,
RSP = 8 * ::libc::RSP as isize,
SS = 8 * ::libc::SS as isize,
FS_BASE = 8 * ::libc::FS_BASE as isize,
GS_BASE = 8 * ::libc::GS_BASE as isize,
DS = 8 * ::libc::DS as isize,
ES = 8 * ::libc::ES as isize,
FS = 8 * ::libc::FS as isize,
GS = 8 * ::libc::GS as isize,
}

/// Represents all possible ptrace-accessible registers on x86
#[cfg(target_arch = "x86")]
#[allow(non_camel_case_types)]
#[derive(Debug, PartialEq)]
pub enum Register {
EBX = 4 * ::libc::EBX as isize,
ECX = 4 * ::libc::ECX as isize,
EDX = 4 * ::libc::EDX as isize,
ESI = 4 * ::libc::ESI as isize,
EDI = 4 * ::libc::EDI as isize,
EBP = 4 * ::libc::EBP as isize,
EAX = 4 * ::libc::EAX as isize,
DS = 4 * ::libc::DS as isize,
ES = 4 * ::libc::ES as isize,
FS = 4 * ::libc::FS as isize,
GS = 4 * ::libc::GS as isize,
ORIG_EAX = 4 * ::libc::ORIG_EAX as isize,
EIP = 4 * ::libc::EIP as isize,
CS = 4 * ::libc::CS as isize,
EFL = 4 * ::libc::EFL as isize,
UESP = 4 * ::libc::UESP as isize,
SS = 4 * ::libc::SS as isize,
}

/// Returns the register containing nth register argument.
///
/// 0th argument is considered to be the syscall number.
/// Please note that these mappings are only valid for 64-bit programs.
/// Use [`syscall_arg32`] for tracing 32-bit programs instead.
///
/// [`syscall_arg32`]: macro.syscall_arg32.html
/// # Examples
///
/// ```
/// # #[macro_use] extern crate nix;
/// # fn main() {
/// assert_eq!(syscall_arg!(1), nix::sys::ptrace::Register::RDI);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please improve this example in line with my comments about syscall_arg32!.

/// # }
#[cfg(target_arch = "x86_64")]
#[macro_export]
macro_rules! syscall_arg {

This comment was marked as resolved.

This comment was marked as resolved.

(0) => ($crate::sys::ptrace::Register::ORIG_RAX);
(1) => ($crate::sys::ptrace::Register::RDI);
(2) => ($crate::sys::ptrace::Register::RSI);
(3) => ($crate::sys::ptrace::Register::RDX);
(4) => ($crate::sys::ptrace::Register::R10);
(5) => ($crate::sys::ptrace::Register::R8);
(6) => ($crate::sys::ptrace::Register::R9);
}

/// Returns the register containing nth register argument for 32-bit programs
///
/// 0th argument is considered to be the syscall number.
/// Please note that these mappings are only valid for 32-bit programs.
/// Use [`syscall_arg`] for tracing 64-bit programs instead.
///
/// [`syscall_arg`]: macro.syscall_arg.html
/// # Examples
///
/// ```
/// # #[macro_use] extern crate nix;
/// # fn main() {
/// assert_eq!(syscall_arg32!(1), nix::sys::ptrace::Register::RBX);
/// # }
#[cfg(target_arch = "x86_64")]
#[macro_export]
macro_rules! syscall_arg32 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these named differently if they're the same thing just for different platforms? I'm confusd by syscall_arg is not on user-API just with different implementations for x86 versus x86_64.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please read the comments before.
On a 64-bit host it all depends on the binary you're tracing. If you're tracing a 64-bit binary, you'll use syscall_arg. If you trace a 32-bit binary on a 64-bit host - syscall_arg32.

(0) => ($crate::sys::ptrace::Register::ORIG_RAX);
(1) => ($crate::sys::ptrace::Register::RBX);
(2) => ($crate::sys::ptrace::Register::RCX);
(3) => ($crate::sys::ptrace::Register::RDX);
(4) => ($crate::sys::ptrace::Register::RSI);
(5) => ($crate::sys::ptrace::Register::RDI);
(6) => ($crate::sys::ptrace::Register::RBP);
}

/// Returns the register containing nth register argument.
///
/// 0th argument is considered to be the syscall number.
///
/// # Examples
///
/// ```
/// # #[macro_use] extern crate nix;
/// # fn main() {
/// assert_eq!(syscall_arg!(1), nix::sys::ptrace::Register::RDI);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't a good example because a) it uses an assert (this should be a test if anything) and b) it doesn't demonstrate actual usage. No one is going to write code like this. How would you actually use syscall_arg! in practice? As an argument to pokeuser, right? This applies to all 3 of these macros.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's similar to what the official docs do: https://doc.rust-lang.org/std/fmt/fn.format.html
And it's not trivial to assert in doc tests without all the boilterplate.

The main thing here is to explain the syntax. The rest is obvious.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no issues with the boilerplate, I'm suggesting that examples be a demonstration to the user of how this is used. While yes, you show the syntax to use with this macro, we can do better by showing it in a broader context, where it's used as an argument to one of the ptrace wrapper functions. So I suggest you copy one of the lines where this is used from the test code (like as an argument to pokedata) and stick it here and make this code block no_run so it will at least be compile-checked.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a valid struct Pid, which has to be obtained from somewhere (a user will wonder what the heck is pid here). What about just mentioning that the purpose is to achieve arch-independent syscall argument lookups?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can leave the boilerplate commented-out here as well. Do what you need to do to get it to compile, but having something like ptrace::peekuser(child, syscall_arg!(0)) As the example would be better in my opinion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need valid values and can't get them, just mark this as only a compilation test by using ```no_run` to start the example. The point of an example isn't to test the code it's to show the user how they might use the code and provide a little additional context. The example I provided above, when annotated with a little description, would suit both of those goals.

/// # }
#[cfg(target_arch = "x86")]
#[macro_export]
macro_rules! syscall_arg {

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

(0) => ($crate::sys::ptrace::Register::ORIG_EAX);
(1) => ($crate::sys::ptrace::Register::EBX);
(2) => ($crate::sys::ptrace::Register::ECX);
(3) => ($crate::sys::ptrace::Register::EDX);
(4) => ($crate::sys::ptrace::Register::ESI);
(5) => ($crate::sys::ptrace::Register::EDI);
(6) => ($crate::sys::ptrace::Register::EBP);
}

/// An integer type, whose size equals a machine word
///
/// `ptrace` always returns a machine word. This type provides an abstraction
/// of the fact that on *nix systems, `c_long` is always a machine word,
/// so as to prevent the library from leaking C implementation-dependent types.
type Word = usize;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a doc comment.


/// Peeks a user-accessible register, as with `ptrace(PTRACE_PEEKUSER, ...)`
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub fn peekuser(pid: Pid, reg: Register) -> Result<Word> {
let reg_arg = (reg as i32) as *mut c_void;
unsafe {
ptrace_peek(Request::PTRACE_PEEKUSER, pid, reg_arg, ptr::null_mut()).map(|r| r as Word)
}
}

/// Sets the value of a user-accessible register, as with `ptrace(PTRACE_POKEUSER, ...)`
///
/// # Safety
/// When incorrectly used, may change the registers to bad values,
/// causing e.g. memory being corrupted by a syscall, thus is marked unsafe
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub unsafe fn pokeuser(pid: Pid, reg: Register, val: Word) -> Result<()> {
let reg_arg = (reg as u64) as *mut c_void;
ptrace_other(Request::PTRACE_POKEUSER, pid, reg_arg, val as *mut c_void).map(|_| ()) // ignore the useless return value
}

/// Peeks the memory of a process, as with `ptrace(PTRACE_PEEKDATA, ...)`
///
/// A memory chunk of a size of a machine word is returned.
/// # Safety
/// This function allows for accessing arbitrary data in the traced process
/// and may crash the inferior if used incorrectly and is thus marked `unsafe`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is "the inferior" here? That doesn't read right to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, gdb always talks about the inferior - the process that's spawned by gdb and traced.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the term used on the man pages? That's the reference docs we link to, so we should use that terminology. I believe they use "tracer" and "tracee".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marmistrz Any comment on this?

pub unsafe fn peekdata(pid: Pid, addr: usize) -> Result<Word> {
ptrace_peek(
Request::PTRACE_PEEKDATA,
pid,
addr as *mut c_void,
ptr::null_mut(),
).map(|r| r as Word)
}

/// Modifies the memory of a process, as with `ptrace(PTRACE_POKEUSER, ...)`
///
/// A memory chunk of a size of a machine word is overwriten in the requested
/// place in the memory of a process.
///
/// # Safety
/// This function allows for accessing arbitrary data in the traced process
/// and may crash the inferior or introduce race conditions if used
/// incorrectly and is thus marked `unsafe`.
pub unsafe fn pokedata(pid: Pid, addr: usize, val: Word) -> Result<()> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see some tests for these if possible, even if they just return errors, if for no other reason than to test the ergonomics of our wrappers. For example here I don't know how you'd generate addr since it's supposed to be a raw pointer type.

Copy link

@vincenthage vincenthage Aug 29, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One simple test that doesn't require any memory allocation is to cancel a syscall by changing its syscall number to 0 during the enter stage, and then reverting it during the exit stage.

ptrace_other(
Request::PTRACE_POKEDATA,
pid,
addr as *mut c_void,
val as *mut c_void,
).map(|_| ()) // ignore the useless return value
}

#[cfg(test)]
mod tests {
use super::Word;
use std::mem::size_of;
use libc::c_long;

#[test]
fn test_types() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please document why this test is necessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Word is an alias for usize, then these tests don't do anything, so let's just remove them. If they do something useful, please document this more clearly because I'm not certain what it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this should've been c_long. I'll fix that.

// c_long is implementation defined, so make sure
// its width matches
assert_eq!(size_of::<Word>(), size_of::<c_long>());
}
}
Loading