From 4b9c47d70fd3f7dd36ed9f7e9a3a289d794ecbae Mon Sep 17 00:00:00 2001 From: Paul Crowley Date: Fri, 7 Dec 2018 15:36:09 -0800 Subject: [PATCH 1/2] Fsync directories after creating files Bug: 112145641 Bug: 124279741 Bug: 120248692 Test: adb shell locksettings set-pin 1111 && \ adb shell "echo b > /proc/sysrq-trigger" Change-Id: I53d252942c21365983b4f8b6e0948b1864f195c1 Merged-In: I53d252942c21365983b4f8b6e0948b1864f195c1 (cherry picked from commit 2e58acb4123e559fddfd4013af3ead6c055bd71c) --- Ext4Crypt.cpp | 2 ++ KeyStorage.cpp | 1 + Utils.cpp | 19 +++++++++++++++++++ Utils.h | 2 ++ 4 files changed, 24 insertions(+) diff --git a/Ext4Crypt.cpp b/Ext4Crypt.cpp index 67b7e90..68439c0 100644 --- a/Ext4Crypt.cpp +++ b/Ext4Crypt.cpp @@ -177,6 +177,7 @@ static void fixate_user_ce_key(const std::string& directory_path, const std::str PLOG(WARNING) << "Unable to rename " << to_fix << " to " << current_path; } } + android::vold::FsyncDirectory(directory_path); } static bool read_and_fixate_user_ce_key(userid_t user_id, @@ -569,6 +570,7 @@ bool e4crypt_add_user_key_auth(userid_t user_id, int serial, const std::string& std::string ce_key_path; if (!get_ce_key_new_path(directory_path, paths, &ce_key_path)) return false; if (!android::vold::storeKeyAtomically(ce_key_path, user_key_temp, auth, ce_key)) return false; + if (!android::vold::FsyncDirectory(directory_path)) return false; return true; } diff --git a/KeyStorage.cpp b/KeyStorage.cpp index 0518930..84dd8ec 100644 --- a/KeyStorage.cpp +++ b/KeyStorage.cpp @@ -480,6 +480,7 @@ bool storeKey(const std::string& dir, const KeyAuthentication& auth, const KeyBu if (!encryptWithoutKeymaster(appId, key, &encryptedKey)) return false; } if (!writeStringToFile(encryptedKey, dir + "/" + kFn_encrypted_key)) return false; + if (!FsyncDirectory(dir)) return false; return true; } diff --git a/Utils.cpp b/Utils.cpp index 98e8a9b..d578d79 100644 --- a/Utils.cpp +++ b/Utils.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -731,5 +732,23 @@ bool IsRunningInEmulator() { return android::base::GetBoolProperty("ro.kernel.qemu", false); } +bool FsyncDirectory(const std::string& dirname) { + android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(dirname.c_str(), O_RDONLY | O_CLOEXEC))); + if (fd == -1) { + PLOG(ERROR) << "Failed to open " << dirname; + return false; + } + if (fsync(fd) == -1) { + if (errno == EROFS || errno == EINVAL) { + PLOG(WARNING) << "Skip fsync " << dirname + << " on a file system does not support synchronization"; + } else { + PLOG(ERROR) << "Failed to fsync " << dirname; + return false; + } + } + return true; +} + } // namespace vold } // namespace android diff --git a/Utils.h b/Utils.h index 5caa4e9..533e17c 100644 --- a/Utils.h +++ b/Utils.h @@ -125,6 +125,8 @@ bool Readlinkat(int dirfd, const std::string& path, std::string* result); /* Checks if Android is running in QEMU */ bool IsRunningInEmulator(); +bool FsyncDirectory(const std::string& dirname); + } // namespace vold } // namespace android From 0496e3698f279bedf9d2ce86aa883e5f4a4351f7 Mon Sep 17 00:00:00 2001 From: Woody Lin Date: Mon, 11 Mar 2019 20:58:20 +0800 Subject: [PATCH 2/2] Fsync directories before delete key The boot failure symptom is reproduced on Walleye devices. System boots up after taking OTA and try to upgrade key, but keymaster returns "failed to ugprade key". Device reboots to recovery mode because of the failure, and finally trapped in bootloader screen. Possible scenario is: (After taking OTA) vold sends old key and op=UPGRADE to keymaster keymaster creates and saves new key to RPMB, responses new key to vold vold saves new key as temp key vold renames temp key to main key -------------- (1) -- still in cache vold sends old key and op=DELETE_KEY to keymaster keymaster removes old key from RPMB ------------ (2) -- write directly to RPMB ==> SYSTEM INTERRUPTED BY CRASH OR SOMETHING; ALL CACHE LOST. ==> System boots up, key in RPMB is deleted but key in storage is old key. Solution: A Fsync is required between (1) and (2) to cover this case. Detail analysis: b/124279741#comment21 Bug: 112145641 Bug: 124279741 Test: Insert fault right after deleteKey in vold::begin (KeyStorage.cpp), original boot failure symptom is NOT reproducible. Change-Id: Ia042b23699c37c94758fb660aecec64d39f39738 Merged-In: Ib8c349d6d033f86b247f4b35b8354d97cf249d26 (cherry picked from commit a598e04a91c64741f9f71c6511a7ced7f71d194e) --- KeyStorage.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/KeyStorage.cpp b/KeyStorage.cpp index 84dd8ec..6fc7250 100644 --- a/KeyStorage.cpp +++ b/KeyStorage.cpp @@ -223,6 +223,10 @@ static KeymasterOperation begin(Keymaster& keymaster, const std::string& dir, PLOG(ERROR) << "Unable to move upgraded key to location: " << kmKeyPath; return KeymasterOperation(); } + if (!android::vold::FsyncDirectory(dir)) { + LOG(ERROR) << "Key dir sync failed: " << dir; + return KeymasterOperation(); + } if (!keymaster.deleteKey(kmKey)) { LOG(ERROR) << "Key deletion failed during upgrade, continuing anyway: " << dir; }