From dd955f83df58a507b55745b92a7f9d921d9b8f40 Mon Sep 17 00:00:00 2001
From: daladim <daladim@users.noreply.github.com>
Date: Wed, 3 Aug 2022 10:01:38 +0200
Subject: [PATCH] Fix: no longer using a pointer to a movable stack item

We're feeding OpenTraceA with an EVENT_TRACE_LOGFILE, which contains (at least) two pointers:
* one to the callback, run on every ETW event
  This one points to a static location in code
* one to a `PVOID Context` that Windows will give back as argument of the callback
  This points at `UserTrace.data`, which previously lived on the stack.
  This means that moving the UserTrace (e.g. by returning it in a function result) would break the pointer.
  This is fixed by putting UserTrace.data on the heap.
  It is now the responsability of the ferrisetw developer (and not its user, who was unaware of it) to *not* move it.

This changes the public API, so this means this should be released as a new major number of this crate
---
 src/native/etw_types.rs |  7 +++++--
 src/native/evntrace.rs  |  4 ++--
 src/trace.rs            | 12 ++++++++----
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/src/native/etw_types.rs b/src/native/etw_types.rs
index 5a0a50a..6be81ed 100644
--- a/src/native/etw_types.rs
+++ b/src/native/etw_types.rs
@@ -10,6 +10,7 @@ use crate::native::tdh_types::Property;
 use crate::provider::Provider;
 use crate::trace::{TraceData, TraceProperties, TraceTrait};
 use crate::utils;
+use std::ffi::c_void;
 use std::fmt::Formatter;
 use std::sync::RwLock;
 use windows::core::GUID;
@@ -196,7 +197,7 @@ impl EventTraceLogfile {
     /// # Safety
     ///
     /// Note that the returned structure contains pointers to the given `TraceData`, that should thus stay valid (and constant) during its lifetime
-    pub fn create(trace_data: &TraceData, callback: unsafe extern "system" fn(*mut EventRecord)) -> Self {
+    pub fn create(trace_data: &Box<TraceData>, callback: unsafe extern "system" fn(*mut EventRecord)) -> Self {
         let mut log_file = EventTraceLogfile::default();
 
         let not_really_mut_ptr = trace_data.name.as_ptr() as *mut _; // That's kind-of fine because the logger name is _not supposed_ to be changed by Windows APIs
@@ -205,7 +206,9 @@ impl EventTraceLogfile {
             u32::from(ProcessTraceMode::RealTime) | u32::from(ProcessTraceMode::EventRecord);
 
         log_file.0.Anonymous2.EventRecordCallback = Some(callback);
-        log_file.0.Context = unsafe { std::mem::transmute(trace_data as *const _) };
+
+        let not_really_mut_ptr = trace_data.as_ref() as *const TraceData as *const c_void as *mut c_void; // That's kind-of fine because the user context is _not supposed_ to be changed by Windows APIs
+        log_file.0.Context = not_really_mut_ptr;
 
         log_file
     }
diff --git a/src/native/evntrace.rs b/src/native/evntrace.rs
index 82d42bc..0cf4666 100644
--- a/src/native/evntrace.rs
+++ b/src/native/evntrace.rs
@@ -87,7 +87,7 @@ impl NativeEtw {
 
     pub(crate) fn open(
         &mut self,
-        trace_data: &TraceData,
+        trace_data: &Box<TraceData>,
     ) -> EvntraceNativeResult<EventTraceLogfile> {
         self.open_trace(trace_data)
     }
@@ -149,7 +149,7 @@ impl NativeEtw {
         Ok(())
     }
 
-    fn open_trace(&mut self, trace_data: &TraceData) -> EvntraceNativeResult<EventTraceLogfile> {
+    fn open_trace(&mut self, trace_data: &Box<TraceData>) -> EvntraceNativeResult<EventTraceLogfile> {
         let mut log_file = EventTraceLogfile::create(trace_data, trace_callback_thunk);
 
         unsafe {
diff --git a/src/trace.rs b/src/trace.rs
index 5f75c42..a84863b 100644
--- a/src/trace.rs
+++ b/src/trace.rs
@@ -251,14 +251,18 @@ macro_rules! impl_base_trace {
 /// User Trace struct
 #[derive(Debug)]
 pub struct UserTrace {
-    data: TraceData,
+    // This is `Box`ed so that it does not move around the stack in case the `UserTrace` is moved
+    // This is important, because we give a pointer to it to Windows, so that it passes it back to us on callbacks
+    data: Box<TraceData>,
     etw: evntrace::NativeEtw,
 }
 
 /// Kernel Trace struct
 #[derive(Debug)]
 pub struct KernelTrace {
-    data: TraceData,
+    // This is `Box`ed so that it does not move around the stack in case the `UserTrace` is moved
+    // This is important, because we give a pointer to it to Windows, so that it passes it back to us on callbacks
+    data: Box<TraceData>,
     etw: evntrace::NativeEtw,
 }
 
@@ -288,7 +292,7 @@ pub trait TraceTrait: TraceBaseTrait {
 impl UserTrace {
     /// Create a UserTrace builder
     pub fn new() -> Self {
-        let data = TraceData::new();
+        let data = Box::new(TraceData::new());
         UserTrace {
             data,
             etw: evntrace::NativeEtw::new(),
@@ -299,7 +303,7 @@ impl UserTrace {
 impl KernelTrace {
     /// Create a KernelTrace builder
     pub fn new() -> Self {
-        let data = TraceData::new();
+        let data = Box::new(TraceData::new());
 
         let mut kt = KernelTrace {
             data,