Skip to content

Conversation

cbarrete
Copy link

What does this PR do

It fixes ptrace::syscall_info, which is currently broken (at least on Linux 6.12.31).

See the commit message for more details.

Reproduction

The following program panics when built against master and behaves correctly (i.e. it does not panic and prints valid syscall info) when built against this branch:

fn main() {
    let pid = unsafe { nix::unistd::fork().unwrap() };

    match pid {
        nix::unistd::ForkResult::Child => {
            nix::sys::ptrace::traceme().unwrap();
            nix::sys::signal::raise(nix::sys::signal::Signal::SIGCONT).unwrap();
            println!("I'm issuing a syscall!");
        }
        nix::unistd::ForkResult::Parent { child } => {
            nix::sys::wait::waitpid(Some(child), None).unwrap();
            nix::sys::ptrace::setoptions(child, nix::sys::ptrace::Options::PTRACE_O_TRACESYSGOOD)
                .unwrap();

            nix::sys::ptrace::syscall(child, None).unwrap();
            nix::sys::wait::waitpid(Some(child), None).unwrap();
            let syscall_info = nix::sys::ptrace::syscall_info(child).unwrap();
            println!("{syscall_info:?}");
            assert!(syscall_info.op == libc::PTRACE_SYSCALL_INFO_ENTRY);

            nix::sys::ptrace::syscall(child, None).unwrap();
            nix::sys::wait::waitpid(Some(child), None).unwrap();
            let syscall_info = nix::sys::ptrace::syscall_info(child).unwrap();
            println!("{syscall_info:?}");
            assert!(syscall_info.op == libc::PTRACE_SYSCALL_INFO_EXIT);
        }
    }
}

Checklist:

  • I have read CONTRIBUTING.md
  • I have written necessary tests and rustdoc comments
  • A change log has been added if this PR modifies nix's API

@cbarrete cbarrete force-pushed the fix-ptrace-syscall-info branch 2 times, most recently from 9076df2 to 499f154 Compare July 26, 2025 05:13
@cbarrete
Copy link
Author

cbarrete commented Aug 7, 2025

Hello @SteveLauC, this is my first contribution here, I'm not sure who to ask for a review. Would you mind having a look? Thanks!

@cbarrete
Copy link
Author

@asomers instead maybe? Feel free to redirect me to someone else!
Thanks in advance.

@cbarrete
Copy link
Author

cbarrete commented Sep 6, 2025

Sorry for bumping this again, but could you please take a look @asomers?
If this PR is not welcome, I can close this and maintain a fork, but this fix seems desirable to have upstream to me, as ptrace::syscall_info is currently completely broken.

Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

Approved. Also, since you wrote such a nice example program, you might consider adding it to Nix's examples directory.

@cbarrete cbarrete force-pushed the fix-ptrace-syscall-info branch 4 times, most recently from 40ecada to c5b44c7 Compare September 17, 2025 03:24
According to the man page:

> The addr argument contains the size of the buffer pointed to by the
> data argument (i.e., sizeof(struct ptrace_syscall_info)).

We were setting the data argument, so this syscall was returning
garbage. This is easily reproduced and verified by looking at the `op`
value, which should range from 0 to 3. The PR associated with this
commit contains a sample program to demonstrate this.

The fix is done in the `ptrace_get_data` helper to avoid duplicating its
implementation just for `syscall_info`.

Of all the other callers, all but one are documented as ignoring `addr`.
The other one (`PTRACE_GETREGS`) is documented as ignoring `addr`
_except on SPARC systems_, on which `addr` and `data` are reversed.
However, this is already not respected by `nix`, so this changes is not
disruptive in this regard.

There should be no performance concerns as we are replacing one constant
with another.
@cbarrete cbarrete force-pushed the fix-ptrace-syscall-info branch from c5b44c7 to 7f6a8de Compare September 17, 2025 03:27
@cbarrete
Copy link
Author

I've rebased the branch and added the example. Sorry for the mess, it took me a while to figure out the right cfg annotations.

If everything looks fine to you, could you merge this after approving it again? I couldn't merge the branch even after you had approved it.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants