crosvm: Fix drift

BUG=b:213146388
TEST=presubmit

Change-Id: I59e6b7ad7aff8d4659c62e310a7955146a10d743
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3777405
Tested-by: Vikram Auradkar <auradkar@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
Auto-Submit: Vikram Auradkar <auradkar@google.com>
Commit-Queue: Vikram Auradkar <auradkar@google.com>

metrics/protos/event_details.proto

@@ -2,13 +2,14 @@
 syntax = "proto2";
 
 message RecordDetails {
-  reserved 1 to 11, 14 to 16;
+  reserved 1 to 11, 14 to 18;
   // Additional details about an unexpected exit of a child process within
   // the emulator.
   optional EmulatorChildProcessExitDetails emulator_child_process_exit_details =
       12;
   // Additional details about wave formats from the Window's host system.
   optional WaveFormatDetails wave_format_details = 13;
+  optional EmulatorDllDetails emulator_dll_details = 19;
 }
 
 message WaveFormatDetails {
@@ -96,3 +97,7 @@ message EmulatorChildProcessExitDetails {
   // emulator code.
   optional EmulatorProcessType process_type = 2;
 }
+
+message EmulatorDllDetails {
+  optional string dll_base_name = 1;
+}

net_util/build.rs

@@ -26,7 +26,6 @@ mod win_slirp {
             env::var("PATH").unwrap(),
             manifest_dir,
             build_type,
-            manifest_dir
         );
     }
 }

win_audio/src/win_audio_impl/mod.rs

@@ -4,7 +4,7 @@
 
 use crate::AudioSharedFormat;
 use audio_streams::{
-    BoxError, BufferDrop, NoopStream, NoopStreamControl, PlaybackBuffer, PlaybackBufferError,
+    BoxError, BufferCommit, NoopStream, NoopStreamControl, PlaybackBuffer, PlaybackBufferError,
     PlaybackBufferStream, SampleFormat, StreamControl, StreamSource,
 };
 use base::{error, info, warn, AsRawDescriptor, Error, Event, EventExt, EventReadResult};
@@ -177,7 +177,7 @@ impl WinAudioRenderer {
 
 impl PlaybackBufferStream for WinAudioRenderer {
     /// Returns a wrapper around the WASAPI buffer.
-    fn next_playback_buffer(&mut self) -> Result<PlaybackBuffer, BoxError> {
+    fn next_playback_buffer<'b, 's: 'b>(&'s mut self) -> Result<PlaybackBuffer<'b>, BoxError> {
         const MAX_REATTACH_TRIES: usize = 50;
         for _ in 0..MAX_REATTACH_TRIES {
             match self.device.next_win_buffer() {
@@ -738,9 +738,9 @@ impl DeviceRenderer {
     }
 }
 
-impl BufferDrop for DeviceRenderer {
+impl BufferCommit for DeviceRenderer {
     // Called after buffer from WASAPI is filled. This will allow the audio bytes to be played as sound.
-    fn trigger(&mut self, nframes: usize) {
+    fn commit(&mut self, nframes: usize) {
         // Safe because `audio_render_client` is initialized and parameters passed
         // into `ReleaseBuffer()` are valid
         unsafe {

win_util/src/dll_notification.rs

@@ -0,0 +1,429 @@
+// Copyright 2022 The ChromiumOS Authors.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+use std::ffi::{c_void, OsString};
+use std::{io, ptr};
+
+use winapi::shared::minwindef::ULONG;
+use winapi::um::winnt::PVOID;
+
+use super::unicode_string_to_os_string;
+
+// Required for Windows API FFI bindings, as the names of the FFI structs and
+// functions get called out by the linter.
+#[allow(non_upper_case_globals)]
+#[allow(non_camel_case_types)]
+#[allow(non_snake_case)]
+#[allow(dead_code)]
+mod dll_notification_sys {
+    use std::io;
+
+    use winapi::shared::minwindef::ULONG;
+    use winapi::shared::ntdef::{NTSTATUS, PCUNICODE_STRING};
+    use winapi::shared::ntstatus::STATUS_SUCCESS;
+    use winapi::um::libloaderapi::{GetModuleHandleA, GetProcAddress};
+    use winapi::um::winnt::{CHAR, PVOID};
+
+    #[repr(C)]
+    pub union _LDR_DLL_NOTIFICATION_DATA {
+        pub Loaded: LDR_DLL_LOADED_NOTIFICATION_DATA,
+        pub Unloaded: LDR_DLL_UNLOADED_NOTIFICATION_DATA,
+    }
+    pub type LDR_DLL_NOTIFICATION_DATA = _LDR_DLL_NOTIFICATION_DATA;
+    pub type PLDR_DLL_NOTIFICATION_DATA = *mut LDR_DLL_NOTIFICATION_DATA;
+
+    #[repr(C)]
+    #[derive(Debug, Copy, Clone)]
+    pub struct _LDR_DLL_LOADED_NOTIFICATION_DATA {
+        pub Flags: ULONG,                  // Reserved.
+        pub FullDllName: PCUNICODE_STRING, // The full path name of the DLL module.
+        pub BaseDllName: PCUNICODE_STRING, // The base file name of the DLL module.
+        pub DllBase: PVOID,                // A pointer to the base address for the DLL in memory.
+        pub SizeOfImage: ULONG,            // The size of the DLL image, in bytes.
+    }
+    pub type LDR_DLL_LOADED_NOTIFICATION_DATA = _LDR_DLL_LOADED_NOTIFICATION_DATA;
+    pub type PLDR_DLL_LOADED_NOTIFICATION_DATA = *mut LDR_DLL_LOADED_NOTIFICATION_DATA;
+
+    #[repr(C)]
+    #[derive(Debug, Copy, Clone)]
+    pub struct _LDR_DLL_UNLOADED_NOTIFICATION_DATA {
+        pub Flags: ULONG,                  // Reserved.
+        pub FullDllName: PCUNICODE_STRING, // The full path name of the DLL module.
+        pub BaseDllName: PCUNICODE_STRING, // The base file name of the DLL module.
+        pub DllBase: PVOID,                // A pointer to the base address for the DLL in memory.
+        pub SizeOfImage: ULONG,            // The size of the DLL image, in bytes.
+    }
+    pub type LDR_DLL_UNLOADED_NOTIFICATION_DATA = _LDR_DLL_UNLOADED_NOTIFICATION_DATA;
+    pub type PLDR_DLL_UNLOADED_NOTIFICATION_DATA = *mut LDR_DLL_UNLOADED_NOTIFICATION_DATA;
+
+    pub const LDR_DLL_NOTIFICATION_REASON_LOADED: ULONG = 1;
+    pub const LDR_DLL_NOTIFICATION_REASON_UNLOADED: ULONG = 2;
+
+    const NTDLL: &'static [u8] = b"ntdll\0";
+    const LDR_REGISTER_DLL_NOTIFICATION: &'static [u8] = b"LdrRegisterDllNotification\0";
+    const LDR_UNREGISTER_DLL_NOTIFICATION: &'static [u8] = b"LdrUnregisterDllNotification\0";
+
+    pub type LdrDllNotification = unsafe extern "C" fn(
+        NotificationReason: ULONG,
+        NotificationData: PLDR_DLL_NOTIFICATION_DATA,
+        Context: PVOID,
+    );
+
+    pub type FnLdrRegisterDllNotification =
+        unsafe extern "C" fn(ULONG, LdrDllNotification, PVOID, *mut PVOID) -> NTSTATUS;
+    pub type FnLdrUnregisterDllNotification = unsafe extern "C" fn(PVOID) -> NTSTATUS;
+
+    extern "C" {
+        pub fn RtlNtStatusToDosError(Status: NTSTATUS) -> ULONG;
+    }
+
+    /// Wrapper for the NTDLL `LdrRegisterDllNotification` function. Dynamically
+    /// gets the address of the function and invokes the function with the given
+    /// arguments.
+    ///
+    /// # Safety
+    /// Unsafe as this function does not verify its arguments; the caller is
+    /// expected to verify the safety as if invoking the underlying C function.
+    pub unsafe fn LdrRegisterDllNotification(
+        Flags: ULONG,
+        NotificationFunction: LdrDllNotification,
+        Context: PVOID,
+        Cookie: *mut PVOID,
+    ) -> io::Result<()> {
+        let proc_addr = GetProcAddress(
+            /* hModule= */
+            GetModuleHandleA(
+                /* lpModuleName= */ NTDLL.as_ptr() as *const u8 as *const CHAR,
+            ),
+            /* lpProcName= */
+            LDR_REGISTER_DLL_NOTIFICATION.as_ptr() as *const u8 as *const CHAR,
+        );
+        if proc_addr.is_null() {
+            return Err(std::io::Error::last_os_error());
+        }
+        let ldr_register_dll_notification: FnLdrRegisterDllNotification =
+            std::mem::transmute(proc_addr);
+        let ret = ldr_register_dll_notification(Flags, NotificationFunction, Context, Cookie);
+        if ret != STATUS_SUCCESS {
+            return Err(io::Error::from_raw_os_error(
+                RtlNtStatusToDosError(/* Status= */ ret) as i32,
+            ));
+        };
+        Ok(())
+    }
+
+    /// Wrapper for the NTDLL `LdrUnregisterDllNotification` function. Dynamically
+    /// gets the address of the function and invokes the function with the given
+    /// arguments.
+    ///
+    /// # Safety
+    /// Unsafe as this function does not verify its arguments; the caller is
+    /// expected to verify the safety as if invoking the underlying C function.
+    pub unsafe fn LdrUnregisterDllNotification(Cookie: PVOID) -> io::Result<()> {
+        let proc_addr = GetProcAddress(
+            /* hModule= */
+            GetModuleHandleA(
+                /* lpModuleName= */ NTDLL.as_ptr() as *const u8 as *const CHAR,
+            ),
+            /* lpProcName= */
+            LDR_UNREGISTER_DLL_NOTIFICATION.as_ptr() as *const u8 as *const CHAR,
+        );
+        if proc_addr.is_null() {
+            return Err(std::io::Error::last_os_error());
+        }
+        let ldr_unregister_dll_notification: FnLdrUnregisterDllNotification =
+            std::mem::transmute(proc_addr);
+        let ret = ldr_unregister_dll_notification(Cookie);
+        if ret != STATUS_SUCCESS {
+            return Err(io::Error::from_raw_os_error(
+                RtlNtStatusToDosError(/* Status= */ ret) as i32,
+            ));
+        };
+        Ok(())
+    }
+}
+
+use dll_notification_sys::*;
+
+#[derive(Debug)]
+pub struct DllNotificationData {
+    pub full_dll_name: OsString,
+    pub base_dll_name: OsString,
+}
+
+/// Callback context wrapper for DLL load notification functions.
+///
+/// This struct provides a wrapper for invoking a function-like type any time a
+/// DLL is loaded in the current process. This is done in a type-safe way,
+/// provided that users of this struct observe some safety invariants.
+///
+/// # Safety
+/// The struct instance must not be used once it has been registered as a
+/// notification target. The callback function assumes that it has a mutable
+/// reference to the struct instance. Only once the callback is unregistered is
+/// it safe to re-use the struct instance.
+struct CallbackContext<F1, F2>
+where
+    F1: FnMut(DllNotificationData),
+    F2: FnMut(DllNotificationData),
+{
+    loaded_callback: F1,
+    unloaded_callback: F2,
+}
+
+impl<F1, F2> CallbackContext<F1, F2>
+where
+    F1: FnMut(DllNotificationData),
+    F2: FnMut(DllNotificationData),
+{
+    /// Create a new `CallbackContext` with the two callback functions. Takes
+    /// two callbacks, a `loaded_callback` which is called when a DLL is
+    /// loaded, and `unloaded_callback` which is called when a DLL is unloaded.
+    pub fn new(loaded_callback: F1, unloaded_callback: F2) -> Self {
+        CallbackContext {
+            loaded_callback,
+            unloaded_callback,
+        }
+    }
+
+    /// Provides a notification function that can be passed to the
+    /// `LdrRegisterDllNotification` function.
+    pub fn get_notification_function(&self) -> LdrDllNotification {
+        Self::notification_function
+    }
+
+    /// A notification function with C linkage. This function assumes that it
+    /// has exclusive access to the instance of the struct passed through the
+    /// `context` parameter.
+    extern "C" fn notification_function(
+        notification_reason: ULONG,
+        notification_data: PLDR_DLL_NOTIFICATION_DATA,
+        context: PVOID,
+    ) {
+        // Safe because the DLLWatcher guarantees that the CallbackContext
+        // instance is not null and that we have exclusive access to it.
+        let callback_context =
+            unsafe { (context as *mut Self).as_mut() }.expect("context was null");
+
+        assert!(!notification_data.is_null());
+
+        match notification_reason {
+            LDR_DLL_NOTIFICATION_REASON_LOADED => {
+                // Safe because we know that the LDR_DLL_NOTIFICATION_DATA union
+                // contains the LDR_DLL_LOADED_NOTIFICATION_DATA because we got
+                // LDR_DLL_NOTIFICATION_REASON_LOADED as the notification
+                // reason.
+                let loaded = unsafe { &mut (*notification_data).Loaded };
+
+                assert!(!loaded.BaseDllName.is_null());
+
+                // Safe because we assert that the pointer is not null and
+                // expect that the OS has provided a valid UNICODE_STRING
+                // struct.
+                let base_dll_name = unsafe { unicode_string_to_os_string(&*loaded.BaseDllName) };
+
+                assert!(!loaded.FullDllName.is_null());
+
+                // Safe because we assert that the pointer is not null and
+                // expect that the OS has provided a valid UNICODE_STRING
+                // struct.
+                let full_dll_name = unsafe { unicode_string_to_os_string(&*loaded.FullDllName) };
+
+                (callback_context.loaded_callback)(DllNotificationData {
+                    base_dll_name,
+                    full_dll_name,
+                });
+            }
+            LDR_DLL_NOTIFICATION_REASON_UNLOADED => {
+                // Safe because we know that the LDR_DLL_NOTIFICATION_DATA union
+                // contains the LDR_DLL_UNLOADED_NOTIFICATION_DATA because we got
+                // LDR_DLL_NOTIFICATION_REASON_UNLOADED as the notification
+                // reason.
+                let unloaded = unsafe { &mut (*notification_data).Unloaded };
+
+                assert!(!unloaded.BaseDllName.is_null());
+
+                // Safe because we assert that the pointer is not null and
+                // expect that the OS has provided a valid UNICODE_STRING
+                // struct.
+                let base_dll_name = unsafe { unicode_string_to_os_string(&*unloaded.BaseDllName) };
+
+                assert!(!unloaded.FullDllName.is_null());
+
+                // Safe because we assert that the pointer is not null and
+                // expect that the OS has provided a valid UNICODE_STRING
+                // struct.
+                let full_dll_name = unsafe { unicode_string_to_os_string(&*unloaded.FullDllName) };
+
+                (callback_context.unloaded_callback)(DllNotificationData {
+                    base_dll_name,
+                    full_dll_name,
+                })
+            }
+            n => panic!("invalid value \"{}\" for dll notification reason", n),
+        }
+    }
+}
+
+/// DLL watcher for monitoring DLL loads/unloads.
+///
+/// Provides a method to invoke a function-like type any time a DLL
+/// is loaded or unloaded in the current process.
+pub struct DllWatcher<F1, F2>
+where
+    F1: FnMut(DllNotificationData),
+    F2: FnMut(DllNotificationData),
+{
+    context: Box<CallbackContext<F1, F2>>,
+    cookie: Option<ptr::NonNull<c_void>>,
+}
+
+impl<F1, F2> DllWatcher<F1, F2>
+where
+    F1: FnMut(DllNotificationData),
+    F2: FnMut(DllNotificationData),
+{
+    /// Create a new `DllWatcher` with the two callback functions. Takes two
+    /// callbacks, a `loaded_callback` which is called when a DLL is loaded,
+    /// and `unloaded_callback` which is called when a DLL is unloaded.
+    pub fn new(loaded_callback: F1, unloaded_callback: F2) -> io::Result<Self> {
+        let mut watcher = Self {
+            context: Box::new(CallbackContext::new(loaded_callback, unloaded_callback)),
+            cookie: None,
+        };
+        let mut cookie: PVOID = ptr::null_mut();
+        // Safe because we guarantee that the notification function that we
+        // register will have exclusive access to the context.
+        unsafe {
+            LdrRegisterDllNotification(
+                /* Flags= */ 0,
+                /* NotificationFunction= */ watcher.context.get_notification_function(),
+                /* Context= */
+                &mut *watcher.context as *mut CallbackContext<F1, F2> as PVOID,
+                /* Cookie= */ &mut cookie as *mut PVOID,
+            )?
+        };
+        watcher.cookie = ptr::NonNull::new(cookie);
+        Ok(watcher)
+    }
+
+    fn unregister_dll_notification(&mut self) -> io::Result<()> {
+        match self.cookie {
+            Some(c) => {
+                // Safe because we guarantee that `Cookie` was previously initialized.
+                unsafe {
+                    LdrUnregisterDllNotification(/* Cookie= */ c.as_ptr() as PVOID)?
+                }
+                self.cookie = None;
+            }
+            None => {}
+        }
+        Ok(())
+    }
+}
+
+impl<F1, F2> Drop for DllWatcher<F1, F2>
+where
+    F1: FnMut(DllNotificationData),
+    F2: FnMut(DllNotificationData),
+{
+    fn drop(&mut self) {
+        self.unregister_dll_notification()
+            .expect("error unregistering dll notification");
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::{collections::HashSet, ffi::CString, io};
+    use winapi::um::libloaderapi::{FreeLibrary, LoadLibraryA};
+
+    // Arbitrarily chosen DLL for load/unload test. Chosen because it's
+    // hopefully esoteric enough that it's probably not already loaded in
+    // the process so we can test load/unload notifications.
+    const TEST_DLL_NAME: &'static str = "Imagehlp.dll";
+
+    #[test]
+    fn load_dll() {
+        let test_dll_name = CString::new(TEST_DLL_NAME).expect("failed to create CString");
+        let mut loaded_dlls: HashSet<OsString> = HashSet::new();
+        let h_module = {
+            let _watcher = DllWatcher::new(
+                |data| {
+                    loaded_dlls.insert(data.base_dll_name);
+                },
+                |_data| (),
+            )
+            .expect("failed to create DllWatcher");
+            // Safe because we pass a valid C string in to the function.
+            unsafe { LoadLibraryA(test_dll_name.as_ptr()) }
+        };
+        assert!(
+            !h_module.is_null(),
+            "failed to load {}: {}",
+            TEST_DLL_NAME,
+            io::Error::last_os_error()
+        );
+        assert!(
+            loaded_dlls.len() >= 1,
+            "no DLL loads recorded by DLL watcher"
+        );
+        assert!(
+            loaded_dlls.contains::<OsString>(&(TEST_DLL_NAME.to_owned().into())),
+            "{} load wasn't recorded by DLL watcher",
+            TEST_DLL_NAME
+        );
+        // Safe because we initialized h_module with a LoadLibraryA call.
+        let success = unsafe { FreeLibrary(h_module) } > 0;
+        assert!(
+            success,
+            "failed to free {}: {}",
+            TEST_DLL_NAME,
+            io::Error::last_os_error(),
+        )
+    }
+
+    // TODO(b/229288169): re-enable after the test is made more reliable.
+    #[ignore]
+    #[test]
+    fn unload_dll() {
+        let mut unloaded_dlls: HashSet<OsString> = HashSet::new();
+        {
+            let test_dll_name = CString::new(TEST_DLL_NAME).expect("failed to create CString");
+            let _watcher = DllWatcher::new(
+                |_data| (),
+                |data| {
+                    unloaded_dlls.insert(data.base_dll_name);
+                },
+            )
+            .expect("failed to create DllWatcher");
+            // Safe because we pass a valid C string in to the function.
+            let h_module = unsafe { LoadLibraryA(test_dll_name.as_ptr()) };
+            assert!(
+                !h_module.is_null(),
+                "failed to load {}: {}",
+                TEST_DLL_NAME,
+                io::Error::last_os_error()
+            );
+            // Safe because we initialized h_module with a LoadLibraryA call.
+            let success = unsafe { FreeLibrary(h_module) } > 0;
+            assert!(
+                success,
+                "failed to free {}: {}",
+                TEST_DLL_NAME,
+                io::Error::last_os_error(),
+            )
+        };
+        assert!(
+            unloaded_dlls.len() >= 1,
+            "no DLL unloads recorded by DLL watcher"
+        );
+        assert!(
+            unloaded_dlls.contains::<OsString>(&(TEST_DLL_NAME.to_owned().into())),
+            "{} unload wasn't recorded by DLL watcher",
+            TEST_DLL_NAME
+        );
+    }
+}

win_util/src/lib.rs

@@ -16,16 +16,20 @@ pub use crate::large_integer::*;
 mod security_attributes;
 pub use crate::security_attributes::*;
 
-use libc::c_ulong;
-use std::ffi::{CString, OsStr};
+mod dll_notification;
+pub use crate::dll_notification::*;
+
+use std::ffi::{CString, OsStr, OsString};
 use std::iter::once;
 use std::mem::MaybeUninit;
-use std::os::windows::ffi::OsStrExt;
+use std::os::windows::ffi::{OsStrExt, OsStringExt};
 use std::os::windows::io::RawHandle;
-use std::slice;
 use std::sync::Once;
-use std::{io, ptr};
+use std::{io, ptr, slice};
+
+use libc::c_ulong;
 use winapi::shared::minwindef::{DWORD, FALSE, TRUE};
+use winapi::shared::ntdef::UNICODE_STRING;
 use winapi::um::handleapi::{
     CloseHandle, DuplicateHandle, SetHandleInformation, INVALID_HANDLE_VALUE,
 };
@@ -35,7 +39,7 @@ use winapi::um::processthreadsapi::{
 };
 use winapi::um::sysinfoapi::{GetNativeSystemInfo, SYSTEM_INFO};
 use winapi::um::winbase::{CreateFileMappingA, HANDLE_FLAG_INHERIT};
-use winapi::um::winnt::{DUPLICATE_SAME_ACCESS, HRESULT, PROCESS_DUP_HANDLE};
+use winapi::um::winnt::{DUPLICATE_SAME_ACCESS, HRESULT, PROCESS_DUP_HANDLE, WCHAR};
 
 #[macro_export]
 macro_rules! syscall_bail {
@@ -112,6 +116,25 @@ pub unsafe fn from_ptr_win32_wide_string(wide: *const u16) -> String {
     String::from_utf16_lossy(slice)
 }
 
+/// Converts a `UNICODE_STRING` into an `OsString`.
+/// ## Safety
+/// Safe when `unicode_string` is non-null and points to a valid
+/// `UNICODE_STRING` struct.
+pub fn unicode_string_to_os_string(unicode_string: &UNICODE_STRING) -> OsString {
+    // Safe because:
+    // * Buffer is guaranteed to be properly aligned and valid for the
+    //   entire length of the string.
+    // * The slice is only temporary, until we perform the `from_wide`
+    //   conversion with `OsString`, so the memory referenced by the slice is
+    //   not modified during that duration.
+    OsString::from_wide(unsafe {
+        slice::from_raw_parts(
+            unicode_string.Buffer,
+            unicode_string.Length as usize / std::mem::size_of::<WCHAR>(),
+        )
+    })
+}
+
 pub fn duplicate_handle_with_target_pid(hndl: RawHandle, target_pid: u32) -> io::Result<RawHandle> {
     // Safe because caller will guarentee `hndl` and `target_pid` are valid and won't be dropped.
     unsafe {

x86_64/src/cpuid.rs

@@ -70,6 +70,7 @@ pub struct CpuIdContext {
     calibrated_tsc_leaf_required: bool,
     /// Whether or not VCPU IDs and APIC IDs should match host cpu IDs.
     host_cpu_topology: bool,
+    /// Whether to expose core temperature, package temperature  and APEF/MPERF to guest
     enable_pnp_data: bool,
     /// Enable Intel Turbo Boost Max Technology 3.0.
     itmt: bool,

x86_64/src/lib.rs

@@ -67,6 +67,7 @@ use arch::{
     MsrValueFrom, RunnableLinuxVm, VmComponents, VmImage,
 };
 use base::{warn, Event, SendTube, TubeError};
+pub use cpuid::{adjust_cpuid, CpuIdContext};
 #[cfg(windows)]
 use devices::Minijail;
 #[cfg(unix)]