From 97466cb1453585930d863cccdedf8acf57520edf Mon Sep 17 00:00:00 2001 From: Tommy Chiu Date: Tue, 26 Mar 2019 17:18:09 +0800 Subject: [PATCH 1/3] vold: Introduce android::vold::writeStringToFile Remove static definition of writeStringToFile, and move it from KeyStorage to Utils (cherry picked from commit 0bd2d116921ab46312cc4a37246a68d38447a72b) Bug: 71810347 Test: Build pass and reboot stress test. Change-Id: I38bfd27370ac2372e446dc699f518122e73c6877 Merged-In: I38bfd27370ac2372e446dc699f518122e73c6877 --- KeyStorage.cpp | 27 --------------------------- Utils.cpp | 28 ++++++++++++++++++++++++++++ Utils.h | 1 + 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/KeyStorage.cpp b/KeyStorage.cpp index 42890ca..d00225b 100644 --- a/KeyStorage.cpp +++ b/KeyStorage.cpp @@ -147,33 +147,6 @@ static bool readFileToString(const std::string& filename, std::string* result) { return true; } -static bool writeStringToFile(const std::string& payload, const std::string& filename) { - android::base::unique_fd fd(TEMP_FAILURE_RETRY( - open(filename.c_str(), O_WRONLY | O_CREAT | O_NOFOLLOW | O_TRUNC | O_CLOEXEC, 0666))); - if (fd == -1) { - PLOG(ERROR) << "Failed to open " << filename; - return false; - } - if (!android::base::WriteStringToFd(payload, fd)) { - PLOG(ERROR) << "Failed to write to " << filename; - unlink(filename.c_str()); - return false; - } - // fsync as close won't guarantee flush data - // see close(2), fsync(2) and b/68901441 - if (fsync(fd) == -1) { - if (errno == EROFS || errno == EINVAL) { - PLOG(WARNING) << "Skip fsync " << filename - << " on a file system does not support synchronization"; - } else { - PLOG(ERROR) << "Failed to fsync " << filename; - unlink(filename.c_str()); - return false; - } - } - return true; -} - static bool readRandomBytesOrLog(size_t count, std::string* out) { auto status = ReadRandomBytes(count, *out); if (status != OK) { diff --git a/Utils.cpp b/Utils.cpp index a8273d7..405c22e 100644 --- a/Utils.cpp +++ b/Utils.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -840,5 +841,32 @@ bool FsyncDirectory(const std::string& dirname) { return true; } +bool writeStringToFile(const std::string& payload, const std::string& filename) { + android::base::unique_fd fd(TEMP_FAILURE_RETRY( + open(filename.c_str(), O_WRONLY | O_CREAT | O_NOFOLLOW | O_TRUNC | O_CLOEXEC, 0666))); + if (fd == -1) { + PLOG(ERROR) << "Failed to open " << filename; + return false; + } + if (!android::base::WriteStringToFd(payload, fd)) { + PLOG(ERROR) << "Failed to write to " << filename; + unlink(filename.c_str()); + return false; + } + // fsync as close won't guarantee flush data + // see close(2), fsync(2) and b/68901441 + if (fsync(fd) == -1) { + if (errno == EROFS || errno == EINVAL) { + PLOG(WARNING) << "Skip fsync " << filename + << " on a file system does not support synchronization"; + } else { + PLOG(ERROR) << "Failed to fsync " << filename; + unlink(filename.c_str()); + return false; + } + } + return true; +} + } // namespace vold } // namespace android diff --git a/Utils.h b/Utils.h index 48a57d9..c083021 100644 --- a/Utils.h +++ b/Utils.h @@ -134,6 +134,7 @@ status_t WaitForFile(const char* filename, std::chrono::nanoseconds timeout); bool FsyncDirectory(const std::string& dirname); +bool writeStringToFile(const std::string& payload, const std::string& filename); } // namespace vold } // namespace android From 11621353f257da69595d5c608b75d5220532f06c Mon Sep 17 00:00:00 2001 From: Tommy Chiu Date: Tue, 26 Mar 2019 14:14:19 +0800 Subject: [PATCH 2/3] vold: fsync both file and directory after write keys Use vold version of writeStringToFile which fsync files, and manually fsync directories after initialize global DE (cherry picked from commit a98464f688d6e16ca7558251306ece98058b55ce) Bug: 71810347 Test: Build pass and reboot stress test. Original boot failure symptom is NOT reproducible. Change-Id: I1ca8f8cf0ccfd01075a9c33f79042e58d99aea26 Merged-In: I1ca8f8cf0ccfd01075a9c33f79042e58d99aea26 --- FsCrypt.cpp | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/FsCrypt.cpp b/FsCrypt.cpp index e810d58..ea8bb73 100644 --- a/FsCrypt.cpp +++ b/FsCrypt.cpp @@ -60,10 +60,10 @@ #include using android::base::StringPrintf; -using android::base::WriteStringToFile; using android::fs_mgr::GetEntryForMountPoint; using android::vold::kEmptyAuthentication; using android::vold::KeyBuffer; +using android::vold::writeStringToFile; namespace { @@ -351,18 +351,14 @@ bool fscrypt_initialize_global_de() { std::string modestring = device_ref.contents_mode + ":" + device_ref.filenames_mode; std::string mode_filename = std::string("/data") + fscrypt_key_mode; - if (!android::base::WriteStringToFile(modestring, mode_filename)) { - PLOG(ERROR) << "Cannot save type"; - return false; - } + if (!android::vold::writeStringToFile(modestring, mode_filename)) return false; std::string ref_filename = std::string("/data") + fscrypt_key_ref; - if (!android::base::WriteStringToFile(device_ref.key_raw_ref, ref_filename)) { - PLOG(ERROR) << "Cannot save key reference to:" << ref_filename; - return false; - } + if (!android::vold::writeStringToFile(device_ref.key_raw_ref, ref_filename)) return false; + LOG(INFO) << "Wrote system DE key reference to:" << ref_filename; + if (!android::vold::FsyncDirectory(device_key_dir)) return false; s_global_de_initialized = true; return true; } @@ -419,7 +415,7 @@ static void drop_caches() { // Clean any dirty pages (otherwise they won't be dropped). sync(); // Drop inode and page caches. - if (!WriteStringToFile("3", "/proc/sys/vm/drop_caches")) { + if (!writeStringToFile("3", "/proc/sys/vm/drop_caches")) { PLOG(ERROR) << "Failed to drop caches during key eviction"; } } From 747b421a22c920d8f8894f9ccf8655328441aa3a Mon Sep 17 00:00:00 2001 From: Paul Crowley Date: Fri, 5 Apr 2019 04:09:57 -0700 Subject: [PATCH 3/3] clang-format Utils.cpp Test: treehugger Change-Id: I405750812ae037088492bfa7d8db6a8a56cb3425 --- Utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Utils.cpp b/Utils.cpp index 405c22e..6a3830f 100644 --- a/Utils.cpp +++ b/Utils.cpp @@ -36,13 +36,13 @@ #include #include #include -#include #include #include #include #include #include #include +#include #include #include