From a132340d251389bf3cbce0863e632d003424f5e2 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 Signed-off-by: Ayush Singh --- library/std/src/sys/pal/uefi/helpers.rs | 75 +++++++--- library/std/src/sys/pal/uefi/process.rs | 189 +++++++++++++++--------- 2 files changed, 173 insertions(+), 91 deletions(-) diff --git a/library/std/src/sys/pal/uefi/helpers.rs b/library/std/src/sys/pal/uefi/helpers.rs index c2419bbb58573..e0eefc9660106 100644 --- a/library/std/src/sys/pal/uefi/helpers.rs +++ b/library/std/src/sys/pal/uefi/helpers.rs @@ -292,25 +292,17 @@ 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 } - } - +impl OwnedProtocol { pub(crate) fn create(protocol: T, mut guid: r_efi::efi::Guid) -> io::Result { let boot_services: 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(); let r = unsafe { @@ -318,7 +310,7 @@ impl Protocol { &mut handle, &mut guid, r_efi::efi::NATIVE_INTERFACE, - protocol.as_mut() as *mut T as *mut crate::ffi::c_void, + protocol as *mut crate::ffi::c_void, ) }; @@ -329,7 +321,7 @@ impl Protocol { 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,7 +329,7 @@ impl Protocol { } } -impl Drop for Protocol { +impl Drop for OwnedProtocol { fn drop(&mut self) { if let Some(bt) = boot_services() { let bt: NonNull = bt.cast(); @@ -345,21 +337,62 @@ impl Drop for Protocol { ((*bt.as_ptr()).uninstall_protocol_interface)( self.handle.as_ptr(), &mut self.guid, - self.protocol.as_mut() as *mut T as *mut crate::ffi::c_void, + self.protocol as *mut crate::ffi::c_void, ) }; } + + 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 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 AsMut for Protocol { - fn as_mut(&mut self) -> &mut T { - &mut self.protocol +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..0d4a60243d3b4 100644 --- a/library/std/src/sys/pal/uefi/process.rs +++ b/library/std/src/sys/pal/uefi/process.rs @@ -35,6 +35,7 @@ pub struct StdioPipes { pub stderr: Option, } +#[derive(Copy, Clone)] pub enum Stdio { Inherit, Null, @@ -96,14 +97,14 @@ impl Command { fn create_pipe( s: Stdio, - ) -> io::Result>> { + ) -> io::Result>> { match s { - Stdio::MakePipe => helpers::Protocol::create( + Stdio::MakePipe => helpers::OwnedProtocol::create( uefi_command_internal::PipeProtocol::new(), simple_text_output::PROTOCOL_GUID, ) .map(Some), - Stdio::Null => helpers::Protocol::create( + Stdio::Null => helpers::OwnedProtocol::create( uefi_command_internal::PipeProtocol::null(), simple_text_output::PROTOCOL_GUID, ) @@ -113,39 +114,27 @@ impl Command { } pub fn output(&mut self) -> io::Result<(ExitStatus, Vec, Vec)> { - let mut cmd = uefi_command_internal::Command::load_image(&self.prog)?; + let mut cmd = uefi_command_internal::Image::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 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(), + 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 */ + // 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(" "); @@ -339,18 +328,18 @@ mod uefi_command_internal { use crate::slice; 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: helpers::OwnedTable, args: Option>, } - impl Command { + impl Image { const fn new( handle: NonNull, - st: Box, + st: helpers::OwnedTable, ) -> Self { Self { handle, stdout: None, stderr: None, st, args: None } } @@ -382,69 +371,96 @@ 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) }); - - unsafe { - (*loaded_image.as_ptr()).system_table = st.as_mut(); - } + let st: helpers::OwnedTable = + helpers::OwnedTable::from_table(unsafe { + (*loaded_image.as_ptr()).system_table + }); Ok(Self::new(child_handle, st)) } } - 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 +492,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 +545,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 +568,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 +592,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 +701,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); + } + } + } }