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

Bug: Possible misalignment of struct windows_sys::Win32::System::Diagnostics::Debug::CONTEXT #1412

Closed
BlackMagicCoding opened this issue Dec 17, 2022 · 5 comments
Labels
blocked bug Something isn't working

Comments

@BlackMagicCoding
Copy link

Which crate is this about?

windows-sys

Crate version

0.42.0

Summary

Hello,

I am currently trying to use the windows_sys crate to get the thread context of a suspended thread, but everytime I do it, I get an unexpected error when calling "GetThreadContext" - 998
According to documentation this means:
ERROR_NOACCESS

998 (0x3E6)
Invalid access to memory location.

Yes, this is a 64 bit process which has been tried to be accessed with the 64 bit version of GetThreadContext (you might correct me if I am wrong).
After some research I found this interesting StackOverflow article (even though it is about C++):
GetThreadContext fails after a successful SuspendThread in Windows 7

Edit: 16 byte aligning the address of the CONTEXT structure for GetThreadContext as suggested by Dan Bartlett seems to be doing the trick!

Before I used the windows_sys crate, I tried unsuccessfuly with winapi crate version 0.3.9.
At their definition of CONTEXT I can see something, which would also hint to the alignment problem:

STRUCT!{struct CONTEXT { // FIXME align 16
    P1Home: DWORD64,
    P2Home: DWORD64,
...

Now I looked at the definition of CONTEXT in windows_sys:

#[repr(C)]
pub struct CONTEXT {
    pub P1Home: u64,
    pub P2Home: u64,
...

As you can see the struct has C representation, but is missing the 16 Byte alignment as well.


From what I gathered, the necessary alignment depends on the OS version - I am running 64 bit Windows 10 (Build 19044.2364) right now.
Can someone of the developers confirm if this is reproducible for them as well?

Could you at least try if doing the alignment would solve it?
Yes, I assume that you most likely won't simply replace 1 line and be done with it, but rather make both versions available somehow.
The representation might look something like that:
#[repr(C, align(16))]

Is there any workaround/hack I can perform to use a correctly aligned CONTEXT struct with the windows_sys crate's call for GetThreadContext?

Toolchain version/configuration

Default host: x86_64-pc-windows-msvc
rustup home: C:\Users\thatsme.rustup

stable-x86_64-pc-windows-msvc (default)
rustc 1.65.0 (897e37553 2022-11-02)

Reproducible example

use std::ffi::CString;
use std::mem::zeroed;
use std::ptr::null_mut;
use windows_sys::Win32::Foundation::{GetLastError};
use windows_sys::Win32::System::Diagnostics::Debug::{CONTEXT, GetThreadContext};
use windows_sys::Win32::System::Threading::{CREATE_SUSPENDED, CreateProcessA, PROCESS_INFORMATION, ResumeThread, STARTUPINFOA};

pub const CONTEXT_AMD64: u32 = 0x00100000;
pub const CONTEXT_CONTROL: u32 = CONTEXT_AMD64 | 0x00000001;
pub const CONTEXT_INTEGER: u32 = CONTEXT_AMD64 | 0x00000002;
pub const CONTEXT_FLOATING_POINT: u32 = CONTEXT_AMD64 | 0x00000008;
pub const CONTEXT_FULL: u32 = CONTEXT_CONTROL | CONTEXT_INTEGER | CONTEXT_FLOATING_POINT;

fn main() {
    unsafe {
        let mut startup = zeroed::<STARTUPINFOA>();
        let mut process_info = zeroed::<PROCESS_INFORMATION>();
        let app = String::from("calc.exe"); // replace this with any 64 bit application you can execute
        let dest = CString::new(app).expect("CString::new failed");
        CreateProcessA(
            null_mut(),
            dest.as_ptr() as *mut _,
            null_mut(),
            null_mut(),
            0,
            CREATE_SUSPENDED,
            null_mut(),
            null_mut(),
            &mut startup,
            &mut process_info,
        );

        let mut context = zeroed::<CONTEXT>();
        context.ContextFlags = CONTEXT_FULL; // context constants shamelessly stolen from the winapi crate... no idea if this is really needed
        if GetThreadContext(process_info.hThread, &mut context) == 0 {
            eprintln!("could not get thread context: {}", GetLastError());
        } else {
            println!("successfully got thread context... yay!");
        }

        ResumeThread(process_info.hThread);
    }
}

Crate manifest

[dependencies]
windows-sys = {version = "0.42.0", features = ["Win32_System_Diagnostics_Debug", "Win32_System_Kernel"]}

Expected behavior

successfully got thread context... yay!

Actual behavior

could not get thread context: 998

Additional comments

This is my first time using unsafe Rust as well as doing Windows programming, so take everything I say with a giant pile of salt.

@BlackMagicCoding BlackMagicCoding added the bug Something isn't working label Dec 17, 2022
@BlackMagicCoding
Copy link
Author

BlackMagicCoding commented Dec 17, 2022

Small update
I tried to create my own CONTEXT struct with alignment like explained:

#[repr(C, align(16))]
pub struct CONTEXT {
    pub P1Home: u64,
...

Of course this leads to GetThreadContext no longer accepting my context variable, since the type does not match.
I tried create a second variable "context2" which is set via transmute of my new struct's variable and passed to the function instead:

let mut context2 = transmute::<CONTEXT, windows_sys::Win32::System::Diagnostics::Debug::CONTEXT>(context);
if GetThreadContext(process_info.hThread, &mut context2) == 0 {

I no longer get the 998 error, but it doesn't work out in the end anyway,... my best guess is, that internally the struct is still being populated the wrong way, since the function internals work with the wrong type.
Thought I let you know, even if it is not a solution :)

Edit: seems I just had a bug in my own code.
Also as pointed out in #1044 you can encapsulate the CONTEXT struct inside your own as a wrapper and set the alignment there, and then pass it that way.
Successfully tried it myself.

@riverar
Copy link
Collaborator

riverar commented Dec 17, 2022

Sorry you hit this issue. It's known and being worked on #1044.

@riverar
Copy link
Collaborator

riverar commented Dec 17, 2022

@kennykerr Shall we transfer this to metadata and close it as a duplicate?

@kennykerr
Copy link
Contributor

Yes, the alignment is wrong. It should be 16 on 64-bit and 4 on 32-bit.

@riverar riverar transferred this issue from microsoft/windows-rs Dec 18, 2022
@riverar
Copy link
Collaborator

riverar commented Dec 18, 2022

Closing as dupe of #1044, thanks @BlackMagicCoding for the ping / should get resolved soon as this is a hot issue.

@riverar riverar closed this as not planned Won't fix, can't repro, duplicate, stale Dec 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants