commit e179157dc543cb4b94479fe069c8c7b2228ed71c
author David Anderson <dvander@google.com> Fri Nov 05 18:57:49 2021 -0700
committer David Anderson <dvander@google.com> Mon Nov 08 13:22:53 2021 -0800
tree cdc3820070df7ae858fb38d67b77af7b46c7c2f9
parent 2a89e7c5778c77d9a01812fb6f9e0ad733ba99d5

Improve vold logging.

This patch adds more error logging to mountFstab. In a few cases, the
were error paths with no existing error logs. In other cases, the log
messages are there to help understand error flow in logs (for example
when a function with lots of error paths returns false).

Bug: 205314634
Test: treehugger builds
Change-Id: I464edc6e74ea0d7419ee9d9b75fd238752c13f4f

KeyStorage.cpp

diff --git a/KeyStorage.cpp b/KeyStorage.cpp
index 50bba56..4b39aef 100644
--- a/KeyStorage.cpp
+++ b/KeyStorage.cpp
@@ -601,9 +601,15 @@
         if (!generateKeyStorageKey(keystore, appId, &ksKey)) return false;
         if (!writeStringToFile(ksKey, dir + "/" + kFn_keymaster_key_blob)) return false;
         km::AuthorizationSet keyParams = beginParams(appId);
-        if (!encryptWithKeystoreKey(keystore, dir, keyParams, key, &encryptedKey)) return false;
+        if (!encryptWithKeystoreKey(keystore, dir, keyParams, key, &encryptedKey)) {
+            LOG(ERROR) << "encryptWithKeystoreKey failed";
+            return false;
+        }
     } else {
-        if (!encryptWithoutKeystore(appId, key, &encryptedKey)) return false;
+        if (!encryptWithoutKeystore(appId, key, &encryptedKey)) {
+            LOG(ERROR) << "encryptWithoutKeystore failed";
+            return false;
+        }
     }
     if (!writeStringToFile(encryptedKey, dir + "/" + kFn_encrypted_key)) return false;
     if (!FsyncDirectory(dir)) return false;
@@ -648,9 +654,15 @@
         Keystore keystore;
         if (!keystore) return false;
         km::AuthorizationSet keyParams = beginParams(appId);
-        if (!decryptWithKeystoreKey(keystore, dir, keyParams, encryptedMessage, key)) return false;
+        if (!decryptWithKeystoreKey(keystore, dir, keyParams, encryptedMessage, key)) {
+            LOG(ERROR) << "decryptWithKeystoreKey failed";
+            return false;
+        }
     } else {
-        if (!decryptWithoutKeystore(appId, encryptedMessage, key)) return false;
+        if (!decryptWithoutKeystore(appId, encryptedMessage, key)) {
+            LOG(ERROR) << "decryptWithoutKeystore failed";
+            return false;
+        }
     }
     return true;
 }

KeyUtil.cpp

diff --git a/KeyUtil.cpp b/KeyUtil.cpp
index 2f9c876..25d5af3 100644
--- a/KeyUtil.cpp
+++ b/KeyUtil.cpp
@@ -54,7 +54,10 @@
 }
 
 bool generateStorageKey(const KeyGeneration& gen, KeyBuffer* key) {
-    if (!gen.allow_gen) return false;
+    if (!gen.allow_gen) {
+        LOG(ERROR) << "Generating storage key not allowed";
+        return false;
+    }
     if (gen.use_hw_wrapped_key) {
         if (gen.keysize != FSCRYPT_MAX_KEY_SIZE) {
             LOG(ERROR) << "Cannot generate a wrapped key " << gen.keysize << " bytes long";

MetadataCrypt.cpp

diff --git a/MetadataCrypt.cpp b/MetadataCrypt.cpp
index 8b2b960..0bd6100 100644
--- a/MetadataCrypt.cpp
+++ b/MetadataCrypt.cpp
@@ -245,7 +245,8 @@
                << fs_type;
     auto encrypted_state = android::base::GetProperty("ro.crypto.state", "");
     if (encrypted_state != "" && encrypted_state != "encrypted") {
-        LOG(DEBUG) << "fscrypt_enable_crypto got unexpected starting state: " << encrypted_state;
+        LOG(ERROR) << "fscrypt_mount_metadata_encrypted got unexpected starting state: "
+                   << encrypted_state;
         return false;
     }
 
@@ -282,12 +283,18 @@
 
     auto gen = needs_encrypt ? makeGen(options) : neverGen();
     KeyBuffer key;
-    if (!read_key(data_rec->metadata_key_dir, gen, &key)) return false;
+    if (!read_key(data_rec->metadata_key_dir, gen, &key)) {
+        LOG(ERROR) << "read_key failed in mountFstab";
+        return false;
+    }
 
     std::string crypto_blkdev;
     uint64_t nr_sec;
-    if (!create_crypto_blk_dev(kDmNameUserdata, blk_device, key, options, &crypto_blkdev, &nr_sec))
+    if (!create_crypto_blk_dev(kDmNameUserdata, blk_device, key, options, &crypto_blkdev,
+                               &nr_sec)) {
+        LOG(ERROR) << "create_crypto_blk_dev failed in mountFstab";
         return false;
+    }
 
     if (needs_encrypt) {
         if (should_format) {
@@ -301,10 +308,17 @@
                 LOG(ERROR) << "Unknown filesystem type: " << fs_type;
                 return false;
             }
-            LOG(DEBUG) << "Format (err=" << error << ") " << crypto_blkdev << " on " << mount_point;
-            if (error != 0) return false;
+            if (error != 0) {
+                LOG(ERROR) << "Format of " << crypto_blkdev << " for " << mount_point
+                           << " failed (err=" << error << ").";
+                return false;
+            }
+            LOG(DEBUG) << "Format of " << crypto_blkdev << " for " << mount_point << " succeeded.";
         } else {
-            if (!encrypt_inplace(crypto_blkdev, blk_device, nr_sec, false)) return false;
+            if (!encrypt_inplace(crypto_blkdev, blk_device, nr_sec, false)) {
+                LOG(ERROR) << "encrypt_inplace failed in mountFstab";
+                return false;
+            }
         }
     }