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

ptrace: implement getsyscallinfo #2006

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

mbyzhang
Copy link

Implement sys::ptrace::getsyscallinfo via ptrace(PTRACE_GET_SYSCALL_INFO, ...), which is available since Linux 5.3.

@emilengler
Copy link

I really hope this gets into upstream very soon. I will try to do a review on this tomorrow.

@@ -22,6 +22,9 @@ pub type AddressType = *mut ::libc::c_void;
))]
use libc::user_regs_struct;

#[cfg(all(target_os = "linux", target_env = "gnu"))]

Choose a reason for hiding this comment

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

I'm curios. Does this really need the target_env = "gnu"? I've just looked into the musl source code and found a definition for struct __ptrace_syscall_info inside include/sys/ptrace.h. Maybe it is even better, to remove the target_enventirely and replace it with a check for kernel 5.3.

Copy link
Author

Choose a reason for hiding this comment

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

In Rust libc crate, __ptrace_syscall_info is only available for gnu env. I will work on upstreaming __ptrace_syscall_info into Rust libc musl first.

@@ -121,6 +124,8 @@ libc_enum! {
#[cfg(all(target_os = "linux", target_env = "gnu",
any(target_arch = "x86", target_arch = "x86_64")))]
PTRACE_SYSEMU_SINGLESTEP,
#[cfg(all(target_os = "linux", target_env = "gnu"))]

Choose a reason for hiding this comment

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

Again, my criticism with the gnu.

@@ -152,6 +157,80 @@ libc_enum! {
}
}

#[cfg(all(target_os = "linux", target_env = "gnu"))]

Choose a reason for hiding this comment

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

^^

src/sys/ptrace/linux.rs Outdated Show resolved Hide resolved
src/sys/ptrace/linux.rs Outdated Show resolved Hide resolved
},
}

#[cfg(all(target_os = "linux", target_env = "gnu"))]

Choose a reason for hiding this comment

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

^^

@@ -292,6 +371,22 @@ pub fn getsiginfo(pid: Pid) -> Result<siginfo_t> {
ptrace_get_data::<siginfo_t>(Request::PTRACE_GETSIGINFO, pid)
}

/// Get ptrace syscall info as with `ptrace(PTRACE_GET_SYSCALL_INFO,...)`
/// Only available on Linux 5.3+
#[cfg(all(target_os = "linux", target_env = "gnu"))]

Choose a reason for hiding this comment

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

^^

@@ -273,3 +273,87 @@ fn test_ptrace_syscall() {
}
}
}

#[cfg(all(target_os = "linux", target_env = "gnu"))]

Choose a reason for hiding this comment

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

^^

test/sys/test_ptrace.rs Show resolved Hide resolved
test/sys/test_ptrace.rs Show resolved Hide resolved
@mbyzhang mbyzhang reopened this Apr 18, 2023
@mbyzhang mbyzhang marked this pull request as draft April 18, 2023 22:34
@mbyzhang
Copy link
Author

Working on upstreaming __ptrace_syscall_info and other syscallinfo-related definitions into Rust libc musl first.

@tamird
Copy link
Contributor

tamird commented Oct 12, 2023

@mbyzhang any progress on this?

@mbyzhang
Copy link
Author

mbyzhang commented Dec 4, 2023

Just submitted a pull request to reorgnize ptrace definitions in Rust libc crate. Waiting for the merge.

Copy link
Member

@SteveLauC SteveLauC left a comment

Choose a reason for hiding this comment

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

Thanks for your interest in contributing to Nix! And I am sorry about the late response, I have left some comments:)

@@ -16,6 +16,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).
- Added `mq_timedreceive` to `::nix::mqueue`.
([#1966])(https://github.com/nix-rust/nix/pull/1966)
- Added `LocalPeerPid` to `nix::sys::socket::sockopt` for macOS. ([#1967](https://github.com/nix-rust/nix/pull/1967))
- Added `getsyscallinfo` to `nix::sys::ptrace` for Linux.
([#2006](https://github.com/nix-rust/nix/pull/2006))
Copy link
Member

Choose a reason for hiding this comment

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

We have changed our changelog mode, please see CONTRIBUTING.md on how to add a changelog.

@@ -152,6 +157,80 @@ libc_enum! {
}
}

#[cfg(all(target_os = "linux", target_env = "gnu"))]
#[cfg_attr(docsrs, doc(cfg(all())))]
Copy link
Member

Choose a reason for hiding this comment

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

Please remove this attribute and the one in line 175 as they are useless now

#[cfg(all(target_os = "linux", target_env = "gnu"))]
#[cfg_attr(docsrs, doc(cfg(all())))]
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub struct SyscallInfo {
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible (and convenient) to reuse the ptrace_syscall_info type from libc, like this:

#[repr(transparent)]
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub struct SyscallInfo(ptrace_syscall_info);

If so, then we should switch to it

Copy link
Author

Choose a reason for hiding this comment

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

libc ptrace_syscall_info has a union, which is not safe to use. So the safe SyscallInfo here is probably preferred.

Request::PTRACE_GET_SYSCALL_INFO,
pid,
mem::size_of::<ptrace_syscall_info>() as *mut c_void,
data.as_mut_ptr() as *mut _ as *mut c_void,
Copy link
Member

Choose a reason for hiding this comment

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

For pointer type casting, we prefer .cast() over the as keyword

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.

None yet

4 participants