Merge "Add CStr conversion to gbl::Slots::Suffix" into main am: 31262f8662

Original change: https://android-review.googlesource.com/c/platform/bootable/libbootloader/+/2944526

Change-Id: I3ab406601e8d63b2edc983385739eb33bae2ad7a
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

3 files changed, 120 insertions, 10 deletions

diff --git a/gbl/libgbl/src/error.rs b/gbl/libgbl/src/error.rs
index ffcb920..5855d50 100644
--- a/gbl/libgbl/src/error.rs
+++ b/gbl/libgbl/src/error.rs
@@ -15,7 +15,7 @@
 //! Error types used in libgbl.
 
 use avb::{DescriptorError, SlotVerifyError};
-use core::ffi::FromBytesWithNulError;
+use core::ffi::{FromBytesUntilNulError, FromBytesWithNulError};
 use core::fmt::{Debug, Display, Formatter};
 
 /// Helper type GBL functions will return.
@@ -77,6 +77,12 @@ impl<'a> From<SlotVerifyError<'a>> for Error {
     }
 }
 
+impl From<FromBytesUntilNulError> for Error {
+    fn from(e: FromBytesUntilNulError) -> Self {
+        Error::Internal
+    }
+}
+
 impl From<FromBytesWithNulError> for Error {
     fn from(e: FromBytesWithNulError) -> Self {
         Error::Internal
diff --git a/gbl/libgbl/src/lib.rs b/gbl/libgbl/src/lib.rs
index db97cb4..41d2483 100644
--- a/gbl/libgbl/src/lib.rs
+++ b/gbl/libgbl/src/lib.rs
@@ -50,7 +50,7 @@ pub mod ops;
 /// querying and modifying slotted boot behavior.
 pub mod slots;
 
-use slots::{BootTarget, BootToken, Cursor, OneShot};
+use slots::{BootTarget, BootToken, Cursor, OneShot, SuffixBytes, UnbootableReason};
 
 #[cfg(feature = "sw_digest")]
 pub mod sw_digest;
@@ -262,6 +262,7 @@ where
         partitions_ram_map: &mut [PartitionRamMap],
         avb_verification_flags: AvbVerificationFlags,
         boot_target: Option<BootTarget>,
+        slot_cursor: &mut Cursor,
     ) -> Result<VerifiedData<'b>>
     where
         'a: 'b,
@@ -272,8 +273,12 @@ where
         let default_vendor_boot_image_buffer = VENDOR_BOOT_IMAGE.lock();
         let default_init_boot_image_buffer = INIT_BOOT_IMAGE.lock();
 
+        let bytes: SuffixBytes =
+            if let Some(tgt) = boot_target { tgt.suffix().into() } else { Default::default() };
+
         let requested_partitions = [CStr::from_bytes_with_nul(b"\0")?];
-        let avb_suffix = CStr::from_bytes_with_nul(b"\0")?;
+        let avb_suffix = CStr::from_bytes_until_nul(&bytes)?;
+
         let verified_data = VerifiedData(avb::slot_verify(
             avb_ops,
             &requested_partitions,
@@ -423,6 +428,12 @@ where
     /// Wrapper around the above functions for devices that don't need custom behavior between each
     /// step
     ///
+    /// Warning: If the call to load_verify_boot fails, the device MUST
+    ///          be restarted in order to make forward boot progress.
+    ///          Callers MAY log the error, enter an interactive mode,
+    ///          or take other actions before rebooting.
+    ///
+    ///
     /// # Arguments
     ///   * `avb_ops` - implementation for `avb::Ops` that would be borrowed in result to prevent
     ///   changes to partitions until it is out of scope.
@@ -466,6 +477,25 @@ where
         self.kernel_jump(kernel_image, ramdisk, dtb, token)
     }
 
+    fn is_unrecoverable_error(error: &Error) -> bool {
+        // Note: these ifs are nested instead of chained because multiple
+        //       expressions in an if-let is an unstable features
+        if let Error::AvbSlotVerifyError(ref avb_error) = error {
+            // These are the AVB errors that are not recoverable on a subsequent attempt.
+            // If necessary in the future, this helper function can be moved to the GblOps trait
+            // and customized for platform specific behavior.
+            if matches!(
+                avb_error,
+                avb::SlotVerifyError::Verification(_)
+                    | avb::SlotVerifyError::PublicKeyRejected
+                    | avb::SlotVerifyError::RollbackIndex
+            ) {
+                return true;
+            }
+        }
+        false
+    }
+
     fn lvb_inner<'b: 'c, 'c, 'd: 'b, 'e>(
         &mut self,
         avb_ops: &'b mut impl avb::Ops,
@@ -473,7 +503,7 @@ where
         kernel_load_buffer: &'e mut MutexGuard<&'static mut [u8]>,
         partitions_ram_map: &'d mut [PartitionRamMap<'b, 'c>],
         avb_verification_flags: AvbVerificationFlags,
-        slot_cursor: Cursor,
+        mut slot_cursor: Cursor,
     ) -> Result<(KernelImage<'e>, BootToken)> {
         let mut oneshot_status = slot_cursor.ctx.get_oneshot_status();
         slot_cursor.ctx.clear_oneshot_status();
@@ -491,12 +521,33 @@ where
             Some(OneShot::Continue(target)) => target,
         };
 
-        let mut verify_data = self.load_and_verify_image(
-            avb_ops,
-            partitions_ram_map,
-            AvbVerificationFlags(0),
-            Some(boot_target),
-        )?;
+        let mut verify_data = self
+            .load_and_verify_image(
+                avb_ops,
+                partitions_ram_map,
+                AvbVerificationFlags(0),
+                Some(boot_target),
+                &mut slot_cursor,
+            )
+            .map_err(|e: Error| {
+                if let BootTarget::NormalBoot(slot) = boot_target {
+                    if Self::is_unrecoverable_error(&e) {
+                        let _ = slot_cursor.ctx.set_slot_unbootable(
+                            slot.suffix,
+                            UnbootableReason::VerificationFailure,
+                        );
+                    } else {
+                        // Note: the call to mark_boot_attempt will fail if any of the following occur:
+                        // * the target was already Unbootable before the call to load_and_verify_image
+                        // * policy, I/O, or other errors in mark_boot_attempt
+                        //
+                        // We don't really care about those circumstances.
+                        // The call here is a best effort attempt to decrement tries remaining.
+                        let _ = slot_cursor.ctx.mark_boot_attempt(boot_target);
+                    }
+                }
+                e
+            })?;
 
         let (boot_image, init_boot_image, vendor_boot_image, _) =
             get_images(&mut verify_data, partitions_ram_map);
@@ -546,6 +597,7 @@ mod tests {
     use avb::PublicKeyForPartitionInfo;
     use avb_test::TestOps;
     use itertools::Itertools;
+    use slots::fuchsia::SlotBlock;
     use std::fs;
 
     pub fn get_end(buf: &[u8]) -> Option<*const u8> {
@@ -629,11 +681,13 @@ mod tests {
         let mut avb_ops = AvbOpsUnimplemented {};
         let mut partitions_ram_map: [PartitionRamMap; 0] = [];
         let avb_verification_flags = AvbVerificationFlags(0);
+        let mut slot_cursor = Cursor { ctx: &mut SlotBlock::default() };
         let res = gbl.load_and_verify_image(
             &mut avb_ops,
             &mut partitions_ram_map,
             avb_verification_flags,
             None,
+            &mut slot_cursor,
         );
         assert_eq!(res.unwrap_err(), Error::AvbSlotVerifyError(avb::SlotVerifyError::Io));
     }
@@ -649,6 +703,8 @@ mod tests {
     fn test_load_and_verify_image_stub() {
         let mut gbl = Gbl::new(DefaultGblOps::new());
         let mut avb_ops = TestOps::default();
+        let mut slot_cursor = Cursor { ctx: &mut SlotBlock::default() };
+
         avb_ops.add_partition(TEST_PARTITION_NAME, fs::read(TEST_IMAGE_PATH).unwrap());
         avb_ops.add_partition("vbmeta", fs::read(TEST_VBMETA_PATH).unwrap());
         avb_ops.add_vbmeta_key(fs::read(TEST_PUBLIC_KEY_PATH).unwrap(), None, true);
@@ -662,6 +718,7 @@ mod tests {
             &mut partitions_ram_map,
             avb_verification_flags,
             None,
+            &mut slot_cursor,
         );
         assert!(res.is_ok());
     }
diff --git a/gbl/libgbl/src/slots.rs b/gbl/libgbl/src/slots.rs
index 2456b38..87198b4 100644
--- a/gbl/libgbl/src/slots.rs
+++ b/gbl/libgbl/src/slots.rs
@@ -15,6 +15,8 @@
 /// Export the default implementation
 pub mod fuchsia;
 
+use core::mem::size_of;
+
 /// A type safe container for describing the number of retries a slot has left
 /// before it becomes unbootable.
 /// Slot tries can only be compared to, assigned to, or assigned from other
@@ -60,6 +62,25 @@ impl From<char> for Suffix {
     }
 }
 
+// Includes a null terminator
+const SUFFIX_CSTR_MAX_BYTES: usize = size_of::<Suffix>() + 1;
+
+/// A buffer large enough to contain the serialized representation of a Suffix.
+/// Can be turned into a &Cstr like so:
+///
+/// let suffix: Suffix = 'a'.into();
+/// let buffer: SuffixBytes = suffix.into();
+/// let cstr = CStr::from_bytes_until_nul(&buffer)?;
+pub type SuffixBytes = [u8; SUFFIX_CSTR_MAX_BYTES];
+
+impl From<Suffix> for SuffixBytes {
+    fn from(val: Suffix) -> Self {
+        let mut buffer: Self = Default::default();
+        let _ = val.0.encode_utf8(&mut buffer);
+        buffer
+    }
+}
+
 /// Slot metadata describing why that slot is unbootable.
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub enum UnbootableReason {
@@ -371,3 +392,29 @@ impl<'a> Drop for Cursor<'a> {
         self.ctx.write_back();
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use core::ffi::CStr;
+
+    #[test]
+    fn test_suffix_to_cstr() {
+        let normal: Suffix = 'a'.into();
+        let normal_buffer: SuffixBytes = normal.into();
+        let normal_cstr = CStr::from_bytes_until_nul(&normal_buffer);
+        assert!(normal_cstr.is_ok());
+
+        // All UTF-8 characters are at most 4 bytes.
+        // The in-memory representation as a chr or Suffix
+        // uses all 4 bytes regardless of the length of the serialized
+        // representation, but we need to make sure that buffer for
+        // the serialized suffix can handle that too.
+        // All emoji are 4 bytes when encoded as UTF-8,
+        // so they're a reasonable test.
+        let squid: Suffix = '🦑'.into();
+        let squid_buffer: SuffixBytes = squid.into();
+        let squid_cstr = CStr::from_bytes_until_nul(&squid_buffer);
+        assert!(squid_cstr.is_ok());
+    }
+}