Merge "[avb] Improve error reporting when parsing descriptors from VBMeta"
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),
)
}