uefi: Fix unaligned memcpy in ConfigurationString parser

Author: seijikun

uefi/src/mem/aligned_buffer.rs

@@ -2,8 +2,8 @@
 
 use alloc::alloc::{Layout, LayoutError, alloc, dealloc};
 use core::error::Error;
-use core::fmt;
 use core::ptr::NonNull;
+use core::{fmt, slice};
 
 /// Helper class to maintain the lifetime of a memory region allocated with a non-standard alignment.
 /// Facilitates RAII to properly deallocate when lifetime of the object ends.
@@ -51,6 +51,18 @@ impl AlignedBuffer {
         self.ptr.as_ptr()
     }
 
+    /// Get the underlying memory region as immutable slice.
+    #[must_use]
+    pub const fn as_slice(&mut self) -> &[u8] {
+        unsafe { slice::from_raw_parts(self.ptr(), self.size()) }
+    }
+
+    /// Get the underlying memory region as mutable slice.
+    #[must_use]
+    pub const fn as_slice_mut(&mut self) -> &mut [u8] {
+        unsafe { slice::from_raw_parts_mut(self.ptr_mut(), self.size()) }
+    }
+
     /// Get the size of the aligned memory region managed by this instance.
     #[must_use]
     pub const fn size(&self) -> usize {
@@ -67,6 +79,14 @@ impl AlignedBuffer {
         }
     }
 
+    /// Fill the aligned memory region with data from the given iterator.
+    /// If the given iterator is shorter than the buffer, the remaining area will be left untouched.
+    pub fn copy_from_iter(&mut self, src: impl Iterator<Item = u8>) {
+        src.take(self.size())
+            .zip(self.as_slice_mut().iter_mut())
+            .for_each(|(src, dst)| *dst = src);
+    }
+
     /// Check the buffer's alignment against the `required_alignment`.
     pub fn check_alignment(&self, required_alignment: usize) -> Result<(), AlignmentError> {
         //TODO: use bfr.addr() when it's available

uefi/src/proto/hii/config_str.rs

@@ -9,6 +9,7 @@ use core::slice;
 use core::str::{self, FromStr};
 use uguid::Guid;
 
+use crate::mem::AlignedBuffer;
 use crate::proto::device_path::DevicePath;
 use crate::{CStr16, Char16};
 
@@ -169,11 +170,15 @@ impl ConfigurationString {
         if data.len() % 2 != 0 {
             return None;
         }
-        let mut data: Vec<_> = Self::parse_bytes_from_hex(data).collect();
-        data.chunks_exact_mut(2).for_each(|c| c.swap(0, 1));
-        data.extend_from_slice(&[0, 0]);
+        let size_bytes = data.len() / 2 + 2; // includes \0 terminator
+        let size_chars = size_bytes / 2;
+        let mut bfr = AlignedBuffer::from_size_align(size_bytes, 2).ok()?;
+        bfr.copy_from_iter(Self::parse_bytes_from_hex(data).chain([0, 0]));
+        bfr.as_slice_mut()
+            .chunks_exact_mut(2)
+            .for_each(|c| c.swap(0, 1));
         let data: &[Char16] =
-            unsafe { slice::from_raw_parts(data.as_slice().as_ptr().cast(), data.len() / 2) };
+            unsafe { slice::from_raw_parts(bfr.as_slice().as_ptr().cast(), size_chars) };
         Some(CStr16::from_char16_with_nul(data).ok()?.to_string())
     }
 
@@ -280,4 +285,4 @@ mod tests {
         assert_eq!(parsed.elements[10].width, 1);
         assert_eq!(&parsed.elements[10].value, &[0x00]);
     }
-}
+}