diff --git a/patina_dxe_core/resources/test/invalid_directory_name_offset_hii.pe32 b/patina_dxe_core/resources/test/invalid_directory_name_offset_hii.pe32 new file mode 100644 index 000000000..a2945759d Binary files /dev/null and b/patina_dxe_core/resources/test/invalid_directory_name_offset_hii.pe32 differ diff --git a/patina_dxe_core/resources/test/invalid_relocation_directory_size.efi b/patina_dxe_core/resources/test/invalid_relocation_directory_size.efi new file mode 100644 index 000000000..40187d9ce Binary files /dev/null and b/patina_dxe_core/resources/test/invalid_relocation_directory_size.efi differ diff --git a/patina_dxe_core/resources/test/invalid_size_of_image.efi b/patina_dxe_core/resources/test/invalid_size_of_image.efi new file mode 100644 index 000000000..c9718b161 Binary files /dev/null and b/patina_dxe_core/resources/test/invalid_size_of_image.efi differ diff --git a/patina_dxe_core/resources/test/section_alignment_200.efi b/patina_dxe_core/resources/test/section_alignment_200.efi new file mode 100644 index 000000000..2a6b29db5 Binary files /dev/null and b/patina_dxe_core/resources/test/section_alignment_200.efi differ diff --git a/patina_dxe_core/resources/test/subsystem_efi_application.efi b/patina_dxe_core/resources/test/subsystem_efi_application.efi new file mode 100644 index 000000000..89d046bff Binary files /dev/null and b/patina_dxe_core/resources/test/subsystem_efi_application.efi differ diff --git a/patina_dxe_core/resources/test/subsystem_efi_runtime_driver.efi b/patina_dxe_core/resources/test/subsystem_efi_runtime_driver.efi new file mode 100644 index 000000000..295a91c90 Binary files /dev/null and b/patina_dxe_core/resources/test/subsystem_efi_runtime_driver.efi differ diff --git a/patina_dxe_core/resources/test/windows_console_app.exe b/patina_dxe_core/resources/test/windows_console_app.exe new file mode 100644 index 000000000..d103f8431 Binary files /dev/null and b/patina_dxe_core/resources/test/windows_console_app.exe differ diff --git a/patina_dxe_core/src/image.rs b/patina_dxe_core/src/image.rs index f30200e75..33b956e4a 100644 --- a/patina_dxe_core/src/image.rs +++ b/patina_dxe_core/src/image.rs @@ -508,13 +508,15 @@ fn core_load_pe_image( .inspect_err(|err| log::error!("core_load_pe_image failed: UefiPeInfo::parse returned {err:?}")) .map_err(|_| EfiError::Unsupported)?; + let pe_file_name = pe_info.filename.as_deref().unwrap_or("Unknown"); + // based on the image type, determine the correct allocator and code/data types. let (code_type, data_type) = match pe_info.image_type { EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION => (efi::LOADER_CODE, efi::LOADER_DATA), EFI_IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER => (efi::BOOT_SERVICES_CODE, efi::BOOT_SERVICES_DATA), EFI_IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER => (efi::RUNTIME_SERVICES_CODE, efi::RUNTIME_SERVICES_DATA), unsupported_type => { - log::error!("core_load_pe_image_failed: unsupported image type: {unsupported_type:#x?}"); + log::error!("core_load_pe_image failed: {pe_file_name} unsupported image type: {unsupported_type:#x?}"); return Err(EfiError::Unsupported); } }; @@ -523,18 +525,18 @@ fn core_load_pe_image( let size = pe_info.size_of_image as usize; // the section alignment must be at least the size of a page - if !alignment.is_multiple_of(UEFI_PAGE_SIZE) || alignment == 0 { + if !alignment.is_multiple_of(UEFI_PAGE_SIZE) { log::error!( - "core_load_pe_image_failed: section alignment of {alignment:#x?} is not a (non-zero) multiple of page size {UEFI_PAGE_SIZE:#x?}", + "core_load_pe_image failed: {pe_file_name} section alignment of {alignment:#x?} is not a multiple of page size {UEFI_PAGE_SIZE:#x?}" ); - debug_assert!(false); return Err(EfiError::LoadError); } // the size of the image must be a multiple of the section alignment per PE/COFF spec if !size.is_multiple_of(alignment) { - log::error!("core_load_pe_image_failed: size of image is not a multiple of the section alignment"); - debug_assert!(false); + log::error!( + "core_load_pe_image failed: {pe_file_name} size of image is not a multiple of the section alignment" + ); return Err(EfiError::LoadError); } @@ -548,13 +550,15 @@ fn core_load_pe_image( //load the image into the new loaded image buffer pecoff::load_image(&pe_info, image, loaded_image) - .inspect_err(|err| log::error!("core_load_pe_image_failed: load_image returned status: {err:?}")) + .inspect_err(|err| log::error!("core_load_pe_image failed: {pe_file_name} load_image returned status: {err:?}")) .map_err(|_| EfiError::LoadError)?; //relocate the image to the address at which it was loaded. let loaded_image_addr = private_info.image_info.image_base as usize; private_info.relocation_data = pecoff::relocate_image(&pe_info, loaded_image_addr, loaded_image, &Vec::new()) - .inspect_err(|err| log::error!("core_load_pe_image_failed: relocate_image returned status: {err:?}")) + .inspect_err(|err| { + log::error!("core_load_pe_image failed: {pe_file_name} relocate_image returned status: {err:?}") + }) .map_err(|_| EfiError::LoadError)?; // update the entry point. Transmute is required here to cast the raw function address to the ImageEntryPoint function pointer type. @@ -565,7 +569,9 @@ fn core_load_pe_image( }; let result = pecoff::load_resource_section(&pe_info, image) - .inspect_err(|err| log::error!("core_load_pe_image_failed: load_resource_section returned status: {err:?}")) + .inspect_err(|err| { + log::error!("core_load_pe_image failed: {pe_file_name} load_resource_section returned status: {err:?}") + }) .map_err(|_| EfiError::LoadError)?; if let Some((resource_section_offset, resource_section_size)) = result { @@ -579,13 +585,12 @@ fn core_load_pe_image( &image_buf_ref[resource_section_offset..resource_section_offset + resource_section_size], ); - log::info!("HII Resource Section found for {}.", pe_info.filename.as_deref().unwrap_or("Unknown")); + log::info!("HII Resource Section found for {pe_file_name}."); } else { log::error!( - "HII Resource Section offset {:#X} and size {:#X} are out of bounds for image {:?}.", + "HII Resource Section offset {:#X} and size {:#X} are out of bounds for image {pe_file_name}.", resource_section_offset, - resource_section_size, - pe_info.filename.as_deref().unwrap_or("Unknown") + resource_section_size ); debug_assert!(false); } @@ -1427,6 +1432,7 @@ mod tests { fn with_locked_state(f: F) { // SAFETY: Test code only - initializing test infrastructure within the global test lock. test_support::with_global_lock(|| unsafe { + test_support::init_test_logger(); test_support::init_test_gcd(None); test_support::init_test_protocol_db(); init_system_table(); @@ -1507,6 +1513,153 @@ mod tests { }); } + #[test] + fn load_image_should_pass_for_subsystem_efi_application() { + with_locked_state(|| { + let mut test_file = + File::open(test_collateral!("subsystem_efi_application.efi")).expect("failed to open test file."); + let mut image: Vec = Vec::new(); + test_file.read_to_end(&mut image).expect("failed to read test file"); + + let mut image_handle: efi::Handle = core::ptr::null_mut(); + let status = load_image( + false.into(), + protocol_db::DXE_CORE_HANDLE, + core::ptr::null_mut(), + image.as_mut_ptr() as *mut c_void, + image.len(), + core::ptr::addr_of_mut!(image_handle), + ); + assert_eq!(status, efi::Status::SUCCESS); + }); + } + + #[test] + fn load_image_should_pass_for_subsystem_efi_runtime_driver() { + with_locked_state(|| { + let mut test_file = + File::open(test_collateral!("subsystem_efi_runtime_driver.efi")).expect("failed to open test file."); + let mut image: Vec = Vec::new(); + test_file.read_to_end(&mut image).expect("failed to read test file"); + + let mut image_handle: efi::Handle = core::ptr::null_mut(); + let status = load_image( + false.into(), + protocol_db::DXE_CORE_HANDLE, + core::ptr::null_mut(), + image.as_mut_ptr() as *mut c_void, + image.len(), + core::ptr::addr_of_mut!(image_handle), + ); + assert_eq!(status, efi::Status::SUCCESS); + }); + } + + #[test] + fn load_image_should_fail_for_windows_image() { + with_locked_state(|| { + let mut test_file = + File::open(test_collateral!("windows_console_app.exe")).expect("failed to open test file."); + let mut image: Vec = Vec::new(); + test_file.read_to_end(&mut image).expect("failed to read test file"); + + let mut image_handle: efi::Handle = core::ptr::null_mut(); + let status = load_image( + false.into(), + protocol_db::DXE_CORE_HANDLE, + core::ptr::null_mut(), + image.as_mut_ptr() as *mut c_void, + image.len(), + core::ptr::addr_of_mut!(image_handle), + ); + assert_eq!(status, efi::Status::UNSUPPORTED); + }); + } + + #[test] + fn load_image_should_fail_for_section_alignment_not_multiple_of_uefi_page_size() { + with_locked_state(|| { + let mut test_file = + File::open(test_collateral!("section_alignment_200.efi")).expect("failed to open test file."); + let mut image: Vec = Vec::new(); + test_file.read_to_end(&mut image).expect("failed to read test file"); + + let mut image_handle: efi::Handle = core::ptr::null_mut(); + let status = load_image( + false.into(), + protocol_db::DXE_CORE_HANDLE, + core::ptr::null_mut(), + image.as_mut_ptr() as *mut c_void, + image.len(), + core::ptr::addr_of_mut!(image_handle), + ); + assert_eq!(status, efi::Status::LOAD_ERROR); + }); + } + + #[test] + fn load_image_should_fail_for_incorrect_size_of_image() { + with_locked_state(|| { + let mut test_file = + File::open(test_collateral!("invalid_size_of_image.efi")).expect("failed to open test file."); + let mut image: Vec = Vec::new(); + test_file.read_to_end(&mut image).expect("failed to read test file"); + + let mut image_handle: efi::Handle = core::ptr::null_mut(); + let status = load_image( + false.into(), + protocol_db::DXE_CORE_HANDLE, + core::ptr::null_mut(), + image.as_mut_ptr() as *mut c_void, + image.len(), + core::ptr::addr_of_mut!(image_handle), + ); + assert_eq!(status, efi::Status::LOAD_ERROR); + }); + } + + #[test] + fn load_image_should_fail_for_hii_section_has_invalid_directory_name_offset() { + with_locked_state(|| { + let mut test_file = File::open(test_collateral!("invalid_directory_name_offset_hii.pe32")) + .expect("failed to open test file."); + let mut image: Vec = Vec::new(); + test_file.read_to_end(&mut image).expect("failed to read test file"); + + let mut image_handle: efi::Handle = core::ptr::null_mut(); + let status = load_image( + false.into(), + protocol_db::DXE_CORE_HANDLE, + core::ptr::null_mut(), + image.as_mut_ptr() as *mut c_void, + image.len(), + core::ptr::addr_of_mut!(image_handle), + ); + assert_eq!(status, efi::Status::LOAD_ERROR); + }); + } + + #[test] + fn load_image_should_fail_for_invalid_relocation_directory_size() { + with_locked_state(|| { + let mut test_file = File::open(test_collateral!("invalid_relocation_directory_size.efi")) + .expect("failed to open test file."); + let mut image: Vec = Vec::new(); + test_file.read_to_end(&mut image).expect("failed to read test file"); + + let mut image_handle: efi::Handle = core::ptr::null_mut(); + let status = load_image( + false.into(), + protocol_db::DXE_CORE_HANDLE, + core::ptr::null_mut(), + image.as_mut_ptr() as *mut c_void, + image.len(), + core::ptr::addr_of_mut!(image_handle), + ); + assert_eq!(status, efi::Status::LOAD_ERROR); + }); + } + #[test] fn load_image_should_authenticate_the_image_with_security_arch() { with_locked_state(|| { diff --git a/patina_dxe_core/src/test_support.rs b/patina_dxe_core/src/test_support.rs index f21258d0e..255a3280b 100644 --- a/patina_dxe_core/src/test_support.rs +++ b/patina_dxe_core/src/test_support.rs @@ -525,6 +525,28 @@ pub(crate) fn build_test_hob_list(mem_size: u64) -> *const c_void { mem.as_ptr() as *const c_void } +struct ConsoleLogger; + +impl log::Log for ConsoleLogger { + fn enabled(&self, _metadata: &log::Metadata) -> bool { + true + } + + fn log(&self, record: &log::Record) { + if self.enabled(record.metadata()) { + println!("[{}] {}", record.level(), record.args()); + } + } + + fn flush(&self) {} +} + +pub(crate) fn init_test_logger() { + static LOGGER: ConsoleLogger = ConsoleLogger; + let _ = log::set_logger(&LOGGER); + log::set_max_level(log::LevelFilter::Info); +} + #[cfg(test)] #[coverage(off)] mod tests {