feedback

Cargo.toml

@@ -30,6 +30,7 @@ rand = "0.8.5"
 # workspace build dependencies have to be defined here
 libbpf-cargo = { version = "0.25.0-beta.0" }
 glob = "0.3.1"
+ring = "0.17.8"
 
 [dependencies]
 gimli = "0.31.1"
@@ -62,6 +63,7 @@ errno = { workspace = true }
 procfs = { workspace = true }
 nix = { workspace = true, features = ["user"] }
 parking_lot = { version = "0.12.3", features = ["deadlock_detection"] }
+ring = { workspace = true }
 
 [dev-dependencies]
 assert_cmd = { version = "2.0.16" }

lightswitch-object/Cargo.toml

@@ -8,7 +8,7 @@ repository = "https://github.com/javierhonduco/lightswitch"
 
 [dependencies]
 data-encoding = "2.6.0"
-ring = "0.17.8"
+ring = { workspace = true }
 memmap2 = { workspace = true }
 object = { workspace = true }
 anyhow = { workspace = true }

src/unwind_info/pages.rs

@@ -130,7 +130,8 @@ mod tests {
             let search_here = &unwind_info[(found.index as usize)..(found.len as usize)];
             let found_row = search_here.iter().find(|el| el.pc == pc).unwrap();
             // And that the high and low bits were done ok
-            assert_eq!((found_row.pc & low_bits_mask) + pc_high, found_row.pc);
+            let pc = found_row.pc;
+            assert_eq!((pc & low_bits_mask) + pc_high, pc);
         }
     }
 }

src/unwind_info/persist.rs

@@ -1,5 +1,9 @@
 #![allow(dead_code)]
 use plain::Plain;
+use ring::digest::{Context, SHA256};
+use std::io::Read;
+use std::io::Seek;
+use std::io::SeekFrom;
 use std::io::Write;
 use std::path::Path;
 use std::path::PathBuf;
@@ -9,18 +13,30 @@ use crate::unwind_info::types::CompactUnwindRow;
 
 // To identify this binary file type.
 const MAGIC_NUMBER: u32 = 0x1357531;
-// Any changes to the ABI must bump the version.
+// Any changes to the ABI / digest must bump the version.
 const VERSION: u32 = 1;
 
+type UnwindInformationDigest = u64;
+
 #[derive(Debug, Default)]
-#[repr(C)]
+#[repr(C, packed)]
 struct Header {
     magic: u32,
     version: u32,
-    unwind_info_len: usize,
+    // To ensure that the unwind information we are reading is not
+    // corrupted in any way we compute a hash of the unwind information
+    // that is checked on the read path.
+    unwind_info_digest: UnwindInformationDigest,
+    unwind_info_len: u64,
 }
 
+/// SAFETY: Using packed C representation, which plain needs, and there is
+/// the extra safety layer of the unwind information digest checked in the
+/// read path, in case the data is corrupted.
 unsafe impl Plain for Header {}
+/// SAFETY: Using packed C representation, which plain needs, and there is
+/// the extra safety layer of the unwind information digest checked in the
+/// read path, in case the data is corrupted.
 unsafe impl Plain for CompactUnwindRow {}
 
 /// Writes compact information to a given writer.
@@ -35,23 +51,32 @@ impl Writer {
         }
     }
 
-    fn write(self, writer: &mut impl Write) -> anyhow::Result<()> {
+    fn write<W: Write + Seek>(self, writer: &mut W) -> anyhow::Result<()> {
         let unwind_info = self.read_unwind_info()?;
-        self.write_header(writer, unwind_info.len())?;
-        self.write_unwind_info(writer, &unwind_info)?;
-
+        // Write dummy header.
+        self.write_header(writer, 0, None)?;
+        let digest = self.write_unwind_info(writer, &unwind_info)?;
+        // Write real header.
+        writer.seek(SeekFrom::Start(0))?;
+        self.write_header(writer, unwind_info.len(), Some(digest))?;
         Ok(())
     }
 
     fn read_unwind_info(&self) -> anyhow::Result<Vec<CompactUnwindRow>> {
         compact_unwind_info(&self.executable_path.to_string_lossy())
     }
 
-    fn write_header(&self, writer: &mut impl Write, unwind_info_len: usize) -> anyhow::Result<()> {
+    fn write_header(
+        &self,
+        writer: &mut impl Write,
+        unwind_info_len: usize,
+        digest: Option<UnwindInformationDigest>,
+    ) -> anyhow::Result<()> {
         let header = Header {
             magic: MAGIC_NUMBER,
             version: VERSION,
-            unwind_info_len,
+            unwind_info_digest: digest.unwrap_or(0),
+            unwind_info_len: unwind_info_len.try_into()?,
         };
         writer.write_all(unsafe { plain::as_bytes(&header) })?;
         Ok(())
@@ -61,12 +86,19 @@ impl Writer {
         &self,
         writer: &mut impl Write,
         unwind_info: &[CompactUnwindRow],
-    ) -> anyhow::Result<()> {
+    ) -> anyhow::Result<UnwindInformationDigest> {
+        let mut context = Context::new(&SHA256);
+
         for unwind_row in unwind_info {
-            writer.write_all(unsafe { plain::as_bytes(unwind_row) })?;
+            let unwind_row_data = unsafe { plain::as_bytes(unwind_row) };
+            context.update(unwind_row_data);
+            writer.write_all(unwind_row_data)?;
         }
 
-        Ok(())
+        let mut buffer = [0; 8];
+        let _ = context.finish().as_ref().read(&mut buffer)?;
+
+        Ok(u64::from_ne_bytes(buffer))
     }
 }
 
@@ -76,10 +108,14 @@ pub enum ReaderError {
     MagicNumber,
     #[error("version is not compatible")]
     Version,
-    #[error("error parsing raw bytes: {0}")]
-    ParsingError(String),
+    #[error("generic error: {0}")]
+    Generic(String),
     #[error("index out of range")]
     OutOfRange,
+    #[error("could not convert between types")]
+    SizeConversion,
+    #[error("digest does not match")]
+    Digest,
 }
 
 /// Reads compact information of a bytes slice.
@@ -99,7 +135,7 @@ impl<'a> Reader<'a> {
         let mut header = Header::default();
         let header_data = data.get(0..header_size).ok_or(ReaderError::OutOfRange)?;
         plain::copy_from_bytes(&mut header, header_data)
-            .map_err(|e| ReaderError::ParsingError(format!("{:?}", e)))?;
+            .map_err(|e| ReaderError::Generic(format!("{:?}", e)))?;
 
         if header.magic != MAGIC_NUMBER {
             return Err(ReaderError::MagicNumber);
@@ -115,41 +151,59 @@ impl<'a> Reader<'a> {
     pub fn unwind_info(self) -> Result<Vec<CompactUnwindRow>, ReaderError> {
         let header_size = std::mem::size_of::<Header>();
         let unwind_row_size = std::mem::size_of::<CompactUnwindRow>();
-        let unwind_info_len = self.header.unwind_info_len;
+        let unwind_info_len: usize = self
+            .header
+            .unwind_info_len
+            .try_into()
+            .map_err(|_| ReaderError::SizeConversion)?;
 
         let mut unwind_info = Vec::with_capacity(unwind_info_len);
         let mut unwind_row = CompactUnwindRow::default();
 
         let unwind_info_data = &self.data[header_size..];
+        let mut context = Context::new(&SHA256);
         for i in 0..unwind_info_len {
             let step = i * unwind_row_size;
             let unwind_row_data = unwind_info_data
                 .get(step..step + unwind_row_size)
                 .ok_or(ReaderError::OutOfRange)?;
+            context.update(unwind_row_data);
             plain::copy_from_bytes(&mut unwind_row, unwind_row_data)
-                .map_err(|e| ReaderError::ParsingError(format!("{:?}", e)))?;
+                .map_err(|e| ReaderError::Generic(format!("{:?}", e)))?;
             unwind_info.push(unwind_row);
         }
 
+        let mut buffer = [0; 8];
+        let _ = context
+            .finish()
+            .as_ref()
+            .read(&mut buffer)
+            .map_err(|e| ReaderError::Generic(e.to_string()));
+        let digest = u64::from_ne_bytes(buffer);
+
+        if self.header.unwind_info_digest != digest {
+            return Err(ReaderError::Digest);
+        }
+
         Ok(unwind_info)
     }
 }
 
 #[cfg(test)]
 mod tests {
+    use std::io::Cursor;
     use std::path::PathBuf;
 
     use super::*;
 
     #[test]
     fn test_write_and_read_unwind_info() {
-        let mut buffer = Vec::new();
+        let mut buffer = Cursor::new(Vec::new());
         let path = PathBuf::from("/proc/self/exe");
         let writer = Writer::new(&path);
         assert!(writer.write(&mut buffer).is_ok());
 
-        let reader = Reader::new(&buffer);
-        assert!(reader.is_ok());
+        let reader = Reader::new(&buffer.get_ref()[..]);
         let unwind_info = reader.unwrap().unwind_info();
         assert!(unwind_info.is_ok());
         let unwind_info = unwind_info.unwrap();
@@ -163,7 +217,9 @@ mod tests {
             magic: 0xBAD,
             ..Default::default()
         };
-        buffer.write_all(unsafe { plain::as_bytes(&header) }).unwrap();
+        buffer
+            .write_all(unsafe { plain::as_bytes(&header) })
+            .unwrap();
         assert!(matches!(
             Reader::new(&buffer),
             Err(ReaderError::MagicNumber)
@@ -178,10 +234,28 @@ mod tests {
             magic: MAGIC_NUMBER,
             ..Default::default()
         };
-        buffer.write_all(unsafe { plain::as_bytes(&header) }).unwrap();
+        buffer
+            .write_all(unsafe { plain::as_bytes(&header) })
+            .unwrap();
         assert!(matches!(Reader::new(&buffer), Err(ReaderError::Version)));
     }
 
+    #[test]
+    fn test_corrupt_unwind_info() {
+        let mut buffer: Cursor<Vec<u8>> = Cursor::new(Vec::new());
+        let path = PathBuf::from("/proc/self/exe");
+        let writer = Writer::new(&path);
+        assert!(writer.write(&mut buffer).is_ok());
+
+        // Corrupt unwind info.
+        buffer.seek(SeekFrom::End(-10)).unwrap();
+        buffer.write_all(&[0, 0, 0, 0, 0, 0, 0]).unwrap();
+
+        let reader = Reader::new(&buffer.get_ref()[..]);
+        let unwind_info = reader.unwrap().unwind_info();
+        assert!(matches!(unwind_info, Err(ReaderError::Digest)));
+    }
+
     #[test]
     fn test_header_too_small() {
         let buffer = Vec::new();
@@ -195,8 +269,11 @@ mod tests {
             version: VERSION,
             magic: MAGIC_NUMBER,
             unwind_info_len: 4,
+            unwind_info_digest: 0x0,
         };
-        buffer.write_all(unsafe { plain::as_bytes(&header) }).unwrap();
+        buffer
+            .write_all(unsafe { plain::as_bytes(&header) })
+            .unwrap();
         assert!(matches!(
             Reader::new(&buffer).unwrap().unwind_info(),
             Err(ReaderError::OutOfRange)

src/unwind_info/types.rs

@@ -1,5 +1,8 @@
 use lazy_static::lazy_static;
 
+// Important: Any changes to the structures below must bump the file
+// version in unwind_info/persist.rs
+
 #[repr(u8)]
 #[derive(Debug, Default, Copy, Clone, PartialEq)]
 pub enum CfaType {
@@ -33,7 +36,7 @@ pub enum PltType {
 }
 
 #[derive(Debug, Default, Copy, Clone, PartialEq)]
-#[repr(C)]
+#[repr(C, packed)]
 pub struct CompactUnwindRow {
     pub pc: u64,
     pub cfa_type: CfaType,