[avb] Improve error reporting when parsing descriptors from VBMeta
Prior to this cl, the only error that was reported was an
InvalidMetadata error when the descriptors failed to parse.
This cl enhances the error reporting by saving the internal error
that occurred during the parsing process and passing it along to
a new InvalidDescriptors error.
Test: atest libpvmfw_avb.integration_test
Bug: 279557218
Change-Id: Id90267e137c09d3604ef069b1aef9fa4dc8d9ad8
diff --git a/pvmfw/avb/src/descriptor/collection.rs b/pvmfw/avb/src/descriptor/collection.rs
index 40158d8..2e9dd99 100644
--- a/pvmfw/avb/src/descriptor/collection.rs
+++ b/pvmfw/avb/src/descriptor/collection.rs
@@ -18,7 +18,7 @@
use super::hash::HashDescriptor;
use crate::error::{AvbIOError, AvbSlotVerifyError};
use crate::partition::PartitionName;
-use crate::utils::{self, is_not_null, to_nonnull, to_usize, usize_checked_add};
+use crate::utils::{self, is_not_null, to_usize, usize_checked_add};
use avb_bindgen::{
avb_descriptor_foreach, avb_descriptor_validate_and_byteswap, AvbDescriptor, AvbDescriptorTag,
AvbVBMetaData,
@@ -45,20 +45,22 @@
/// * `vbmeta.vbmeta_data` must be valid for reading `vbmeta.vbmeta_size` bytes.
pub(crate) unsafe fn from_vbmeta(vbmeta: AvbVBMetaData) -> Result<Self, AvbSlotVerifyError> {
is_not_null(vbmeta.vbmeta_data).map_err(|_| AvbSlotVerifyError::Io)?;
- let mut descriptors = Self::default();
+ let mut res: Result<Self, AvbIOError> = Ok(Self::default());
// SAFETY: It is safe as the raw pointer `vbmeta.vbmeta_data` is a non-null pointer and
// points to a valid VBMeta structure.
- if !unsafe {
+ let output = unsafe {
avb_descriptor_foreach(
vbmeta.vbmeta_data,
vbmeta.vbmeta_size,
Some(check_and_save_descriptor),
- &mut descriptors as *mut _ as *mut c_void,
+ &mut res as *mut _ as *mut c_void,
)
- } {
- return Err(AvbSlotVerifyError::InvalidMetadata);
+ };
+ if output == res.is_ok() {
+ res.map_err(AvbSlotVerifyError::InvalidDescriptors)
+ } else {
+ Err(AvbSlotVerifyError::InvalidMetadata)
}
- Ok(descriptors)
}
pub(crate) fn num_hash_descriptor(&self) -> usize {
@@ -96,29 +98,40 @@
///
/// Behavior is undefined if any of the following conditions are violated:
/// * The `descriptor` pointer must be non-null and points to a valid `AvbDescriptor` struct.
-/// * The `user_data` pointer must be non-null and points to a valid `Descriptors` struct.
+/// * The `user_data` pointer must be non-null, points to a valid `Result<Descriptors, AvbIOError>`
+/// struct and is initialized.
unsafe extern "C" fn check_and_save_descriptor(
descriptor: *const AvbDescriptor,
user_data: *mut c_void,
) -> bool {
- // SAFETY: It is safe because the caller must ensure that the `descriptor` pointer and
- // the `user_data` are non-null and valid.
- unsafe { try_check_and_save_descriptor(descriptor, user_data).is_ok() }
+ let res = user_data as *mut Result<Descriptors, AvbIOError>;
+ // SAFETY: It is safe because the caller ensures that `user_data` points to a valid struct and
+ // is initialized.
+ let Some(Ok(descriptors)) = (unsafe { res.as_mut() }) else {
+ return false;
+ };
+ // SAFETY: It is safe because the caller ensures that the `descriptor` pointer is non-null
+ // and valid.
+ if let Err(e) = unsafe { try_check_and_save_descriptor(descriptor, descriptors) } {
+ // SAFETY: It is safe because the caller ensures that `user_data` points to a valid
+ // `Result<Descriptors, AvbIOError>` struct and has no other user when this is called.
+ unsafe {
+ *res = Err(e);
+ }
+ false
+ } else {
+ true
+ }
}
/// # Safety
///
/// Behavior is undefined if any of the following conditions are violated:
/// * The `descriptor` pointer must be non-null and points to a valid `AvbDescriptor` struct.
-/// * The `user_data` pointer must be non-null and points to a valid `Descriptors` struct.
unsafe fn try_check_and_save_descriptor(
descriptor: *const AvbDescriptor,
- user_data: *mut c_void,
+ descriptors: &mut Descriptors,
) -> utils::Result<()> {
- let mut descriptors = to_nonnull(user_data as *mut Descriptors)?;
- // SAFETY: It is safe because the caller ensures that `user_data` is a non-null pointer
- // pointing to a valid struct.
- let descriptors = unsafe { descriptors.as_mut() };
// SAFETY: It is safe because the caller ensures that `descriptor` is a non-null pointer
// pointing to a valid struct.
let descriptor = unsafe { Descriptor::from_descriptor_ptr(descriptor)? };
diff --git a/pvmfw/avb/src/error.rs b/pvmfw/avb/src/error.rs
index c2b7428..1add368 100644
--- a/pvmfw/avb/src/error.rs
+++ b/pvmfw/avb/src/error.rs
@@ -39,6 +39,8 @@
UnsupportedVersion,
/// AVB_SLOT_VERIFY_RESULT_ERROR_VERIFICATION
Verification,
+ /// VBMeta has invalid descriptors.
+ InvalidDescriptors(AvbIOError),
}
impl fmt::Display for AvbSlotVerifyError {
@@ -55,6 +57,9 @@
"Some of the metadata requires a newer version of libavb than what is in use."
),
Self::Verification => write!(f, "Data does not verify."),
+ Self::InvalidDescriptors(e) => {
+ write!(f, "VBMeta has invalid descriptors. Error: {:?}", e)
+ }
}
}
}
@@ -87,8 +92,9 @@
}
}
-#[derive(Debug)]
-pub(crate) enum AvbIOError {
+/// AVB IO Error.
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum AvbIOError {
/// AVB_IO_RESULT_ERROR_OOM,
#[allow(dead_code)]
Oom,
diff --git a/pvmfw/avb/src/lib.rs b/pvmfw/avb/src/lib.rs
index d83737f..9ad7fc3 100644
--- a/pvmfw/avb/src/lib.rs
+++ b/pvmfw/avb/src/lib.rs
@@ -24,5 +24,5 @@
mod verify;
pub use descriptor::Digest;
-pub use error::AvbSlotVerifyError;
+pub use error::{AvbIOError, AvbSlotVerifyError};
pub use verify::{verify_payload, DebugLevel, VerifiedBootData};
diff --git a/pvmfw/avb/tests/api_test.rs b/pvmfw/avb/tests/api_test.rs
index 78f274a..a4a33f7 100644
--- a/pvmfw/avb/tests/api_test.rs
+++ b/pvmfw/avb/tests/api_test.rs
@@ -18,7 +18,7 @@
use anyhow::{anyhow, Result};
use avb_bindgen::{AvbFooter, AvbVBMetaImageHeader};
-use pvmfw_avb::{verify_payload, AvbSlotVerifyError, DebugLevel, VerifiedBootData};
+use pvmfw_avb::{verify_payload, AvbIOError, AvbSlotVerifyError, DebugLevel, VerifiedBootData};
use std::{fs, mem::size_of, ptr};
use utils::*;
@@ -79,7 +79,7 @@
&fs::read(TEST_IMG_WITH_NON_INITRD_HASHDESC_PATH)?,
/*initrd=*/ None,
&load_trusted_public_key()?,
- AvbSlotVerifyError::InvalidMetadata,
+ AvbSlotVerifyError::InvalidDescriptors(AvbIOError::NoSuchPartition),
)
}
@@ -89,7 +89,7 @@
&fs::read(TEST_IMG_WITH_INITRD_AND_NON_INITRD_DESC_PATH)?,
&load_latest_initrd_normal()?,
&load_trusted_public_key()?,
- AvbSlotVerifyError::InvalidMetadata,
+ AvbSlotVerifyError::InvalidDescriptors(AvbIOError::NoSuchPartition),
)
}
@@ -99,7 +99,7 @@
&fs::read(TEST_IMG_WITH_PROP_DESC_PATH)?,
/*initrd=*/ None,
&load_trusted_public_key()?,
- AvbSlotVerifyError::InvalidMetadata,
+ AvbSlotVerifyError::InvalidDescriptors(AvbIOError::NoSuchValue),
)
}