From facf00c2f11586630a1b6c30591290e7a36f6f54 Mon Sep 17 00:00:00 2001 From: Evian-Zhang Date: Fri, 14 Jun 2024 22:59:13 +0800 Subject: [PATCH 1/6] Fix inconsistent reg struct layout for 64bit tracer and 32bit tracee --- src/sys/ptrace/linux.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/sys/ptrace/linux.rs b/src/sys/ptrace/linux.rs index 8abaf4d71b..541e2ca7a4 100644 --- a/src/sys/ptrace/linux.rs +++ b/src/sys/ptrace/linux.rs @@ -310,6 +310,9 @@ fn ptrace_peek( /// `ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, ...)` is used instead to achieve the same effect /// on aarch64 and riscv64. /// +/// Currently, in x86_64 platform, if the tracer is 64bit and tracee is 32bit, this function +/// will return an error. +/// /// [ptrace(2)]: https://www.man7.org/linux/man-pages/man2/ptrace.2.html #[cfg(all( target_os = "linux", @@ -342,6 +345,9 @@ pub fn getregs(pid: Pid) -> Result { } /// Get a particular set of user registers, as with `ptrace(PTRACE_GETREGSET, ...)` +/// +/// Currently, in x86_64 platform, if the tracer is 64bit and tracee is 32bit, this function +/// will return an error. #[cfg(all( target_os = "linux", target_env = "gnu", @@ -367,6 +373,9 @@ pub fn getregset(pid: Pid) -> Result { (&mut iov as *mut libc::iovec).cast(), )?; }; + if iov.iov_len != mem::size_of::() { + return Err(Errno::EIO); + } Ok(unsafe { data.assume_init() }) } From db979507fbd7ea3fa06dc27d4faf071265819ef8 Mon Sep 17 00:00:00 2001 From: Evian-Zhang Date: Sat, 15 Jun 2024 10:45:58 +0800 Subject: [PATCH 2/6] Mark getregs as unsafe. Encourage to use getregset for safe version --- src/sys/ptrace/linux.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sys/ptrace/linux.rs b/src/sys/ptrace/linux.rs index 541e2ca7a4..9cc95f3ac8 100644 --- a/src/sys/ptrace/linux.rs +++ b/src/sys/ptrace/linux.rs @@ -304,14 +304,14 @@ fn ptrace_peek( } } -/// Get user registers, as with `ptrace(PTRACE_GETREGS, ...)` +/// Get user registers, as with `ptrace(PTRACE_GETREGS, ...)`. Call [`getregset`] for safe version /// /// Note that since `PTRACE_GETREGS` are not available on all platforms (as in [ptrace(2)]), /// `ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, ...)` is used instead to achieve the same effect /// on aarch64 and riscv64. /// -/// Currently, in x86_64 platform, if the tracer is 64bit and tracee is 32bit, this function -/// will return an error. +/// Currently, in x86_64 platform, if the tracer is 64bit and tracee is 32bit, the return value is +/// undefined. /// /// [ptrace(2)]: https://www.man7.org/linux/man-pages/man2/ptrace.2.html #[cfg(all( @@ -324,7 +324,7 @@ fn ptrace_peek( all(target_arch = "x86", target_env = "gnu") ) ))] -pub fn getregs(pid: Pid) -> Result { +pub unsafe fn getregs(pid: Pid) -> Result { ptrace_get_data::(Request::PTRACE_GETREGS, pid) } From 79c309334a7b7200f5938a0b1a32dd2d768aed83 Mon Sep 17 00:00:00 2001 From: Evian-Zhang Date: Sat, 15 Jun 2024 10:50:41 +0800 Subject: [PATCH 3/6] Fix test failed due to the unsafe change of getregs --- test/sys/test_ptrace.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/sys/test_ptrace.rs b/test/sys/test_ptrace.rs index c99c6762c3..3914921b69 100644 --- a/test/sys/test_ptrace.rs +++ b/test/sys/test_ptrace.rs @@ -224,12 +224,16 @@ fn test_ptrace_syscall() { .unwrap(); #[cfg(target_arch = "x86_64")] - let get_syscall_id = - || ptrace::getregs(child).unwrap().orig_rax as libc::c_long; + let get_syscall_id = || { + unsafe { ptrace::getregs(child) }.unwrap().orig_rax + as libc::c_long + }; #[cfg(target_arch = "x86")] - let get_syscall_id = - || ptrace::getregs(child).unwrap().orig_eax as libc::c_long; + let get_syscall_id = || { + unsafe { ptrace::getregs(child) }.unwrap().orig_eax + as libc::c_long + }; #[cfg(target_arch = "aarch64")] let get_syscall_id = From 581857b8ef8e3f179bd4f6944200ad87e28d50d1 Mon Sep 17 00:00:00 2001 From: Evian-Zhang Date: Sat, 15 Jun 2024 10:55:54 +0800 Subject: [PATCH 4/6] Fix Safety section missing for unsafe getregs --- src/sys/ptrace/linux.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sys/ptrace/linux.rs b/src/sys/ptrace/linux.rs index 9cc95f3ac8..d6c6f0573c 100644 --- a/src/sys/ptrace/linux.rs +++ b/src/sys/ptrace/linux.rs @@ -310,6 +310,8 @@ fn ptrace_peek( /// `ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, ...)` is used instead to achieve the same effect /// on aarch64 and riscv64. /// +/// # Safety +/// /// Currently, in x86_64 platform, if the tracer is 64bit and tracee is 32bit, the return value is /// undefined. /// From 8de81245acfb141bb8a3c5075205cdb88eb0d666 Mon Sep 17 00:00:00 2001 From: Evian-Zhang Date: Sat, 15 Jun 2024 11:00:14 +0800 Subject: [PATCH 5/6] Fix broken documentation link --- src/sys/ptrace/linux.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sys/ptrace/linux.rs b/src/sys/ptrace/linux.rs index d6c6f0573c..b9fa77a5cb 100644 --- a/src/sys/ptrace/linux.rs +++ b/src/sys/ptrace/linux.rs @@ -304,7 +304,7 @@ fn ptrace_peek( } } -/// Get user registers, as with `ptrace(PTRACE_GETREGS, ...)`. Call [`getregset`] for safe version +/// Get user registers, as with `ptrace(PTRACE_GETREGS, ...)`. Call `getregset` for safe version /// /// Note that since `PTRACE_GETREGS` are not available on all platforms (as in [ptrace(2)]), /// `ptrace(PTRACE_GETREGSET, pid, NT_PRSTATUS, ...)` is used instead to achieve the same effect From 45466fdf63d80ce352d9b3b6eebd37271b4189a7 Mon Sep 17 00:00:00 2001 From: Evian-Zhang Date: Sat, 15 Jun 2024 21:58:15 +0800 Subject: [PATCH 6/6] Mention EIO error when getregset have inconsistent arch between tracee and tracer; Add changelog since ptrace::getregs is now unsafe --- changelog/2448.changed.md | 1 + src/sys/ptrace/linux.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog/2448.changed.md diff --git a/changelog/2448.changed.md b/changelog/2448.changed.md new file mode 100644 index 0000000000..00774b665e --- /dev/null +++ b/changelog/2448.changed.md @@ -0,0 +1 @@ +Module sys/ptrace changes `ptrace::getregs` for x86_64/x86 to be unsafe function. \ No newline at end of file diff --git a/src/sys/ptrace/linux.rs b/src/sys/ptrace/linux.rs index b9fa77a5cb..38ec554ad5 100644 --- a/src/sys/ptrace/linux.rs +++ b/src/sys/ptrace/linux.rs @@ -349,7 +349,7 @@ pub fn getregs(pid: Pid) -> Result { /// Get a particular set of user registers, as with `ptrace(PTRACE_GETREGSET, ...)` /// /// Currently, in x86_64 platform, if the tracer is 64bit and tracee is 32bit, this function -/// will return an error. +/// will return EIO error. #[cfg(all( target_os = "linux", target_env = "gnu",