From e2903989da89b43722efca49bcc21749014739ff Mon Sep 17 00:00:00 2001 From: Ayush Singh Date: Sun, 31 Mar 2024 01:06:33 +0530 Subject: [PATCH] uefi: process: Fixes from PR - Update system table crc32 - Fix unsound use of Box - Free exit data - Code improvements - Introduce OwnedTable - Update r-efi to latest version - Use extended_varargs_abi_support for install_multiple_protocol_interfaces and uninstall_multiple_protocol_interfaces - Fix comments - Stub out args implementation Signed-off-by: Ayush Singh --- library/std/Cargo.toml | 2 +- library/std/src/lib.rs | 1 + library/std/src/sys/pal/uefi/helpers.rs | 131 +++++++++---- library/std/src/sys/pal/uefi/process.rs | 250 +++++++++++++----------- 4 files changed, 232 insertions(+), 152 deletions(-) diff --git a/library/std/Cargo.toml b/library/std/Cargo.toml index b991b1cf22dd8..3db2f1138605d 100644 --- a/library/std/Cargo.toml +++ b/library/std/Cargo.toml @@ -56,7 +56,7 @@ hermit-abi = { version = "0.4.0", features = ['rustc-dep-of-std'], public = true wasi = { version = "0.11.0", features = ['rustc-dep-of-std'], default-features = false } [target.'cfg(target_os = "uefi")'.dependencies] -r-efi = { version = "4.2.0", features = ['rustc-dep-of-std'] } +r-efi = { version = "4.5.0", features = ['rustc-dep-of-std'] } r-efi-alloc = { version = "1.0.0", features = ['rustc-dep-of-std'] } [features] diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index f0a73a308a4a4..eb30a3355e5b6 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -293,6 +293,7 @@ #![feature(doc_masked)] #![feature(doc_notable_trait)] #![feature(dropck_eyepatch)] +#![feature(extended_varargs_abi_support)] #![feature(f128)] #![feature(f16)] #![feature(if_let_guard)] diff --git a/library/std/src/sys/pal/uefi/helpers.rs b/library/std/src/sys/pal/uefi/helpers.rs index c2419bbb58573..29984a915b894 100644 --- a/library/std/src/sys/pal/uefi/helpers.rs +++ b/library/std/src/sys/pal/uefi/helpers.rs @@ -21,6 +21,12 @@ use crate::slice; use crate::sync::atomic::{AtomicPtr, Ordering}; use crate::sys_common::wstr::WStrUnits; +type BootInstallMultipleProtocolInterfaces = + unsafe extern "efiapi" fn(_: *mut r_efi::efi::Handle, _: ...) -> r_efi::efi::Status; + +type BootUninstallMultipleProtocolInterfaces = + unsafe extern "efiapi" fn(_: r_efi::efi::Handle, _: ...) -> r_efi::efi::Status; + const BOOT_SERVICES_UNAVAILABLE: io::Error = const_io_error!(io::ErrorKind::Other, "Boot Services are no longer available"); @@ -231,6 +237,13 @@ impl DevicePath { protocol: NonNull, ) -> io::Result { let path_vec = p.encode_wide().chain(Some(0)).collect::>(); + if path_vec[..path_vec.len() - 1].contains(&0) { + return Err(const_io_error!( + io::ErrorKind::InvalidInput, + "strings passed to UEFI cannot contain NULs", + )); + } + let path = unsafe { ((*protocol.as_ptr()).convert_text_to_device_path)(path_vec.as_ptr()) }; @@ -267,17 +280,9 @@ impl DevicePath { "DevicePathFromText Protocol not found" )) } -} - -impl AsRef for DevicePath { - fn as_ref(&self) -> &r_efi::protocols::device_path::Protocol { - unsafe { self.0.as_ref() } - } -} -impl AsMut for DevicePath { - fn as_mut(&mut self) -> &mut r_efi::protocols::device_path::Protocol { - unsafe { self.0.as_mut() } + pub(crate) fn as_ptr(&self) -> *mut r_efi::protocols::device_path::Protocol { + self.0.as_ptr() } } @@ -292,44 +297,42 @@ impl Drop for DevicePath { } } -pub(crate) struct Protocol { +pub(crate) struct OwnedProtocol { guid: r_efi::efi::Guid, handle: NonNull, - protocol: Box, + protocol: *mut T, } -impl Protocol { - const fn new( - guid: r_efi::efi::Guid, - handle: NonNull, - protocol: Box, - ) -> Self { - Self { guid, handle, protocol } - } - - pub(crate) fn create(protocol: T, mut guid: r_efi::efi::Guid) -> io::Result { - let boot_services: NonNull = +impl OwnedProtocol { + // FIXME: Consider using unsafe trait for matching protocol with guid + pub(crate) unsafe fn create(protocol: T, mut guid: r_efi::efi::Guid) -> io::Result { + let bt: NonNull = boot_services().ok_or(BOOT_SERVICES_UNAVAILABLE)?.cast(); - let mut protocol = Box::new(protocol); + let protocol: *mut T = Box::into_raw(Box::new(protocol)); let mut handle: r_efi::efi::Handle = crate::ptr::null_mut(); + // FIXME: Move into r-efi once extended_varargs_abi_support is stablized + let func: BootInstallMultipleProtocolInterfaces = + unsafe { crate::mem::transmute((*bt.as_ptr()).install_multiple_protocol_interfaces) }; + let r = unsafe { - ((*boot_services.as_ptr()).install_protocol_interface)( + func( &mut handle, - &mut guid, - r_efi::efi::NATIVE_INTERFACE, - protocol.as_mut() as *mut T as *mut crate::ffi::c_void, + &mut guid as *mut _ as *mut crate::ffi::c_void, + protocol as *mut crate::ffi::c_void, + crate::ptr::null_mut() as *mut crate::ffi::c_void, ) }; if r.is_error() { + drop(unsafe { Box::from_raw(protocol) }); return Err(crate::io::Error::from_raw_os_error(r.as_usize())); }; let handle = NonNull::new(handle) .ok_or(io::const_io_error!(io::ErrorKind::Uncategorized, "found null handle"))?; - Ok(Self::new(guid, handle, protocol)) + Ok(Self { guid, handle, protocol }) } pub(crate) fn handle(&self) -> NonNull { @@ -337,29 +340,79 @@ impl Protocol { } } -impl Drop for Protocol { +impl Drop for OwnedProtocol { fn drop(&mut self) { + // Do not deallocate a runtime protocol if let Some(bt) = boot_services() { let bt: NonNull = bt.cast(); - unsafe { - ((*bt.as_ptr()).uninstall_protocol_interface)( + // FIXME: Move into r-efi once extended_varargs_abi_support is stablized + let func: BootUninstallMultipleProtocolInterfaces = unsafe { + crate::mem::transmute((*bt.as_ptr()).uninstall_multiple_protocol_interfaces) + }; + let status = unsafe { + func( self.handle.as_ptr(), - &mut self.guid, - self.protocol.as_mut() as *mut T as *mut crate::ffi::c_void, + &mut self.guid as *mut _ as *mut crate::ffi::c_void, + self.protocol as *mut crate::ffi::c_void, + crate::ptr::null_mut() as *mut crate::ffi::c_void, ) }; + + // Leak the protocol in case uninstall fails + if status == r_efi::efi::Status::SUCCESS { + let _ = unsafe { Box::from_raw(self.protocol) }; + } } } } -impl AsRef for Protocol { +impl AsRef for OwnedProtocol { fn as_ref(&self) -> &T { - &self.protocol + unsafe { self.protocol.as_ref().unwrap() } + } +} + +pub(crate) struct OwnedTable { + layout: crate::alloc::Layout, + ptr: *mut T, +} + +impl OwnedTable { + pub(crate) fn from_table_header(hdr: &r_efi::efi::TableHeader) -> Self { + let header_size = hdr.header_size as usize; + let layout = crate::alloc::Layout::from_size_align(header_size, 8).unwrap(); + let ptr = unsafe { crate::alloc::alloc(layout) as *mut T }; + Self { layout, ptr } + } + + pub(crate) const fn as_ptr(&self) -> *const T { + self.ptr + } + + pub(crate) const fn as_mut_ptr(&self) -> *mut T { + self.ptr } } -impl AsMut for Protocol { - fn as_mut(&mut self) -> &mut T { - &mut self.protocol +impl OwnedTable { + pub(crate) fn from_table(tbl: *const r_efi::efi::SystemTable) -> Self { + let hdr = unsafe { (*tbl).hdr }; + + let owned_tbl = Self::from_table_header(&hdr); + unsafe { + crate::ptr::copy_nonoverlapping( + tbl as *const u8, + owned_tbl.as_mut_ptr() as *mut u8, + hdr.header_size as usize, + ) + }; + + owned_tbl + } +} + +impl Drop for OwnedTable { + fn drop(&mut self) { + unsafe { crate::alloc::dealloc(self.ptr as *mut u8, self.layout) }; } } diff --git a/library/std/src/sys/pal/uefi/process.rs b/library/std/src/sys/pal/uefi/process.rs index 7075af186f9bd..5c7c8415ee295 100644 --- a/library/std/src/sys/pal/uefi/process.rs +++ b/library/std/src/sys/pal/uefi/process.rs @@ -20,9 +20,9 @@ use super::helpers; // Command //////////////////////////////////////////////////////////////////////////////// +#[derive(Debug)] pub struct Command { prog: OsString, - args: Vec, stdout: Option, stderr: Option, } @@ -35,6 +35,7 @@ pub struct StdioPipes { pub stderr: Option, } +#[derive(Copy, Clone, Debug)] pub enum Stdio { Inherit, Null, @@ -43,11 +44,12 @@ pub enum Stdio { impl Command { pub fn new(program: &OsStr) -> Command { - Command { prog: program.to_os_string(), args: Vec::new(), stdout: None, stderr: None } + Command { prog: program.to_os_string(), stdout: None, stderr: None } } - pub fn arg(&mut self, arg: &OsStr) { - self.args.push(arg.to_os_string()); + // FIXME: Implement arguments as reverse of parsing algorithm + pub fn arg(&mut self, _arg: &OsStr) { + panic!("unsupported") } pub fn env_mut(&mut self) -> &mut CommandEnv { @@ -75,7 +77,7 @@ impl Command { } pub fn get_args(&self) -> CommandArgs<'_> { - CommandArgs { iter: self.args.iter() } + panic!("unsupported") } pub fn get_envs(&self) -> CommandEnvs<'_> { @@ -96,65 +98,47 @@ impl Command { fn create_pipe( s: Stdio, - ) -> io::Result>> { + ) -> io::Result>> { match s { - Stdio::MakePipe => helpers::Protocol::create( - uefi_command_internal::PipeProtocol::new(), - simple_text_output::PROTOCOL_GUID, - ) + Stdio::MakePipe => unsafe { + helpers::OwnedProtocol::create( + uefi_command_internal::PipeProtocol::new(), + simple_text_output::PROTOCOL_GUID, + ) + } .map(Some), - Stdio::Null => helpers::Protocol::create( - uefi_command_internal::PipeProtocol::null(), - simple_text_output::PROTOCOL_GUID, - ) + Stdio::Null => unsafe { + helpers::OwnedProtocol::create( + uefi_command_internal::PipeProtocol::null(), + simple_text_output::PROTOCOL_GUID, + ) + } .map(Some), Stdio::Inherit => Ok(None), } } pub fn output(&mut self) -> io::Result<(ExitStatus, Vec, Vec)> { - let mut cmd = uefi_command_internal::Command::load_image(&self.prog)?; - - /* Setup Stdout */ - let stdout: Option> = - match self.stdout.take() { - Some(s) => Self::create_pipe(s), - None => helpers::Protocol::create( - uefi_command_internal::PipeProtocol::new(), - simple_text_output::PROTOCOL_GUID, - ) - .map(Some), - }?; - match stdout { - Some(stdout) => cmd.stdout_init(stdout), - None => cmd.stdout_inherit(), + let mut cmd = uefi_command_internal::Image::load_image(&self.prog)?; + + // Setup Stdout + let stdout = self.stdout.unwrap_or(Stdio::MakePipe); + let stdout = Self::create_pipe(stdout)?; + if let Some(con) = stdout { + cmd.stdout_init(con) + } else { + cmd.stdout_inherit() }; - /* Setup Stderr */ - let stderr: Option> = - match self.stderr.take() { - Some(s) => Self::create_pipe(s), - None => helpers::Protocol::create( - uefi_command_internal::PipeProtocol::new(), - simple_text_output::PROTOCOL_GUID, - ) - .map(Some), - }?; - match stderr { - Some(stderr) => cmd.stderr_init(stderr), - None => cmd.stderr_inherit(), + // Setup Stderr + let stderr = self.stderr.unwrap_or(Stdio::MakePipe); + let stderr = Self::create_pipe(stderr)?; + if let Some(con) = stderr { + cmd.stderr_init(con) + } else { + cmd.stderr_inherit() }; - /* No reason to set args if only program name is preset */ - if !self.args.is_empty() { - let args = self.args.iter().fold(OsString::from(&self.prog), |mut acc, arg| { - acc.push(" "); - acc.push(arg); - acc - }); - cmd.set_args(&args); - } - let stat = cmd.start_image()?; let stdout = cmd.stdout()?; @@ -194,16 +178,6 @@ impl From for Stdio { } } -impl fmt::Debug for Command { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.prog.fmt(f)?; - for arg in &self.args { - arg.fmt(f)?; - } - Ok(()) - } -} - #[derive(PartialEq, Eq, Clone, Copy, Debug)] #[non_exhaustive] pub struct ExitStatus(r_efi::efi::Status); @@ -326,6 +300,7 @@ impl<'a> fmt::Debug for CommandArgs<'a> { } } +#[allow(dead_code)] mod uefi_command_internal { use r_efi::protocols::{loaded_image, simple_text_output}; @@ -337,26 +312,20 @@ mod uefi_command_internal { use crate::os::uefi::ffi::{OsStrExt, OsStringExt}; use crate::ptr::NonNull; use crate::slice; + use crate::sys::pal::uefi::helpers::OwnedTable; use crate::sys_common::wstr::WStrUnits; - pub struct Command { + pub struct Image { handle: NonNull, - stdout: Option>, - stderr: Option>, - st: Box, + stdout: Option>, + stderr: Option>, + st: OwnedTable, args: Option>, } - impl Command { - const fn new( - handle: NonNull, - st: Box, - ) -> Self { - Self { handle, stdout: None, stderr: None, st, args: None } - } - + impl Image { pub fn load_image(p: &OsStr) -> io::Result { - let mut path = helpers::DevicePath::from_text(p)?; + let path = helpers::DevicePath::from_text(p)?; let boot_services: NonNull = boot_services() .ok_or_else(|| const_io_error!(io::ErrorKind::NotFound, "Boot Services not found"))? .cast(); @@ -367,7 +336,7 @@ mod uefi_command_internal { ((*boot_services.as_ptr()).load_image)( r_efi::efi::Boolean::FALSE, image_handle.as_ptr(), - path.as_mut(), + path.as_ptr(), crate::ptr::null_mut(), 0, child_handle.as_mut_ptr(), @@ -382,69 +351,93 @@ mod uefi_command_internal { let loaded_image: NonNull = helpers::open_protocol(child_handle, loaded_image::PROTOCOL_GUID).unwrap(); - let mut st: Box = - Box::new(unsafe { crate::ptr::read((*loaded_image.as_ptr()).system_table) }); + let st = OwnedTable::from_table(unsafe { (*loaded_image.as_ptr()).system_table }); - unsafe { - (*loaded_image.as_ptr()).system_table = st.as_mut(); - } - - Ok(Self::new(child_handle, st)) + Ok(Self { handle: child_handle, stdout: None, stderr: None, st, args: None }) } } - pub fn start_image(&self) -> io::Result { + pub fn start_image(&mut self) -> io::Result { + self.update_st_crc32()?; + + // Use our system table instead of the default one + let loaded_image: NonNull = + helpers::open_protocol(self.handle, loaded_image::PROTOCOL_GUID).unwrap(); + unsafe { + (*loaded_image.as_ptr()).system_table = self.st.as_mut_ptr(); + } + let boot_services: NonNull = boot_services() .ok_or_else(|| const_io_error!(io::ErrorKind::NotFound, "Boot Services not found"))? .cast(); - let mut exit_data_size: MaybeUninit = MaybeUninit::uninit(); + let mut exit_data_size: usize = 0; let mut exit_data: MaybeUninit<*mut u16> = MaybeUninit::uninit(); let r = unsafe { ((*boot_services.as_ptr()).start_image)( self.handle.as_ptr(), - exit_data_size.as_mut_ptr(), + &mut exit_data_size, exit_data.as_mut_ptr(), ) }; // Drop exitdata - unsafe { - exit_data_size.assume_init_drop(); - exit_data.assume_init_drop(); + if exit_data_size != 0 { + unsafe { + let exit_data = exit_data.assume_init(); + ((*boot_services.as_ptr()).free_pool)(exit_data as *mut crate::ffi::c_void); + } } Ok(r) } - pub fn stdout_init(&mut self, mut protocol: helpers::Protocol) { - self.st.console_out_handle = protocol.handle().as_ptr(); - self.st.con_out = - protocol.as_mut() as *mut PipeProtocol as *mut simple_text_output::Protocol; + fn set_stdout( + &mut self, + handle: r_efi::efi::Handle, + protocol: *mut simple_text_output::Protocol, + ) { + unsafe { + (*self.st.as_mut_ptr()).console_out_handle = handle; + (*self.st.as_mut_ptr()).con_out = protocol; + } + } + + fn set_stderr( + &mut self, + handle: r_efi::efi::Handle, + protocol: *mut simple_text_output::Protocol, + ) { + unsafe { + (*self.st.as_mut_ptr()).standard_error_handle = handle; + (*self.st.as_mut_ptr()).std_err = protocol; + } + } + pub fn stdout_init(&mut self, protocol: helpers::OwnedProtocol) { + self.set_stdout( + protocol.handle().as_ptr(), + protocol.as_ref() as *const PipeProtocol as *mut simple_text_output::Protocol, + ); self.stdout = Some(protocol); } pub fn stdout_inherit(&mut self) { let st: NonNull = system_table().cast(); - - self.st.console_out_handle = unsafe { (*st.as_ptr()).console_out_handle }; - self.st.con_out = unsafe { (*st.as_ptr()).con_out }; + unsafe { self.set_stdout((*st.as_ptr()).console_out_handle, (*st.as_ptr()).con_out) } } - pub fn stderr_init(&mut self, mut protocol: helpers::Protocol) { - self.st.standard_error_handle = protocol.handle().as_ptr(); - self.st.std_err = - protocol.as_mut() as *mut PipeProtocol as *mut simple_text_output::Protocol; - + pub fn stderr_init(&mut self, protocol: helpers::OwnedProtocol) { + self.set_stderr( + protocol.handle().as_ptr(), + protocol.as_ref() as *const PipeProtocol as *mut simple_text_output::Protocol, + ); self.stderr = Some(protocol); } pub fn stderr_inherit(&mut self) { let st: NonNull = system_table().cast(); - - self.st.standard_error_handle = unsafe { (*st.as_ptr()).standard_error_handle }; - self.st.std_err = unsafe { (*st.as_ptr()).std_err }; + unsafe { self.set_stderr((*st.as_ptr()).standard_error_handle, (*st.as_ptr()).std_err) } } pub fn stderr(&self) -> io::Result> { @@ -476,9 +469,37 @@ mod uefi_command_internal { self.args = Some(args); } + + fn update_st_crc32(&mut self) -> io::Result<()> { + let bt: NonNull = boot_services().unwrap().cast(); + let st_size = unsafe { (*self.st.as_ptr()).hdr.header_size as usize }; + let mut crc32: u32 = 0; + + // Set crc to 0 before calcuation + unsafe { + (*self.st.as_mut_ptr()).hdr.crc32 = 0; + } + + let r = unsafe { + ((*bt.as_ptr()).calculate_crc32)( + self.st.as_mut_ptr() as *mut crate::ffi::c_void, + st_size, + &mut crc32, + ) + }; + + if r.is_error() { + Err(io::Error::from_raw_os_error(r.as_usize())) + } else { + unsafe { + (*self.st.as_mut_ptr()).hdr.crc32 = crc32; + } + Ok(()) + } + } } - impl Drop for Command { + impl Drop for Image { fn drop(&mut self) { if let Some(bt) = boot_services() { let bt: NonNull = bt.cast(); @@ -501,13 +522,12 @@ mod uefi_command_internal { set_cursor_position: simple_text_output::ProtocolSetCursorPosition, enable_cursor: simple_text_output::ProtocolEnableCursor, mode: *mut simple_text_output::Mode, - _mode: Box, _buffer: Vec, } impl PipeProtocol { pub fn new() -> Self { - let mut mode = Box::new(simple_text_output::Mode { + let mode = Box::new(simple_text_output::Mode { max_mode: 0, mode: 0, attribute: 0, @@ -525,14 +545,13 @@ mod uefi_command_internal { clear_screen: Self::clear_screen, set_cursor_position: Self::set_cursor_position, enable_cursor: Self::enable_cursor, - mode: mode.as_mut(), - _mode: mode, + mode: Box::into_raw(mode), _buffer: Vec::new(), } } pub fn null() -> Self { - let mut mode = Box::new(simple_text_output::Mode { + let mode = Box::new(simple_text_output::Mode { max_mode: 0, mode: 0, attribute: 0, @@ -550,8 +569,7 @@ mod uefi_command_internal { clear_screen: Self::clear_screen, set_cursor_position: Self::set_cursor_position, enable_cursor: Self::enable_cursor, - mode: mode.as_mut(), - _mode: mode, + mode: Box::into_raw(mode), _buffer: Vec::new(), } } @@ -660,4 +678,12 @@ mod uefi_command_internal { r_efi::efi::Status::UNSUPPORTED } } + + impl Drop for PipeProtocol { + fn drop(&mut self) { + unsafe { + let _ = Box::from_raw(self.mode); + } + } + } }