Merge pull request #14 from tocklime/main

Fix segfaults inside g_free on windows

.github/workflows/ci.yml

@@ -89,6 +89,6 @@ jobs:
           echo "Sleeping 180.0 seconds until booted (boot process took 118s first time)"
           Start-Sleep -Seconds 180.0
           echo "Stopping process"
-          Stop-Process -Id $process.id
+          Stop-Process -Id $process.id -ErrorAction SilentlyContinue
           cat out.txt
           cat err.txt

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 = [

qemu-plugin/src/lib.rs

@@ -80,8 +80,6 @@ mod unix_weak_link;
 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 +104,19 @@ extern "C" {
     fn g_free(mem: *mut c_void);
 }
 
+#[cfg(windows)]
+lazy_static::lazy_static! {
+    static ref G_FREE : libloading::os::windows::Symbol<unsafe extern fn(*mut c_void)> = {
+        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 +127,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

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(),
+            );
         }
     }