emoon / rust_minifb

Cross platfrom window and framebuffer crate for Rust
MIT License
1.04k stars 99 forks source link

Incorrect construction of a `Window`'s Win32 `RawWindowHandle` causes segmentation fault/pointer deref panic #365

Closed jardhu closed 1 month ago

jardhu commented 2 months ago

When requesting a RawWindowHandle for a win32 Window (such as passing a Window to functions written with the raw_window_handle API), the instantiation of the RawWindowHandle performs some incorrect logic which results in either a panic after dereferencing a byte-misaligned pointer, or a segmentation fault.

thread 'main' panicked at C:\Users\jard\.cargo\registry\src\index.crates.io-6f17d22bba15001f\minifb-0.27.0\src\os\windows\mod.rs:518:53:
misaligned pointer dereference: address must be a multiple of 0x8 but is 0x2d61dca
stack backtrace:
   0: rust_begin_unwind
             at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6/library\std\src/panicking.rs:645:5
   1: core::panicking::panic_nounwind_fmt::runtime
   ...
error: process didn't exit successfully: `target\debug\project.exe` (exit code: 0xc0000005, STATUS_ACCESS_VIOLATION)

Process finished with exit code -1073741819 (0xC0000005)

There were three bugs which I've encountered and fixed with a workaround:

  1. The HasWindowHandle implementation for win32 assumes that the underlying window handle is a pointer to a HWND and dereferences it, when the handle is itself the HWND and simply needs to be casted to an isize. Hence, retrieving the HWND should look something like:
    let hwnd = NonZeroIsize::new(self.window.unwrap() as isize).expect("invalid hwnd");
  2. Similarly, the code assumes the HINSTANCE handle is a pointer to an HINSTANCE and dereferences it. The HINSTANCE handle from GetWindowLongPtr needs to be casted to an isize.
  3. On my machine (Windows 11, minifb 0.27.0), GetWindowLongPtrW(hwnd, GWLP_HINSTANCE) returns a null pointer for a valid HWND. I'm not exactly sure why this happens, but I instead opted to retrieve the HMODULE and pass that as the HINSTANCE:
    let hinstance = NonZeroIsize::new(unsafe { GetModuleHandleA(ptr::null()) } as isize);

I can confirm that the resulting RawWindowHandle constructed from the HWND and HMODULE works for the purposes of, in my particular use case, constructing a valid Vulkan surface with the window and display handles.

emoon commented 2 months ago

Hey, Thanks for the report. Seems it was broken in https://github.com/emoon/rust_minifb/commit/4b4af731ccd3738492aa4f785733f9af207e0623 cc @StefanoIncardone

The old code was using unsafe { libloaderapi::GetModuleHandleA(ptr::null()) } as *mut raw::c_void;