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::getregs & ptrace::getregset may lead to UB of uninitialized data read #2447

Open
Evian-Zhang opened this issue Jun 14, 2024 · 4 comments · May be fixed by #2448
Open

ptrace::getregs & ptrace::getregset may lead to UB of uninitialized data read #2447

Evian-Zhang opened this issue Jun 14, 2024 · 4 comments · May be fixed by #2448
Labels

Comments

@Evian-Zhang
Copy link

ptrace::getregs and ptrace::getregset are used to get register values of certain process (tracee), which utilizes the ptrace(PTRACE_GETREGSET, ..) syscall:

pub fn getregset<S: RegisterSet>(pid: Pid) -> Result<S::Regs> {
    let request = Request::PTRACE_GETREGSET;
    let mut data = mem::MaybeUninit::<S::Regs>::uninit();
    let mut iov = libc::iovec {
        iov_base: data.as_mut_ptr().cast(),
        iov_len: mem::size_of::<S::Regs>(),
    };
    unsafe {
        ptrace_other(
            request,
            pid,
            S::VALUE as i32 as AddressType,
            (&mut iov as *mut libc::iovec).cast(),
        )?;
    };
    Ok(unsafe { data.assume_init() })
}

In amd64 Linux, if the tracer is 64bit process, tracee is 32bit process, the S::Regs will be resolved to 64bit version of libc::user_regs_struct, while after the syscall return, the S::VALUE buffer is filled by kernel with 32bit version of such struct. (The iovec.iov_len field will be 68 after returning, which confirms such thing). Reading the returned S::Regs will lead to uninitialized data, which is UB.

@asomers
Copy link
Member

asomers commented Jun 14, 2024

It isn't UB because the data is half-uninitialized. After all the rust compiler doesn't know how much data is initialized by the OS, so it must assume that all is. But it certainly doesn't sound like the data in this case is a valid S::Regs. Would you be willing to file a PR to fix the bug? Without wanting to add full 32-bit support, it would be sufficient to simply return an error in that case.

@Evian-Zhang
Copy link
Author

@asomers Thank you for your reply :)

I have sent a PR (#2448) to fix this.

However, I do think this is a UB. It's true that Rust compiler doesn't know how much data is initialized by the OS, and as a result, it is considered UB to read the uninitialized part, and that is what the read-buf RFC proposed to solve.

@SteveLauC
Copy link
Member

SteveLauC commented Jun 15, 2024

Thanks for the report!

cc @hack3ric


I am curious if this problem exists on other targets? e.g., arm64.

@Evian-Zhang
Copy link
Author

I am curious if this problem exists on other targets? e.g., arm64.

I have no arm64 machine, maybe someone could help testing this.

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

Successfully merging a pull request may close this issue.

3 participants