Fix keystore2 crash counting
https://r.android.com/1971319 changed the return type of
rustutils::system_properties::read() from Result<String> to
Result<Option<String>>. But, read_keystore_crash_count() was not
correctly updated to handle the Ok(None) case. Consequently, the case
of "property doesn't exist" started being considered an error, and the
code intended to handle this case stopped being executed. Fix this by
correctly handling the return value.
Bug: 284163087
Test: Verified that the read_keystore_crash_count() error message is no
longer present in logcat at boot time, and
'getprop keystore.crash_count' shows 0.
Change-Id: I4b9ff16cba9e7500623dab7c3bc888cba0daf997
diff --git a/keystore2/src/metrics_store.rs b/keystore2/src/metrics_store.rs
index 77cead8..5d311f0 100644
--- a/keystore2/src/metrics_store.rs
+++ b/keystore2/src/metrics_store.rs
@@ -43,9 +43,8 @@
RkpError::RkpError as MetricsRkpError, RkpErrorStats::RkpErrorStats,
SecurityLevel::SecurityLevel as MetricsSecurityLevel, Storage::Storage as MetricsStorage,
};
-use anyhow::{Context, Result};
+use anyhow::{anyhow, Context, Result};
use lazy_static::lazy_static;
-use rustutils::system_properties::PropertyWatcherError;
use std::collections::HashMap;
use std::sync::Mutex;
@@ -93,12 +92,15 @@
// Process keystore crash stats.
if AtomID::CRASH_STATS == atom_id {
- return Ok(vec![KeystoreAtom {
- payload: KeystoreAtomPayload::CrashStats(CrashStats {
- count_of_crash_events: read_keystore_crash_count()?,
- }),
- ..Default::default()
- }]);
+ return match read_keystore_crash_count()? {
+ Some(count) => Ok(vec![KeystoreAtom {
+ payload: KeystoreAtomPayload::CrashStats(CrashStats {
+ count_of_crash_events: count,
+ }),
+ ..Default::default()
+ }]),
+ None => Err(anyhow!("Crash count property is not set")),
+ };
}
// It is safe to call unwrap here since the lock can not be poisoned based on its usage
@@ -564,27 +566,21 @@
/// If the property is absent, it sets the property with value 0. If the property is present, it
/// increments the value. This helps tracking keystore crashes internally.
pub fn update_keystore_crash_sysprop() {
- let crash_count = read_keystore_crash_count();
- let new_count = match crash_count {
- Ok(count) => count + 1,
+ let new_count = match read_keystore_crash_count() {
+ Ok(Some(count)) => count + 1,
+ // If the property is absent, then this is the first start up during the boot.
+ // Proceed to write the system property with value 0.
+ Ok(None) => 0,
Err(error) => {
- // If the property is absent, this is the first start up during the boot.
- // Proceed to write the system property with value 0. Otherwise, log and return.
- if !matches!(
- error.root_cause().downcast_ref::<PropertyWatcherError>(),
- Some(PropertyWatcherError::SystemPropertyAbsent)
- ) {
- log::warn!(
- concat!(
- "In update_keystore_crash_sysprop: ",
- "Failed to read the existing system property due to: {:?}.",
- "Therefore, keystore crashes will not be logged."
- ),
- error
- );
- return;
- }
- 0
+ log::warn!(
+ concat!(
+ "In update_keystore_crash_sysprop: ",
+ "Failed to read the existing system property due to: {:?}.",
+ "Therefore, keystore crashes will not be logged."
+ ),
+ error
+ );
+ return;
}
};
@@ -602,12 +598,12 @@
}
/// Read the system property: keystore.crash_count.
-pub fn read_keystore_crash_count() -> Result<i32> {
- rustutils::system_properties::read("keystore.crash_count")
- .context(ks_err!("Failed read property."))?
- .context(ks_err!("Property not set."))?
- .parse::<i32>()
- .map_err(std::convert::Into::into)
+pub fn read_keystore_crash_count() -> Result<Option<i32>> {
+ match rustutils::system_properties::read("keystore.crash_count") {
+ Ok(Some(count)) => count.parse::<i32>().map(Some).map_err(std::convert::Into::into),
+ Ok(None) => Ok(None),
+ Err(e) => Err(e).context(ks_err!("Failed to read crash count property.")),
+ }
}
/// Enum defining the bit position for each padding mode. Since padding mode can be repeatable, it