From b5ebd7d9c7fe8d2b4bc60d54f1f5ad70cac4e28e Mon Sep 17 00:00:00 2001 From: Paul Lawrence Date: Tue, 25 Jun 2019 14:44:33 -0700 Subject: [PATCH] Make ext4 userdata checkpoints work with metadata encryption When both ext4 user data checkpoints and metadata encryption are enabled, we are creating two stacked dm devices. This had not been properly thought through or debugged. Test: Enable metadata encryption on taimen (add keydirectory=/metadata/vold/metadata_encryption to flags for userdata in fstab.hardware) Unfortunately metadata is not wiped by fastboot -w, so it is necessary to rm metadata/vold -rf whenever you wipe data. fastboot flashall -w works fastboot reboot -w works A normal boot works Disable checkpoint commits with setprop persist.vold.dont_commit_checkpoint 1 vdc checkpoint startCheckpoint 10 adb reboot wait for device to fully boot then adb reboot Wait for device to fully boot then adb logcat -d | grep Checkpoint shows the rollback in the logs This tests encryption on top of checkpoints with commit, encryption without checkpoints, and rollback, which seems to be the key cases. Also ran same tests on unmodified Taimen and Blueline Bug: 135905679 Merged-In: I8365a40298b752af4bb10d00d9ff58ce04beab1f Change-Id: I8365a40298b752af4bb10d00d9ff58ce04beab1f --- Checkpoint.cpp | 25 +++++++++++-------------- EncryptInplace.cpp | 17 +++++++++-------- EncryptInplace.h | 2 +- MetadataCrypt.cpp | 8 +++++--- MetadataCrypt.h | 3 ++- VoldNativeService.cpp | 10 ++++++---- VoldNativeService.h | 4 ++-- binder/android/os/IVold.aidl | 4 ++-- vdc.cpp | 8 ++++---- 9 files changed, 42 insertions(+), 39 deletions(-) diff --git a/Checkpoint.cpp b/Checkpoint.cpp index a3d78e7..c8af08c 100644 --- a/Checkpoint.cpp +++ b/Checkpoint.cpp @@ -62,14 +62,11 @@ namespace { const std::string kMetadataCPFile = "/metadata/vold/checkpoint"; bool setBowState(std::string const& block_device, std::string const& state) { - if (block_device.substr(0, 5) != "/dev/") { - LOG(ERROR) << "Expected block device, got " << block_device; - return false; - } + std::string bow_device = fs_mgr_find_bow_device(block_device); + if (bow_device.empty()) return false; - std::string state_filename = std::string("/sys/") + block_device.substr(5) + "/bow/state"; - if (!android::base::WriteStringToFile(state, state_filename)) { - PLOG(ERROR) << "Failed to write to file " << state_filename; + if (!android::base::WriteStringToFile(state, bow_device + "/bow/state")) { + PLOG(ERROR) << "Failed to write to file " << bow_device + "/bow/state"; return false; } @@ -279,7 +276,6 @@ const bool commit_on_full_default = true; static void cp_healthDaemon(std::string mnt_pnt, std::string blk_device, bool is_fs_cp) { struct statvfs data; - uint64_t free_bytes = 0; uint32_t msleeptime = GetUintProperty(kSleepTimeProp, msleeptime_default, max_msleeptime); uint64_t min_free_bytes = GetUintProperty(kMinFreeBytesProp, min_free_bytes_default, (uint64_t)-1); @@ -290,16 +286,17 @@ static void cp_healthDaemon(std::string mnt_pnt, std::string blk_device, bool is msleeptime %= 1000; req.tv_nsec = msleeptime * 1000000; while (isCheckpointing) { + uint64_t free_bytes = 0; if (is_fs_cp) { statvfs(mnt_pnt.c_str(), &data); free_bytes = data.f_bavail * data.f_frsize; } else { - int ret; - std::string size_filename = std::string("/sys/") + blk_device.substr(5) + "/bow/free"; - std::string content; - ret = android::base::ReadFileToString(size_filename, &content); - if (ret) { - free_bytes = std::strtoul(content.c_str(), NULL, 10); + std::string bow_device = fs_mgr_find_bow_device(blk_device); + if (!bow_device.empty()) { + std::string content; + if (android::base::ReadFileToString(bow_device + "/bow/free", &content)) { + free_bytes = std::strtoul(content.c_str(), NULL, 10); + } } } if (free_bytes < min_free_bytes) { diff --git a/EncryptInplace.cpp b/EncryptInplace.cpp index f55932d..3755718 100644 --- a/EncryptInplace.cpp +++ b/EncryptInplace.cpp @@ -62,7 +62,8 @@ struct encryptGroupsData { off64_t one_pct, cur_pct, new_pct; off64_t blocks_already_done, tot_numblocks; off64_t used_blocks_already_done, tot_used_blocks; - char *real_blkdev, *crypto_blkdev; + const char* real_blkdev; + const char* crypto_blkdev; int count; off64_t offset; char* buffer; @@ -244,8 +245,8 @@ errout: return rc; } -static int cryptfs_enable_inplace_ext4(char* crypto_blkdev, char* real_blkdev, off64_t size, - off64_t* size_already_done, off64_t tot_size, +static int cryptfs_enable_inplace_ext4(const char* crypto_blkdev, const char* real_blkdev, + off64_t size, off64_t* size_already_done, off64_t tot_size, off64_t previously_encrypted_upto, bool set_progress_properties) { u32 i; @@ -383,8 +384,8 @@ static int encrypt_one_block_f2fs(u64 pos, void* data) { return 0; } -static int cryptfs_enable_inplace_f2fs(char* crypto_blkdev, char* real_blkdev, off64_t size, - off64_t* size_already_done, off64_t tot_size, +static int cryptfs_enable_inplace_f2fs(const char* crypto_blkdev, const char* real_blkdev, + off64_t size, off64_t* size_already_done, off64_t tot_size, off64_t previously_encrypted_upto, bool set_progress_properties) { struct encryptGroupsData data; @@ -457,8 +458,8 @@ errout: return rc; } -static int cryptfs_enable_inplace_full(char* crypto_blkdev, char* real_blkdev, off64_t size, - off64_t* size_already_done, off64_t tot_size, +static int cryptfs_enable_inplace_full(const char* crypto_blkdev, const char* real_blkdev, + off64_t size, off64_t* size_already_done, off64_t tot_size, off64_t previously_encrypted_upto, bool set_progress_properties) { int realfd, cryptofd; @@ -570,7 +571,7 @@ errout: } /* returns on of the ENABLE_INPLACE_* return codes */ -int cryptfs_enable_inplace(char* crypto_blkdev, char* real_blkdev, off64_t size, +int cryptfs_enable_inplace(const char* crypto_blkdev, const char* real_blkdev, off64_t size, off64_t* size_already_done, off64_t tot_size, off64_t previously_encrypted_upto, bool set_progress_properties) { int rc_ext4, rc_f2fs, rc_full; diff --git a/EncryptInplace.h b/EncryptInplace.h index 71644ac..bf0c314 100644 --- a/EncryptInplace.h +++ b/EncryptInplace.h @@ -24,7 +24,7 @@ #define RETRY_MOUNT_ATTEMPTS 10 #define RETRY_MOUNT_DELAY_SECONDS 1 -int cryptfs_enable_inplace(char* crypto_blkdev, char* real_blkdev, off64_t size, +int cryptfs_enable_inplace(const char* crypto_blkdev, const char* real_blkdev, off64_t size, off64_t* size_already_done, off64_t tot_size, off64_t previously_encrypted_upto, bool set_progress_properties); diff --git a/MetadataCrypt.cpp b/MetadataCrypt.cpp index deb7194..35cf3e2 100644 --- a/MetadataCrypt.cpp +++ b/MetadataCrypt.cpp @@ -249,7 +249,8 @@ static bool create_crypto_blk_dev(const std::string& dm_name, uint64_t nr_sec, return true; } -bool fscrypt_mount_metadata_encrypted(const std::string& mount_point, bool needs_encrypt) { +bool fscrypt_mount_metadata_encrypted(const std::string& blk_device, const std::string& mount_point, + bool needs_encrypt) { LOG(DEBUG) << "fscrypt_mount_metadata_encrypted: " << mount_point << " " << needs_encrypt; auto encrypted_state = android::base::GetProperty("ro.crypto.state", ""); if (encrypted_state != "") { @@ -268,13 +269,14 @@ bool fscrypt_mount_metadata_encrypted(const std::string& mount_point, bool needs if (!get_number_of_sectors(data_rec->blk_device, &nr_sec)) return false; std::string crypto_blkdev; if (!create_crypto_blk_dev(kDmNameUserdata, nr_sec, DEFAULT_KEY_TARGET_TYPE, - default_key_params(data_rec->blk_device, key), &crypto_blkdev)) + default_key_params(blk_device, key), &crypto_blkdev)) return false; + // FIXME handle the corrupt case if (needs_encrypt) { LOG(INFO) << "Beginning inplace encryption, nr_sec: " << nr_sec; off64_t size_already_done = 0; - auto rc = cryptfs_enable_inplace(crypto_blkdev.data(), data_rec->blk_device.data(), nr_sec, + auto rc = cryptfs_enable_inplace(crypto_blkdev.data(), blk_device.data(), nr_sec, &size_already_done, nr_sec, 0, false); if (rc != 0) { LOG(ERROR) << "Inplace crypto failed with code: " << rc; diff --git a/MetadataCrypt.h b/MetadataCrypt.h index d82a43b..cd0f5e5 100644 --- a/MetadataCrypt.h +++ b/MetadataCrypt.h @@ -19,6 +19,7 @@ #include -bool fscrypt_mount_metadata_encrypted(const std::string& mount_point, bool needs_encrypt); +bool fscrypt_mount_metadata_encrypted(const std::string& block_device, + const std::string& mount_point, bool needs_encrypt); #endif diff --git a/VoldNativeService.cpp b/VoldNativeService.cpp index 84c06f1..1762b70 100644 --- a/VoldNativeService.cpp +++ b/VoldNativeService.cpp @@ -721,18 +721,20 @@ binder::Status VoldNativeService::isConvertibleToFbe(bool* _aidl_return) { return ok(); } -binder::Status VoldNativeService::mountFstab(const std::string& mountPoint) { +binder::Status VoldNativeService::mountFstab(const std::string& blkDevice, + const std::string& mountPoint) { ENFORCE_UID(AID_SYSTEM); ACQUIRE_LOCK; - return translateBool(fscrypt_mount_metadata_encrypted(mountPoint, false)); + return translateBool(fscrypt_mount_metadata_encrypted(blkDevice, mountPoint, false)); } -binder::Status VoldNativeService::encryptFstab(const std::string& mountPoint) { +binder::Status VoldNativeService::encryptFstab(const std::string& blkDevice, + const std::string& mountPoint) { ENFORCE_UID(AID_SYSTEM); ACQUIRE_LOCK; - return translateBool(fscrypt_mount_metadata_encrypted(mountPoint, true)); + return translateBool(fscrypt_mount_metadata_encrypted(blkDevice, mountPoint, true)); } binder::Status VoldNativeService::createUserKey(int32_t userId, int32_t userSerial, bool ephemeral) { diff --git a/VoldNativeService.h b/VoldNativeService.h index 5ca298e..07a0b9f 100644 --- a/VoldNativeService.h +++ b/VoldNativeService.h @@ -104,8 +104,8 @@ class VoldNativeService : public BinderService, public os::Bn binder::Status mountDefaultEncrypted(); binder::Status initUser0(); binder::Status isConvertibleToFbe(bool* _aidl_return); - binder::Status mountFstab(const std::string& mountPoint); - binder::Status encryptFstab(const std::string& mountPoint); + binder::Status mountFstab(const std::string& blkDevice, const std::string& mountPoint); + binder::Status encryptFstab(const std::string& blkDevice, const std::string& mountPoint); binder::Status createUserKey(int32_t userId, int32_t userSerial, bool ephemeral); binder::Status destroyUserKey(int32_t userId); diff --git a/binder/android/os/IVold.aidl b/binder/android/os/IVold.aidl index 75fef06..03fe258 100644 --- a/binder/android/os/IVold.aidl +++ b/binder/android/os/IVold.aidl @@ -81,8 +81,8 @@ interface IVold { void mountDefaultEncrypted(); void initUser0(); boolean isConvertibleToFbe(); - void mountFstab(@utf8InCpp String mountPoint); - void encryptFstab(@utf8InCpp String mountPoint); + void mountFstab(@utf8InCpp String blkDevice, @utf8InCpp String mountPoint); + void encryptFstab(@utf8InCpp String blkDevice, @utf8InCpp String mountPoint); void createUserKey(int userId, int userSerial, boolean ephemeral); void destroyUserKey(int userId); diff --git a/vdc.cpp b/vdc.cpp index 76eca3e..f05d2af 100644 --- a/vdc.cpp +++ b/vdc.cpp @@ -101,10 +101,10 @@ int main(int argc, char** argv) { checkStatus(vold->shutdown()); } else if (args[0] == "cryptfs" && args[1] == "checkEncryption" && args.size() == 3) { checkStatus(vold->checkEncryption(args[2])); - } else if (args[0] == "cryptfs" && args[1] == "mountFstab" && args.size() == 3) { - checkStatus(vold->mountFstab(args[2])); - } else if (args[0] == "cryptfs" && args[1] == "encryptFstab" && args.size() == 3) { - checkStatus(vold->encryptFstab(args[2])); + } else if (args[0] == "cryptfs" && args[1] == "mountFstab" && args.size() == 4) { + checkStatus(vold->mountFstab(args[2], args[3])); + } else if (args[0] == "cryptfs" && args[1] == "encryptFstab" && args.size() == 4) { + checkStatus(vold->encryptFstab(args[2], args[3])); } else if (args[0] == "checkpoint" && args[1] == "supportsCheckpoint" && args.size() == 2) { bool supported = false; checkStatus(vold->supportsCheckpoint(&supported));