Use BOOTED_IMAGE to warn when using wrong A/B image

cmd/probe/src/lib.rs

@@ -319,7 +319,7 @@ fn probecmd(subargs: ProbeArgs, context: &mut ExecutionContext) -> Result<()> {
     }
 
     let regions = if let Some(hubris) = &hubris {
-        match hubris.validate(core, HubrisValidate::ArchiveMatch) {
+        match hubris.validate(core, HubrisValidate::ArchiveMatch, log) {
             Ok(_) => hubris.regions(core).unwrap(),
             Err(err) => {
                 //

cmd/readmem/src/lib.rs

@@ -188,7 +188,7 @@ fn readmem(subargs: ReadmemArgs, context: &mut ExecutionContext) -> Result<()> {
 
     if subargs.symbol {
         if let Some(hubris) = &hubris {
-            hubris.validate(core, HubrisValidate::ArchiveMatch)?;
+            hubris.validate(core, HubrisValidate::ArchiveMatch, log)?;
         } else {
             bail!("cannot specify `--symbol` without Hubris archive");
         }
@@ -198,7 +198,7 @@ fn readmem(subargs: ReadmemArgs, context: &mut ExecutionContext) -> Result<()> {
         Ok(addr) => addr,
         _ => {
             if let Some(hubris) = &hubris {
-                hubris.validate(core, HubrisValidate::ArchiveMatch)?;
+                hubris.validate(core, HubrisValidate::ArchiveMatch, log)?;
                 hubris.lookup_peripheral(&subargs.address)?
             } else {
                 bail!("cannot look up peripheral without archive");

cmd/test/src/lib.rs

@@ -165,7 +165,7 @@ fn test(subargs: TestArgs, context: &mut ExecutionContext) -> Result<()> {
     let log = context.log();
     let core = &mut *context.cli.attach_live_booted(hubris)?;
 
-    hubris.validate(core, HubrisValidate::Booted)?;
+    hubris.validate(core, HubrisValidate::Booted, log)?;
 
     // This type is &[(&str, &(dyn Fn() + Send + Sync))]
     let test_slice = hubris

humility-cli/src/lib.rs

@@ -193,7 +193,7 @@ impl Cli {
             let Some(hubris) = hubris else {
                 bail!("cannot validate without Hubris archive");
             };
-            hubris.validate(&mut *core, validate)?;
+            hubris.validate(&mut *core, validate, self.log())?;
         }
         Ok(core)
     }
@@ -269,7 +269,7 @@ impl Cli {
             let Some(hubris) = hubris else {
                 bail!("cannot validate without Hubris archive");
             };
-            hubris.validate(&mut *core, validate)?;
+            hubris.validate(&mut *core, validate, self.log())?;
         }
         Ok(core)
     }

humility-core/src/hubris.rs

@@ -2071,36 +2071,39 @@ impl HubrisArchive {
         &self,
         core: &mut dyn crate::core::Core,
         criteria: HubrisValidate,
+        log: &Logger,
     ) -> Result<()> {
-        let ntasks = self.ntasks();
         if core.is_net() || core.is_archive() {
             return Ok(());
         }
 
-        // To validate that what we're running on the target matches what
-        // we have in the archive, we are going to check the image ID, an
-        // identifer created for this purpose.
-        let addr = self.imageid.0;
-        let nbytes = self.imageid.1.len();
-        assert!(nbytes > 0);
-
-        let mut id = vec![0; nbytes];
-        core.read_8(addr, &mut id[0..nbytes]).with_context(|| {
-            format!("failed to read image ID at 0x{:x}; board mismatch?", addr)
-        })?;
-
-        let deltas = id
-            .iter()
-            .zip(self.imageid.1.iter())
-            .filter(|&(lhs, rhs)| lhs != rhs)
-            .count();
+        // To validate that what we're running on the target matches what we
+        // have in the archive, we are going to check the image ID, an identifer
+        // created for this purpose. First try to read the ID of the image that
+        // actually booted, as recorded by the kernel. If that fails then fall
+        // back to reading the ID from FLASH, which can be misleading on targets
+        // with A/B images. For example, if the archive is an A image, then
+        // `read_image_id_from_flash` will return the ID of whichever A image
+        // was last flashed regardless of whether the target is currently
+        // running an A or B image.
+        let id = match self.booted_image(core) {
+            Ok((id, _)) => id,
+            Err(err) => {
+                info!(
+                    log,
+                    "can't detect booted image, reading image ID from FLASH ({})",
+                    err
+                );
+                self.read_image_id_from_flash(core)?
+            }
+        };
 
-        if deltas > 0 || id.len() != self.imageid.1.len() {
+        if id != self.imageid.1 {
             bail!(
                     "image ID in archive ({:x?}) does not equal \
-                    ID at 0x{:x} ({:x?})",
-                    self.imageid.1, self.imageid.0, id,
-                );
+                    image ID of target ({:x?})",
+                    self.imageid.1, id,
+            );
         }
 
         if criteria == HubrisValidate::ArchiveMatch {
@@ -2111,6 +2114,7 @@ impl HubrisArchive {
             return Ok(());
         }
 
+        let ntasks = self.ntasks();
         let (_, n) = self.task_table(core)?;
 
         if n == ntasks as u32 {
@@ -2170,6 +2174,67 @@ impl HubrisArchive {
         );
     }
 
+    /// Reads the kernel's `BOOTED_IMAGE` array to determine which image is
+    /// actually running on the target.
+    ///
+    /// Returns the image ID and the image name.
+    pub fn booted_image(
+        &self,
+        core: &mut dyn crate::core::Core,
+    ) -> Result<(Vec<u8>, String)> {
+        const MAGIC: &[u8] = b"HUBRISID";
+        const ID_LEN: usize = 8;
+
+        // If BOOTED_IMAGE isn't in the symbol table, it's probably because the
+        // archive is too old.
+        let &(addr, size) =
+            self.esyms_byname.get("BOOTED_IMAGE").ok_or_else(|| {
+                anyhow!("BOOTED_IMAGE not in archive's symbol table")
+            })?;
+
+        let size = size as usize;
+        if size < ID_LEN + MAGIC.len() {
+            // Even if the name was empty, the fixed length parts wouldn't fit.
+            // Don't try to interpret it.
+            return Err(anyhow!("BOOTED_IMAGE array is unexpectedly short"));
+        }
+
+        let mut block = vec![0u8; size];
+        core.read_8(addr, &mut block).with_context(|| {
+            format!("failed to read BOOTED_IMAGE at 0x{:x}", addr)
+        })?;
+
+        let id = &block[..ID_LEN];
+        // We don't know the length of the name, but we know the lengths of the
+        // stuff on either side.
+        let name = &block[ID_LEN..size - MAGIC.len()];
+        let magic = &block[size - MAGIC.len()..];
+        if magic != MAGIC {
+            return Err(
+                anyhow!("BOOTED_IMAGE array does not contain magic marker"),
+            );
+        }
+        let name_string =
+            String::from_utf8_lossy(name).trim_end_matches('\0').to_string();
+
+        Ok((id.to_vec(), name_string))
+    }
+
+    pub fn read_image_id_from_flash(
+        &self,
+        core: &mut dyn crate::core::Core,
+    ) -> Result<Vec<u8>> {
+        let addr = self.imageid.0;
+        let nbytes = self.imageid.1.len();
+        assert!(nbytes > 0);
+
+        let mut id = vec![0; nbytes];
+        core.read_8(addr, &mut id[0..nbytes]).with_context(|| {
+            format!("failed to read image ID at 0x{:x}; board mismatch?", addr)
+        })?;
+        Ok(id)
+    }
+
     pub fn verify(
         &self,
         core: &mut dyn crate::core::Core,

humility-flash/src/lib.rs

@@ -5,10 +5,12 @@
 //! Functions to check flash state and reprogram a processor
 #![warn(missing_docs)]
 
+use anyhow::anyhow;
+
 use humility::{
     core::Core,
-    hubris::{HubrisArchive, HubrisValidate},
-    log::{Logger, info},
+    hubris::HubrisArchive,
+    log::{Logger, info, warn},
 };
 use humility_auxflash::{AuxFlashHandler, AuxFlashWriter};
 use humility_probes_core::ProbeCore;
@@ -92,11 +94,45 @@ pub fn get_image_state(
 ) -> Result<ImageStateResult, ImageStateError> {
     core.halt().map_err(ImageStateError::HaltFailed)?;
 
-    // First pass: check only the image ID
-    if let Err(e) = hubris.validate(core, HubrisValidate::ArchiveMatch) {
-        return Ok(ImageStateResult::DoesNotMatch(
-            ImageStateMismatch::FlashArchiveMismatch(e),
-        ));
+    // Warn if we're sure that the user is flashing an archive for a different
+    // image slot than the one currently running (e.g. flashing an A image onto
+    // an RoT that's running a B image). This isn't necessarily a problem but it
+    // means that the newly flashed image won't actually run after the target
+    // resets. If the user does it by accident, they could get very confused.
+    if let Some(archive_image_name) = hubris.manifest.image.as_deref()
+        && let Ok((_, name)) = hubris.booted_image(core)
+        && name != archive_image_name
+    {
+        warn!(
+            log,
+            "the archive contains image '{archive_image_name}' but the target \
+             is running image '{name}'; image '{archive_image_name}' won't run \
+             unless the boot preference is changed"
+        );
+    }
+
+    // First pass: check only the image ID.
+    //
+    // We specifically care about the ID of the image stored in the FLASH slot
+    // that we're planning to overwrite, not the ID of the image that's
+    // currently running.
+    match hubris.read_image_id_from_flash(core) {
+        Err(e) => {
+            return Ok(ImageStateResult::DoesNotMatch(
+                ImageStateMismatch::FlashArchiveMismatch(e),
+            ));
+        }
+        Ok(id) if id != hubris.image_id() => {
+            return Ok(ImageStateResult::DoesNotMatch(
+                ImageStateMismatch::FlashArchiveMismatch(anyhow!(
+                    "image ID in archive ({:x?}) does not equal \
+                         image ID of target ({:x?})",
+                    hubris.image_id(),
+                    id,
+                )),
+            ));
+        }
+        _ => (),
     }
 
     // More rigorous checks if requested

humility-probes-core/src/lib.rs

@@ -300,6 +300,7 @@ impl HubrisAttach for humility::hubris::HubrisArchive {
         self.validate(
             &mut core,
             humility::hubris::HubrisValidate::ArchiveMatch,
+            log,
         )?;
 
         Ok(core)