From 7e05c044326d7d4ebc281a1810e3443ab23557d3 Mon Sep 17 00:00:00 2001 From: Kelvin Zhang Date: Wed, 12 Oct 2022 10:08:03 -0700 Subject: [PATCH] Fix bug in WriteStringToFileAtomic According to https://www.slideshare.net/nan1nan1/eat-my-data , rename() without an fsync() is not safe, and cannot guarantee data integrity in case of powerloss of OS failure. Test: partner verification, th Bug: 238702018 Change-Id: I5809770062ed7bfa47df81de418a2d8f7cbc6620 --- fs_mgr/libsnapshot/utility.cpp | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/fs_mgr/libsnapshot/utility.cpp b/fs_mgr/libsnapshot/utility.cpp index 0a1be0d77..cadd24d38 100644 --- a/fs_mgr/libsnapshot/utility.cpp +++ b/fs_mgr/libsnapshot/utility.cpp @@ -153,9 +153,23 @@ AutoUnmountDevice::~AutoUnmountDevice() { } bool WriteStringToFileAtomic(const std::string& content, const std::string& path) { - std::string tmp_path = path + ".tmp"; - if (!android::base::WriteStringToFile(content, tmp_path)) { - return false; + const std::string tmp_path = path + ".tmp"; + { + const int flags = O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC | O_BINARY; + android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(tmp_path.c_str(), flags, 0666))); + if (fd == -1) { + PLOG(ERROR) << "Failed to open " << path; + return false; + } + if (!android::base::WriteStringToFd(content, fd)) { + PLOG(ERROR) << "Failed to write to fd " << fd; + return false; + } + // rename() without fsync() is not safe. Data could still be living on page cache. To ensure + // atomiticity, call fsync() + if (fsync(fd) != 0) { + PLOG(ERROR) << "Failed to fsync " << tmp_path; + } } if (rename(tmp_path.c_str(), path.c_str()) == -1) { PLOG(ERROR) << "rename failed from " << tmp_path << " to " << path;