From 6ea460249c093604cb09f3ab8d1fab69dd2a8b2e Mon Sep 17 00:00:00 2001 From: Greg Manning Date: Tue, 28 May 2024 09:39:03 +0100 Subject: [PATCH] Fix segfaults inside g_free on windows calling `libc::free` actually doesn't always work, because there are multiple heaps, and we're trying to free it from the wrong one. Instead, we find the g_free export from libglib-2.0, and invoke that. Added the `libloading` crate to help with this, and also rewrote the `delaylink_hook` to use the functions there, rather than directly calling the windows API. --- qemu-plugin/Cargo.toml | 5 +++++ qemu-plugin/src/lib.rs | 26 ++++++++++++++------------ qemu-plugin/src/win_link_hook/mod.rs | 18 +++++++----------- 3 files changed, 26 insertions(+), 23 deletions(-) diff --git a/qemu-plugin/Cargo.toml b/qemu-plugin/Cargo.toml index f30c1c8..e41b1a5 100644 --- a/qemu-plugin/Cargo.toml +++ b/qemu-plugin/Cargo.toml @@ -17,6 +17,11 @@ once_cell = "1.19.0" qemu-plugin-sys = { version = "8.2.2-v0", workspace = true } thiserror = "1.0.51" +[target.'cfg(windows)'.dependencies.libloading] +version = "0.8.3" +[target.'cfg(windows)'.dependencies.lazy_static] +version = "1.4" + [target.'cfg(windows)'.dependencies.windows] version = "0.52" features = [ diff --git a/qemu-plugin/src/lib.rs b/qemu-plugin/src/lib.rs index 13c3834..1f1dde0 100644 --- a/qemu-plugin/src/lib.rs +++ b/qemu-plugin/src/lib.rs @@ -81,7 +81,6 @@ mod win_link_hook; use crate::error::{Error, Result}; #[cfg(windows)] -use libc::free; use qemu_plugin_sys::{ qemu_plugin_cb_flags, qemu_plugin_hwaddr, qemu_plugin_id_t, qemu_plugin_insn, qemu_plugin_mem_rw, qemu_plugin_meminfo_t, qemu_plugin_simple_cb_t, qemu_plugin_tb, @@ -106,6 +105,19 @@ extern "C" { fn g_free(mem: *mut c_void); } +#[cfg(windows)] +lazy_static::lazy_static! { + static ref G_FREE : libloading::os::windows::Symbol = { + let lib = + libloading::os::windows::Library::open_already_loaded("libglib-2.0-0.dll") + .expect("libglib-2.0-0.dll should already be loaded"); + // SAFETY + // "Users of `Library::get` should specify the correct type of the function loaded". + // We are specifying the correct type of g_free above (`void g_free(void*)`) + unsafe{lib.get(b"g_free").expect("find g_free")} + }; +} + #[cfg(windows)] /// Define g_free, because on Windows we cannot delay link it /// @@ -116,17 +128,7 @@ extern "C" { /// such values are documented to do so, and it is safe to call `g_free` on these values /// provided they are not used afterward. unsafe fn g_free(mem: *mut c_void) { - //TODO: We would really like to call g_free in the qemu binary here - //but we can't, because windows doesn't export symbols unless you explicitly export them - //and g_free isn't so exported. - - // NOTE: glib 2.46 g_malloc always uses system malloc implementation: - // https://docs.gtk.org/glib/func.mem_is_system_malloc.html - // So it is safe to call libc free to free a `g_malloc`-ed object - if !mem.is_null() { - // SAFETY: `mem` is non-null. - unsafe { free(mem) } - } + unsafe { G_FREE(mem) } } /// The index of a vCPU diff --git a/qemu-plugin/src/win_link_hook/mod.rs b/qemu-plugin/src/win_link_hook/mod.rs index e3db704..aaf68c6 100644 --- a/qemu-plugin/src/win_link_hook/mod.rs +++ b/qemu-plugin/src/win_link_hook/mod.rs @@ -1,8 +1,6 @@ //! Hook for linking against exported QEMU symbols at runtime -use windows::core::PCSTR; use windows::Win32::Foundation::HMODULE; -use windows::Win32::System::LibraryLoader::GetModuleHandleExA; use windows::Win32::System::WindowsProgramming::DELAYLOAD_INFO; /// The helper function for linker-supported delayed loading which is what actually @@ -12,8 +10,9 @@ type DelayHook = unsafe extern "C" fn(dli_notify: DliNotify, pdli: DELAYLOAD_INF #[no_mangle] /// Helper function invoked when failures occur in delay linking (as opposed to /// notifications) -pub static __pfnDliFailureHook2: DelayHook = delaylink_hook; +static __pfnDliFailureHook2: DelayHook = delaylink_hook; +#[allow(dead_code, clippy::enum_variant_names)] //We only need one of these variants. #[repr(C)] /// Delay load import hook notifications /// @@ -52,16 +51,13 @@ extern "C" fn delaylink_hook(dli_notify: DliNotify, pdli: DELAYLOAD_INFO) -> HMO // the target dll name is provided by Windows and is null-terminated. let name = unsafe { pdli.TargetDllName.to_string() }.unwrap_or_default(); - let mut module = HMODULE::default(); - // NOTE: QEMU executables on windows are named qemu-system.*.exe if name == "qemu.exe" { - // SAFETY: Getting the module handle for NULL is safe and does not dereference any - // pointers except to write the `module` argument which we know is alive here. - match unsafe { GetModuleHandleExA(0, PCSTR::null(), &mut module as *mut HMODULE) } { - Ok(_) => return module, - Err(e) => panic!("Failed to open QEMU module: {e:?}"), - } + return HMODULE( + libloading::os::windows::Library::this() + .expect("Get QEMU module") + .into_raw(), + ); } }