commit 78d5352289ad07487d7bac77341c5ba582b46072
author Kelvin Zhang <zhangkelvin@google.com> Thu Oct 12 10:16:44 2023 -0700
committer Kelvin Zhang <zhangkelvin@google.com> Fri Oct 13 15:46:28 2023 -0700
tree 0c6c95df123e5c055b70ae527ae152990c3591b6
parent c519d1dde2967c337d256a3d29f1d60c4a57237f

Make partition metadata write atomic

We have seen multiple OTA failures with invalid geometry magic
signature. Make partition metadata write atomic by writing to a tmpfile
first and then do a rename.

Test: th
Bug: 303770065
Bug: 298149189
Change-Id: Id1d565de73439b95b665144c2f02fc97273d341c

fs_mgr/libfiemap/metadata.cpp

diff --git a/fs_mgr/libfiemap/metadata.cpp b/fs_mgr/libfiemap/metadata.cpp
index 22b8afb..0a56f6a 100644
--- a/fs_mgr/libfiemap/metadata.cpp
+++ b/fs_mgr/libfiemap/metadata.cpp
@@ -111,13 +111,7 @@
         return true;
     }
 
-    unique_fd fd(open(metadata_file.c_str(), O_CREAT | O_RDWR | O_TRUNC | O_CLOEXEC | O_BINARY | O_SYNC, 0644));
-    if (fd < 0) {
-        LOG(ERROR) << "open failed: " << metadata_file;
-        return false;
-    }
-
-    if (!WriteToImageFile(fd, *exported.get())) {
+    if (!WriteToImageFile(metadata_file, *exported.get())) {
         LOG(ERROR) << "Unable to save new metadata";
         return false;
     }

fs_mgr/liblp/images.cpp

diff --git a/fs_mgr/liblp/images.cpp b/fs_mgr/liblp/images.cpp
index 02b64ac..a2dbb10 100644
--- a/fs_mgr/liblp/images.cpp
+++ b/fs_mgr/liblp/images.cpp
@@ -123,13 +123,46 @@
     return true;
 }
 
-bool WriteToImageFile(const std::string& file, const LpMetadata& input) {
-    unique_fd fd(open(file.c_str(), O_CREAT | O_RDWR | O_TRUNC | O_CLOEXEC | O_BINARY, 0644));
-    if (fd < 0) {
-        PERROR << __PRETTY_FUNCTION__ << " open failed: " << file;
+#if !defined(_WIN32)
+bool FsyncDirectory(const char* dirname) {
+    android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(dirname, O_RDONLY | O_CLOEXEC)));
+    if (fd == -1) {
+        PLOG(ERROR) << "Failed to open " << dirname;
         return false;
     }
-    return WriteToImageFile(fd, input);
+    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;
+}
+#endif
+
+bool WriteToImageFile(const std::string& file, const LpMetadata& input) {
+    const auto parent_dir = base::Dirname(file);
+    TemporaryFile tmpfile(parent_dir);
+    if (!WriteToImageFile(tmpfile.fd, input)) {
+        PLOG(ERROR) << "Failed to write geometry data to tmpfile " << tmpfile.path;
+        return false;
+    }
+
+#if !defined(_WIN32)
+    fsync(tmpfile.fd);
+#endif
+    const auto err = rename(tmpfile.path, file.c_str());
+    if (err != 0) {
+        PLOG(ERROR) << "Failed to rename tmp geometry file " << tmpfile.path << " to " << file;
+        return false;
+    }
+#if !defined(_WIN32)
+    FsyncDirectory(parent_dir.c_str());
+#endif
+    return true;
 }
 
 ImageBuilder::ImageBuilder(const LpMetadata& metadata, uint32_t block_size,
@@ -208,7 +241,8 @@
         std::string file_name = "super_" + name + ".img";
         std::string file_path = output_dir + "/" + file_name;
 
-        static const int kOpenFlags = O_CREAT | O_RDWR | O_TRUNC | O_CLOEXEC | O_NOFOLLOW | O_BINARY;
+        static const int kOpenFlags =
+                O_CREAT | O_RDWR | O_TRUNC | O_CLOEXEC | O_NOFOLLOW | O_BINARY;
         unique_fd fd(open(file_path.c_str(), kOpenFlags, 0644));
         if (fd < 0) {
             PERROR << "open failed: " << file_path;