From c9fa2f423d938a6ccc0053799befd2d6f59fdad6 Mon Sep 17 00:00:00 2001 From: Francis Nixon Date: Fri, 22 Jan 2021 12:52:08 -0500 Subject: [PATCH] got rid of check_vm_instruction related allocations. --- mythril/src/acpi/madt.rs | 44 +++++++++++---------- mythril/src/acpi/mod.rs | 6 ++- mythril/src/acpi/rsdp.rs | 21 +++++----- mythril/src/acpi/rsdt.rs | 21 ++++++---- mythril/src/emulate/controlreg.rs | 2 +- mythril/src/emulate/memio.rs | 17 ++++---- mythril/src/error.rs | 15 ++++--- mythril/src/ioapic.rs | 63 +++++++++++++++--------------- mythril/src/linux.rs | 20 +++++----- mythril/src/memory.rs | 3 +- mythril/src/physdev/pit.rs | 3 +- mythril/src/vcpu.rs | 2 +- mythril/src/virtdev/lapic.rs | 8 ++-- mythril/src/virtdev/mod.rs | 22 ++++------- mythril/src/virtdev/pci.rs | 5 +-- mythril/src/virtdev/pit.rs | 20 ++++------ mythril/src/virtdev/qemu_fw_cfg.rs | 5 +-- mythril/src/virtdev/vga.rs | 21 +++++----- mythril/src/vm.rs | 6 +-- mythril/src/vmcs.rs | 8 ++-- mythril/src/vmexit.rs | 7 ++-- mythril/src/vmx.rs | 8 ++-- 22 files changed, 160 insertions(+), 167 deletions(-) diff --git a/mythril/src/acpi/madt.rs b/mythril/src/acpi/madt.rs index aeb19ca..9ebd51d 100644 --- a/mythril/src/acpi/madt.rs +++ b/mythril/src/acpi/madt.rs @@ -16,7 +16,6 @@ use num_enum::TryFromPrimitive; /// SDT (the end of the Creator Revision at offset 36). mod offsets { use super::*; - /// 32-bit physical address at which each processor can access its /// local APIC. pub const LOCAL_INT_CTRL_ADDR: Range = 0..4; @@ -81,8 +80,7 @@ impl IcsType { if length == self.expected_len() as usize - 2 { Ok(()) } else { - error!("Invalid length={} for type=0x{:x}", - *self as u8, length); + error!("Invalid length={} for type=0x{:x}", *self as u8, length); Err(Error::InvalidValue) } } @@ -255,16 +253,18 @@ impl Ics { apic_proc_uid: NativeEndian::read_u32(&bytes[10..14]), }), _ => { - error!("type=0x{:x} length={} not implemented", - ty as u8, - bytes.len()); + error!( + "type=0x{:x} length={} not implemented", + ty as u8, + bytes.len() + ); Err(Error::NotImplemented) } } } /// Encode into the byte sequence - pub fn encode>( + pub fn encode>( &self, buffer: &mut ArrayVec, ) -> Result<()> { @@ -295,8 +295,10 @@ impl Ics { NativeEndian::write_u32(&mut tmp_buf[8..12], gsi_base); } _ => { - error!("The ICS Type {:?} has not been implemented", - self.ics_type()); + error!( + "The ICS Type {:?} has not been implemented", + self.ics_type() + ); return Err(Error::NotImplemented); } } @@ -393,22 +395,22 @@ impl<'a> Iterator for IcsIterator<'a> { let ty = match IcsType::try_from(self.bytes[0]) { Ok(ty) => ty, _ => { - error!("Invalid ICS type: {}", - self.bytes[0]); + error!("Invalid ICS type: {}", self.bytes[0]); return Some(Err(Error::InvalidValue)); } }; let len = self.bytes[1] as usize; if len > self.bytes.len() { - error!("Payload for type=0x{:x} and len={} to big for buffer len={}", - ty as u8, - len, - self.bytes.len()); + error!( + "Payload for type=0x{:x} and len={} to big for buffer len={}", + ty as u8, + len, + self.bytes.len() + ); return Some(Err(Error::InvalidValue)); } else if len < 3 { - error!("length `{}` provided is too small", - len); + error!("length `{}` provided is too small", len); return Some(Err(Error::InvalidValue)); } @@ -434,7 +436,7 @@ pub struct MADTBuilder { structures: ArrayVec, } -impl> MADTBuilder { +impl> MADTBuilder { /// Create a new builder for the MADT SDT. pub fn new() -> MADTBuilder { MADTBuilder { @@ -462,8 +464,8 @@ impl> MADTBuilder { } impl SDTBuilder for MADTBuilder - where - U: Array, +where + U: Array, { const SIGNATURE: [u8; 4] = [b'A', b'P', b'I', b'C']; @@ -472,7 +474,7 @@ impl SDTBuilder for MADTBuilder 5u8 } - fn encode_table>( + fn encode_table>( &mut self, buffer: &mut ArrayVec, ) -> Result<()> { diff --git a/mythril/src/acpi/mod.rs b/mythril/src/acpi/mod.rs index a92068e..05983e7 100644 --- a/mythril/src/acpi/mod.rs +++ b/mythril/src/acpi/mod.rs @@ -48,7 +48,11 @@ pub(self) fn verify_checksum(bytes: &[u8], cksum_idx: usize) -> Result<()> { if (result & 0xff) == 0x00 { Ok(()) } else { - error!("Checksum mismatch checksum={:x} {:x} != 0x00", bytes[cksum_idx], result & 0xff); + error!( + "Checksum mismatch checksum={:x} {:x} != 0x00", + bytes[cksum_idx], + result & 0xff + ); Err(Error::InvalidValue) } } diff --git a/mythril/src/acpi/rsdp.rs b/mythril/src/acpi/rsdp.rs index 6680acd..62fd08e 100644 --- a/mythril/src/acpi/rsdp.rs +++ b/mythril/src/acpi/rsdp.rs @@ -117,10 +117,8 @@ impl RSDP { xsdt_addr: NativeEndian::read_u64(&bytes[offsets::XSDT_ADDR]), }, _ => { - error!("Invalid RSDP revision: {}", - bytes[offsets::REVISION]); - return Err(Error::InvalidValue) - .into(); + error!("Invalid RSDP revision: {}", bytes[offsets::REVISION]); + return Err(Error::InvalidValue).into(); } }; @@ -179,9 +177,11 @@ impl RSDP { Ok(&range[i..rsdp_v2_end]) } _ => { - error!("Invalid RSDP revision: {} at {:p}", - candidate[offsets::REVISION], - candidate.as_ptr()); + error!( + "Invalid RSDP revision: {} at {:p}", + candidate[offsets::REVISION], + candidate.as_ptr() + ); Err(Error::InvalidValue) } }; @@ -205,8 +205,7 @@ impl RSDP { offsets::EXT_CHECKSUM, ), _ => { - error!("Invalid RSDP revision: {}", - bytes[offsets::REVISION]); + error!("Invalid RSDP revision: {}", bytes[offsets::REVISION]); Err(Error::InvalidValue) } } @@ -222,11 +221,11 @@ impl RSDP { } /// Builder structure for the RSDP -pub struct RSDPBuilder<'a, T: Array> { +pub struct RSDPBuilder<'a, T: Array> { builder: RSDTBuilder<'a, T>, } -impl<'a, T: Array> RSDPBuilder<'a, T> { +impl<'a, T: Array> RSDPBuilder<'a, T> { /// Create a new RSDP Builder. pub fn new( map: ManagedMap<'a, [u8; 4], (ArrayVec, usize)>, diff --git a/mythril/src/acpi/rsdt.rs b/mythril/src/acpi/rsdt.rs index 3c21340..a63013f 100644 --- a/mythril/src/acpi/rsdt.rs +++ b/mythril/src/acpi/rsdt.rs @@ -246,9 +246,11 @@ fn write_sdt_header( // The SDT length value is the value of the entire SDT including // the header. if buffer.len() < sdt_len { - error!("Buffer length should be at least `{}` but was `{}`", - sdt_len, - buffer.len()); + error!( + "Buffer length should be at least `{}` but was `{}`", + sdt_len, + buffer.len() + ); return Err(Error::InvalidValue); } // Fill in the SDT header with the implementations values @@ -338,8 +340,10 @@ impl<'a, T: Array> RSDTBuilder<'a, T> { Ok(()) } } else { - error!("The key `{}` already exists", - str::from_utf8(&U::SIGNATURE).unwrap()); + error!( + "The key `{}` already exists", + str::from_utf8(&U::SIGNATURE).unwrap() + ); Err(Error::InvalidValue) } } @@ -371,10 +375,11 @@ impl<'a, T: Array> RSDTBuilder<'a, T> { })?; for (i, (name, (sdt, size))) in self.map.iter().enumerate() { - const LEN_OF_ETC_MYTHRIL :usize = 12; + const LEN_OF_ETC_MYTHRIL: usize = 12; const LEN_OF_NAME: usize = 4; - let mut table_name_bytes = [0u8;LEN_OF_ETC_MYTHRIL + LEN_OF_NAME]; - table_name_bytes[0..LEN_OF_ETC_MYTHRIL].copy_from_slice("etc/mythril/".as_bytes()); + let mut table_name_bytes = [0u8; LEN_OF_ETC_MYTHRIL + LEN_OF_NAME]; + table_name_bytes[0..LEN_OF_ETC_MYTHRIL] + .copy_from_slice("etc/mythril/".as_bytes()); table_name_bytes[LEN_OF_ETC_MYTHRIL..].copy_from_slice(name); let table_name = str::from_utf8(&table_name_bytes)?; diff --git a/mythril/src/emulate/controlreg.rs b/mythril/src/emulate/controlreg.rs index cff1cfe..2b7348f 100644 --- a/mythril/src/emulate/controlreg.rs +++ b/mythril/src/emulate/controlreg.rs @@ -61,7 +61,7 @@ pub fn emulate_access( }, _ => { error!("Unsupported CR number access"); - return Err(Error::InvalidValue) + return Err(Error::InvalidValue); } } Ok(()) diff --git a/mythril/src/emulate/memio.rs b/mythril/src/emulate/memio.rs index 4c63d1f..1f3804f 100644 --- a/mythril/src/emulate/memio.rs +++ b/mythril/src/emulate/memio.rs @@ -145,7 +145,7 @@ fn read_register_value( _ => { error!("Invalid register '{:?}'", register); - return Err(Error::InvalidValue) + return Err(Error::InvalidValue); } } @@ -351,9 +351,8 @@ fn do_mmio_read( } register => { - error!("mmio read into invalid register '{:?}'", - register); - return Err(Error::InvalidValue) + error!("mmio read into invalid register '{:?}'", register); + return Err(Error::InvalidValue); } }, _ => return Err(Error::NotSupported), @@ -431,10 +430,12 @@ fn process_memio_op( { do_mmio_read(addr, vcpu, guest_cpu, responses, instr, on_read)?; } else { - error!("Unsupported mmio instruction: {:?} (rip=0x{:x}, bytes={:?})", - instr.code(), - ip, - bytes); + error!( + "Unsupported mmio instruction: {:?} (rip=0x{:x}, bytes={:?})", + instr.code(), + ip, + bytes + ); return Err(Error::InvalidValue); } Ok(()) diff --git a/mythril/src/error.rs b/mythril/src/error.rs index 7733f8b..e476cbc 100644 --- a/mythril/src/error.rs +++ b/mythril/src/error.rs @@ -1,5 +1,4 @@ use crate::vmcs; -use alloc::string::String; use arrayvec::CapacityError; use core::convert::TryFrom; use core::num::TryFromIntError; @@ -44,11 +43,11 @@ pub enum VmInstructionError { InvalidOperandToInveptInvvpid = 28, } -pub fn check_vm_insruction(rflags: u64, error: String) -> Result<()> { +pub fn check_vm_instruction(rflags: u64, log_error: impl Fn()) -> Result<()> { let rflags = rflags::RFlags::from_bits_truncate(rflags); if rflags.contains(RFlags::FLAGS_CF) { - error!("{}",error); + log_error(); Err(Error::VmFailInvalid) } else if rflags.contains(RFlags::FLAGS_ZF) { let errno = unsafe { @@ -63,8 +62,8 @@ pub fn check_vm_insruction(rflags: u64, error: String) -> Result<()> { let vm_error = VmInstructionError::try_from(errno) .unwrap_or(VmInstructionError::UnknownError); - error!("{:?}",vm_error); - error!("{}",error); + error!("{:?}", vm_error); + log_error(); Err(Error::VmFailValid) } else { Ok(()) @@ -88,21 +87,21 @@ pub enum Error { impl From> for Error { fn from(error: TryFromPrimitiveError) -> Error { - error!("{}",error); + error!("{}", error); Error::InvalidValue } } impl From for Error { fn from(error: TryFromIntError) -> Error { - error!("{}",error); + error!("{}", error); Error::InvalidValue } } impl From for Error { fn from(error: core::str::Utf8Error) -> Error { - error!("{}",error); + error!("{}", error); Error::InvalidValue } } diff --git a/mythril/src/ioapic.rs b/mythril/src/ioapic.rs index 978a1a9..434fda6 100644 --- a/mythril/src/ioapic.rs +++ b/mythril/src/ioapic.rs @@ -171,8 +171,7 @@ impl IoApic { /// See section 3.2.1 of the I/O APIC specification. pub fn set_id(&self, id: u32) -> Result<()> { if id > 0x0f { - error!("I/O APIC ID `0x{:x}` too large", - id); + error!("I/O APIC ID `0x{:x}` too large", id); Err(Error::InvalidValue) } else { unsafe { @@ -211,8 +210,7 @@ impl IoApic { /// See section 3.2.3 of the I/O APIC specification. pub fn set_arbitration_id(&self, id: u8) -> Result<()> { if id > 15 { - error!("I/O APIC Arbitration ID `0x{:x}` too large", - id); + error!("I/O APIC Arbitration ID `0x{:x}` too large", id); Err(Error::InvalidValue) } else { unsafe { @@ -237,8 +235,7 @@ impl IoApic { /// Read the IO Redirect Table Register for a given id. pub fn read_ioredtbl(&self, id: u8) -> Result { if id > 23 { - error!("I/O APIC IO Redirect Table register`0x{:x}` too large", - id); + error!("I/O APIC IO Redirect Table register`0x{:x}` too large", id); Err(Error::InvalidValue) } else { let bits = unsafe { self.read_ioredtbl_raw(id) }; @@ -267,12 +264,13 @@ impl IoApic { pub fn write_ioredtbl(&self, id: u8, entry: IoRedTblEntry) -> Result<()> { let val: u64 = entry.into(); if id > 23 { - error!("I/O APIC IO Redirect Table register`0x{:x}` too large", - id); + error!("I/O APIC IO Redirect Table register`0x{:x}` too large", id); Err(Error::InvalidValue) } else if (val & !IOREDTBL_RW_MASK) != 0 { - error!("Read-only IO Redirect Table Entry bits set: 0x{:x}", - val & !IOREDTBL_RW_MASK); + error!( + "Read-only IO Redirect Table Entry bits set: 0x{:x}", + val & !IOREDTBL_RW_MASK + ); Err(Error::InvalidValue) } else { unsafe { @@ -303,8 +301,10 @@ impl TryFrom for IoApic { .. } => IoApic::new(ioapic_addr, gsi_base), _ => { - error!("Attempting to create an IoApic from: {:?}", - value.ics_type()); + error!( + "Attempting to create an IoApic from: {:?}", + value.ics_type() + ); Err(Error::InvalidValue) } } @@ -346,8 +346,7 @@ impl TryFrom for DestinationMode { 0x00 => Ok(DestinationMode::Physical), 0x01 => Ok(DestinationMode::Logical), _ => { - error!("Invalid destination mode: 0x{:x}", - value); + error!("Invalid destination mode: 0x{:x}", value); Err(Error::InvalidValue) } } @@ -372,8 +371,7 @@ impl TryFrom for TriggerMode { 0x00 => Ok(TriggerMode::Edge), 0x01 => Ok(TriggerMode::Level), _ => { - error!("Invalid trigger mode: 0x{:x}", - value); + error!("Invalid trigger mode: 0x{:x}", value); Err(Error::InvalidValue) } } @@ -398,8 +396,7 @@ impl TryFrom for PinPolarity { 0x00 => Ok(PinPolarity::ActiveHigh), 0x01 => Ok(PinPolarity::ActiveLow), _ => { - error!("Invalid pin polarity: 0x{:x}", - value); + error!("Invalid pin polarity: 0x{:x}", value); Err(Error::InvalidValue) } } @@ -454,10 +451,9 @@ impl TryFrom for DeliveryMode { 0b101 => Ok(DeliveryMode::INIT), 0b111 => Ok(DeliveryMode::ExtINT), _ => { - error!("Invalid pin polarity: 0x{:x}", - value); + error!("Invalid pin polarity: 0x{:x}", value); Err(Error::InvalidValue) - }, + } } } } @@ -480,8 +476,7 @@ impl TryFrom for DeliveryStatus { 0x00 => Ok(DeliveryStatus::Idle), 0x01 => Ok(DeliveryStatus::SendPending), _ => { - error!("Invalid delivery status: 0x{:x}", - value); + error!("Invalid delivery status: 0x{:x}", value); Err(Error::InvalidValue) } } @@ -550,8 +545,10 @@ impl IoRedTblEntry { if self.trigger_mode == TriggerMode::Level && !self.delivery_mode.valid_for_level_trigger() { - error!("The delivery mode `0b{:b}` is invalid for level trigger mode", - self.delivery_mode as u8); + error!( + "The delivery mode `0b{:b}` is invalid for level trigger mode", + self.delivery_mode as u8 + ); return Err(Error::InvalidValue); } @@ -561,14 +558,18 @@ impl IoRedTblEntry { if self.destination_mode == DestinationMode::Physical && self.destination > 15 { - error!("Invalid Physical APIC ID destination: 0x{:x}", - self.destination); + error!( + "Invalid Physical APIC ID destination: 0x{:x}", + self.destination + ); return Err(Error::InvalidValue); } if self.delivery_mode == DeliveryMode::SMI && self.vector != 0 { - error!("SMI delivery mode requires an empty vector: 0x{:x}", - self.vector); + error!( + "SMI delivery mode requires an empty vector: 0x{:x}", + self.vector + ); return Err(Error::InvalidValue); } @@ -701,8 +702,8 @@ mod test { // is used. let invalid_dest = 0xff000000_0000_0000; let err = Error::InvalidValue; - // ( - // "Invalid Physical APIC ID destination: 0xff".to_string(), + // ( + // "Invalid Physical APIC ID destination: 0xff".to_string(), // ); assert_eq!(err, IoRedTblEntry::try_from(invalid_dest).unwrap_err()); } diff --git a/mythril/src/linux.rs b/mythril/src/linux.rs index a7e5233..6bac987 100644 --- a/mythril/src/linux.rs +++ b/mythril/src/linux.rs @@ -32,8 +32,7 @@ pub fn load_linux( let mut kernel = info .find_module(kernel_name.as_ref()) .ok_or_else(|| { - error!("No such kernel '{}'", - kernel_name.as_ref()); + error!("No such kernel '{}'", kernel_name.as_ref()); Error::InvalidValue })? .data() @@ -41,15 +40,13 @@ pub fn load_linux( let initramfs = info .find_module(initramfs_name.as_ref()) .ok_or_else(|| { - error!("No such initramfs '{}'", - initramfs_name.as_ref()); + error!("No such initramfs '{}'", initramfs_name.as_ref()); Error::InvalidValue })? .data(); if kernel.len() < 8192 { - error!("Kernel image is too small ({} < 8192)", - kernel.len()); + error!("Kernel image is too small ({} < 8192)", kernel.len()); return Err(Error::InvalidValue); } @@ -57,8 +54,7 @@ pub fn load_linux( // HdrS if magic != 0x53726448 { - error!("Invalid kernel image (bad magic = 0x{:x})", - magic); + error!("Invalid kernel image (bad magic = 0x{:x})", magic); return Err(Error::InvalidValue); } @@ -129,9 +125,11 @@ pub fn load_linux( } if initramfs.len() as u32 > initrd_max { - error!("Initramfs too large (0x{:x} bytes > max of 0x{:x})", - initramfs.len(), - initrd_max); + error!( + "Initramfs too large (0x{:x} bytes > max of 0x{:x})", + initramfs.len(), + initrd_max + ); return Err(Error::InvalidValue); } diff --git a/mythril/src/memory.rs b/mythril/src/memory.rs index dd8376e..13c12c4 100644 --- a/mythril/src/memory.rs +++ b/mythril/src/memory.rs @@ -748,8 +748,7 @@ fn map_guest_memory( let ept_pte = unsafe { &mut (*ept_pt)[guest_addr.p1_index()] }; if !ept_pte.is_unused() { - error!("Duplicate mapping for address 0x{:x}", - guest_addr.as_u64()); + error!("Duplicate mapping for address 0x{:x}", guest_addr.as_u64()); return Err(Error::DuplicateMapping); } diff --git a/mythril/src/physdev/pit.rs b/mythril/src/physdev/pit.rs index dd77efb..6e1ee7a 100644 --- a/mythril/src/physdev/pit.rs +++ b/mythril/src/physdev/pit.rs @@ -61,8 +61,7 @@ impl TryFrom for OperatingMode { 0b110 => Ok(OperatingMode::Mode2), 0b111 => Ok(OperatingMode::Mode3), _ => { - error!("Invalid PIT operating mode: {}", - val); + error!("Invalid PIT operating mode: {}", val); Err(Error::InvalidValue) } } diff --git a/mythril/src/vcpu.rs b/mythril/src/vcpu.rs index 03d3af7..2ea94bd 100644 --- a/mythril/src/vcpu.rs +++ b/mythril/src/vcpu.rs @@ -171,7 +171,7 @@ impl VCpu { /// Begin execution in the guest context for this core pub fn launch(&mut self) -> Result { let rflags = unsafe { vmlaunch_wrapper() }; - error::check_vm_insruction(rflags, "Failed to launch vm".into())?; + error::check_vm_instruction(rflags, ||error!("Failed to launch vm"))?; unreachable!() } diff --git a/mythril/src/virtdev/lapic.rs b/mythril/src/virtdev/lapic.rs index 7ab01c5..cb46de5 100644 --- a/mythril/src/virtdev/lapic.rs +++ b/mythril/src/virtdev/lapic.rs @@ -68,8 +68,7 @@ impl TryFrom for ApicRegisterOffset { fn try_from(value: u16) -> Result { if value & 0b1111 != 0 { - error!("APIC register offset not aligned: 0x{:x}", - value); + error!("APIC register offset not aligned: 0x{:x}", value); return Err(Error::InvalidValue); } @@ -91,9 +90,8 @@ impl TryFrom for ApicRegisterOffset { ApicRegisterOffset::InterruptCommand((value - 0x300) >> 4) } offset => { - error!("Invalid APIC register offset: 0x{:x}", - offset); - return Err(Error::InvalidValue) + error!("Invalid APIC register offset: 0x{:x}", offset); + return Err(Error::InvalidValue); } }; diff --git a/mythril/src/virtdev/mod.rs b/mythril/src/virtdev/mod.rs index 1dd5d50..0906f45 100644 --- a/mythril/src/virtdev/mod.rs +++ b/mythril/src/virtdev/mod.rs @@ -25,7 +25,7 @@ pub mod vga; const MAX_EVENT_RESPONSES: usize = 8; pub type ResponseEventArray = -ArrayVec<[DeviceEventResponse; MAX_EVENT_RESPONSES]>; + ArrayVec<[DeviceEventResponse; MAX_EVENT_RESPONSES]>; pub type Port = u16; /// Dynamic virtual devices are devices that are not part of the architectural @@ -287,8 +287,7 @@ impl<'a> TryFrom<&'a mut [u8]> for PortReadRequest<'a> { &mut *(buff.as_mut_ptr() as *mut [u8; 4]) }), len => { - error!("Invalid slice length: {}", - len); + error!("Invalid slice length: {}", len); return Err(Error::InvalidValue); } }; @@ -344,8 +343,7 @@ impl<'a> TryFrom<&'a [u8]> for PortWriteRequest<'a> { Self::FourBytes(unsafe { &*(buff.as_ptr() as *const [u8; 4]) }) } len => { - error!("Invalid slice length: {}", - len); + error!("Invalid slice length: {}", len); return Err(Error::InvalidValue); } }; @@ -360,10 +358,9 @@ impl<'a> TryFrom> for u8 { match value { PortWriteRequest::OneByte(val) => Ok(val[0]), val => { - error!("Value {} cannot be converted to u8", - val); + error!("Value {} cannot be converted to u8", val); Err(Error::InvalidValue) - }, + } } } } @@ -375,8 +372,7 @@ impl<'a> TryFrom> for u16 { match value { PortWriteRequest::TwoBytes(val) => Ok(u16::from_be_bytes(*val)), val => { - error!("Value {} cannot be converted to u16", - val); + error!("Value {} cannot be converted to u16", val); Err(Error::InvalidValue) } } @@ -390,8 +386,7 @@ impl<'a> TryFrom> for u32 { match value { PortWriteRequest::FourBytes(val) => Ok(u32::from_be_bytes(*val)), val => { - error!("Value {} cannot be converted to u32", - val); + error!("Value {} cannot be converted to u32", val); Err(Error::InvalidValue) } } @@ -451,8 +446,7 @@ impl<'a> TryInto for MemWriteRequest<'a> { if self.data.len() == 1 { Ok(self.data[0]) } else { - error!("Value {} cannot be converted to u8", - self); + error!("Value {} cannot be converted to u8", self); Err(Error::InvalidValue) } } diff --git a/mythril/src/virtdev/pci.rs b/mythril/src/virtdev/pci.rs index d05dd27..11bf581 100644 --- a/mythril/src/virtdev/pci.rs +++ b/mythril/src/virtdev/pci.rs @@ -233,9 +233,8 @@ impl EmulatedDevice for PciRootComplex { } } _ => { - error!("Invalid PCI port read 0x{:x}", - port); - return Err(Error::InvalidValue) + error!("Invalid PCI port read 0x{:x}", port); + return Err(Error::InvalidValue); } } } diff --git a/mythril/src/virtdev/pit.rs b/mythril/src/virtdev/pit.rs index 8103f86..8aff77d 100644 --- a/mythril/src/virtdev/pit.rs +++ b/mythril/src/virtdev/pit.rs @@ -127,9 +127,8 @@ impl Pit8254 { start_time: None, }, value => { - error!("Invalid PIT operating state '0x{:x}'", - value); - return Err(Error::InvalidValue) + error!("Invalid PIT operating state '0x{:x}'", value); + return Err(Error::InvalidValue); } }; @@ -139,9 +138,8 @@ impl Pit8254 { 0b10 => AccessModeState::HiByte, 0b11 => AccessModeState::Word { lo_byte: None }, value => { - error!("Invalid PIT access state '0x{:x}'", - value); - return Err(Error::InvalidValue) + error!("Invalid PIT access state '0x{:x}'", value); + return Err(Error::InvalidValue); } }; @@ -154,9 +152,8 @@ impl Pit8254 { 0b00 => &mut self.channel0, 0b10 => &mut self.channel2, value => { - error!("Invalid PIT channel '0x{:x}'", - value); - return Err(Error::InvalidValue) + error!("Invalid PIT channel '0x{:x}'", value); + return Err(Error::InvalidValue); } }; @@ -175,9 +172,8 @@ impl Pit8254 { let channel_state = match port { PIT_COUNTER_0 => &mut self.channel0, PIT_COUNTER_1 => { - error!("Invalid PIT port '0x{:x}'", - port); - return Err(Error::InvalidValue) + error!("Invalid PIT port '0x{:x}'", port); + return Err(Error::InvalidValue); } PIT_COUNTER_2 => &mut self.channel2, _ => unreachable!(), diff --git a/mythril/src/virtdev/qemu_fw_cfg.rs b/mythril/src/virtdev/qemu_fw_cfg.rs index db198e4..f2f1d08 100644 --- a/mythril/src/virtdev/qemu_fw_cfg.rs +++ b/mythril/src/virtdev/qemu_fw_cfg.rs @@ -191,8 +191,7 @@ impl QemuFwCfgBuilder { data: &[u8], ) -> Result<()> { if name.as_ref().len() > FW_CFG_MAX_FILE_NAME { - error!("qemu_fw_cfg: file name too long: {}", - name.as_ref()); + error!("qemu_fw_cfg: file name too long: {}", name.as_ref()); return Err(Error::InvalidValue); } let selector = self.next_file_selector(); @@ -369,7 +368,7 @@ impl QemuFwCfg { } Self::FW_CFG_PORT_DATA => { error!("Write to QEMU FW CFG data port not yet supported"); - return Err(Error::NotImplemented) + return Err(Error::NotImplemented); } Self::FW_CFG_PORT_DMA_LOW => { let low = u32::from_be(val.try_into()?); diff --git a/mythril/src/virtdev/vga.rs b/mythril/src/virtdev/vga.rs index d3563de..cdc077e 100644 --- a/mythril/src/virtdev/vga.rs +++ b/mythril/src/virtdev/vga.rs @@ -75,9 +75,11 @@ impl VgaController { val.copy_from_u32(self.registers[self.index as usize] as u32); } _ => { - error!("Unsupported attempt to read from vga port 0x{:x}", - port); - return Err(Error::NotImplemented) + error!( + "Unsupported attempt to read from vga port 0x{:x}", + port + ); + return Err(Error::NotImplemented); } } Ok(()) @@ -104,18 +106,19 @@ impl VgaController { self.registers[self.index as usize] = data; } _ => { - error!("Invalid port write to VGA index register: {:?}", - val); - return Err(Error::InvalidValue) + error!( + "Invalid port write to VGA index register: {:?}", + val + ); + return Err(Error::InvalidValue); } }, Self::VGA_DATA => { self.registers[self.index as usize] = val.try_into()?; } _ => { - error!("Unsupported attempt to write to vga port 0x{:x}", - port); - return Err(Error::NotImplemented) + error!("Unsupported attempt to write to vga port 0x{:x}", port); + return Err(Error::NotImplemented); } } Ok(()) diff --git a/mythril/src/vm.rs b/mythril/src/vm.rs index 0618d75..07fea2e 100644 --- a/mythril/src/vm.rs +++ b/mythril/src/vm.rs @@ -225,8 +225,7 @@ impl VirtualMachineSet { .context_by_core_id(core_id) .ok_or_else(|| Error::NotFound)?; context.msgqueue.write().push_back(msg).map_err(|_| { - error!("RX queue is full for core_id = {}", - core_id); + error!("RX queue is full for core_id = {}", core_id); Error::InvalidValue })?; @@ -264,8 +263,7 @@ impl VirtualMachineSet { notify: bool, ) -> Result<()> { let vm_bsp = self.bsp_core_id(vm_id).ok_or_else(|| { - error!("Unable to find BSP for VM id '{}'", - vm_id); + error!("Unable to find BSP for VM id '{}'", vm_id); Error::InvalidValue })?; self.send_msg_core(msg, vm_bsp, notify) diff --git a/mythril/src/vmcs.rs b/mythril/src/vmcs.rs index d2d4976..e0d402c 100644 --- a/mythril/src/vmcs.rs +++ b/mythril/src/vmcs.rs @@ -333,9 +333,9 @@ fn vmcs_write(field: VmcsField, value: u64) -> Result<()> { rflags }; - error::check_vm_insruction( + error::check_vm_instruction( rflags, - format!("Failed to write 0x{:x} to field {:?}", value, field), + ||error!("Failed to write 0x{:x} to field {:?}", value, field), ) } @@ -369,7 +369,7 @@ fn vmcs_activate(vmcs: &mut Vmcs, _vmx: &vmx::Vmx) -> Result<()> { rflags }; - error::check_vm_insruction(rflags, "Failed to activate VMCS".into()) + error::check_vm_instruction(rflags, ||error!("Failed to activate VMCS")) } fn vmcs_clear(vmcs_page: &mut Raw4kPage) -> Result<()> { @@ -382,7 +382,7 @@ fn vmcs_clear(vmcs_page: &mut Raw4kPage) -> Result<()> { : "volatile"); rflags }; - error::check_vm_insruction(rflags, "Failed to clear VMCS".into()) + error::check_vm_instruction(rflags, ||error!("Failed to clear VMCS")) } pub struct Vmcs { diff --git a/mythril/src/vmexit.rs b/mythril/src/vmexit.rs index c568775..a4e6c9e 100644 --- a/mythril/src/vmexit.rs +++ b/mythril/src/vmexit.rs @@ -54,7 +54,7 @@ pub extern "C" fn vmexit_handler(state: *mut GuestCpuState) { #[no_mangle] pub extern "C" fn vmresume_failure_handler(rflags: u64) { - error::check_vm_insruction(rflags, "Failed to vmresume".into()) + error::check_vm_instruction(rflags,|| error!("Failed to vmresume")) .expect("vmresume failed"); } @@ -221,9 +221,8 @@ impl ExitReason { 63 => ExitInformation::Xsaves, 64 => ExitInformation::Xrstors, reason => { - error!("Unexpected basic vmexit reason: {}", - reason); - return Err(Error::InvalidValue) + error!("Unexpected basic vmexit reason: {}", reason); + return Err(Error::InvalidValue); } }; Ok(ExitReason { diff --git a/mythril/src/vmx.rs b/mythril/src/vmx.rs index aae45e3..df37876 100644 --- a/mythril/src/vmx.rs +++ b/mythril/src/vmx.rs @@ -52,7 +52,7 @@ impl Vmx { rflags }; - error::check_vm_insruction(rflags, "Failed to enable vmx".into())?; + error::check_vm_instruction(rflags, ||error!("Failed to enable vmx"))?; Ok(Vmx { _vmxon_region: vmxon_region, }) @@ -70,7 +70,7 @@ impl Vmx { rflags }; - error::check_vm_insruction(rflags, "Failed to disable vmx".into()) + error::check_vm_instruction(rflags, ||error!("Failed to disable vmx")) } pub fn revision() -> u32 { @@ -90,7 +90,7 @@ impl Vmx { : "m"(val), "r"(t)); rflags }; - error::check_vm_insruction(rflags, "Failed to execute invept".into()) + error::check_vm_instruction(rflags, ||error!("Failed to execute invept")) } pub fn invvpid(&self, mode: InvVpidMode) -> Result<()> { @@ -112,7 +112,7 @@ impl Vmx { : "m"(val), "r"(t)); rflags }; - error::check_vm_insruction(rflags, "Failed to execute invvpid".into()) + error::check_vm_instruction(rflags, ||error!("Failed to execute invvpid")) } }