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

Fix segfaults inside g_free on windows #14

Merged
merged 2 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions qemu-plugin/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
27 changes: 14 additions & 13 deletions qemu-plugin/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
///
Expand All @@ -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
Expand Down
18 changes: 7 additions & 11 deletions qemu-plugin/src/win_link_hook/mod.rs
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
///
Expand Down Expand Up @@ -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(),
);
}
}

Expand Down