From 237308a671bd2bbea2059bea9e75cb1272edbdbf Mon Sep 17 00:00:00 2001
From: ebiggers <ebiggers@google.com>
Date: Mon, 9 Sep 2019 13:41:32 -0700
Subject: [PATCH] writeDataAtomic() fixes (#140)

* filesystem: ensure data is persisted before returning success

Sync the temporary file before renaming it, to ensure that after a
crash, the destination file isn't zero-length or otherwise incomplete.

Also sync the directory after the rename, to ensure the rename has been
persisted before returning success.

* filesystem: don't use fixed temporary file name

Using a fixed temporary file name in a world-writable sticky directory
is problematic since another user can create the file first.

Use ioutil.TempFile() to do it properly.  It uses O_EXCL under the hood
to ensure the file is newly created.
---
 filesystem/filesystem.go | 37 +++++++++++++++++++++++++++++++------
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/filesystem/filesystem.go b/filesystem/filesystem.go
index f4f9201..66b3804 100644
--- a/filesystem/filesystem.go
+++ b/filesystem/filesystem.go
@@ -250,27 +250,52 @@ func (m *Mount) RemoveAllMetadata() error {
 	return m.err(os.Rename(m.BaseDir(), temp.BaseDir()))
 }
 
+func syncDirectory(dirPath string) error {
+	dirFile, err := os.Open(dirPath)
+	if err != nil {
+		return err
+	}
+	if err = dirFile.Sync(); err != nil {
+		dirFile.Close()
+		return err
+	}
+	return dirFile.Close()
+}
+
 // writeDataAtomic writes the data to the path such that the data is either
 // written to stable storage or an error is returned.
 func (m *Mount) writeDataAtomic(path string, data []byte) error {
-	// Write the file to a temporary file then move into place so that the
-	// operation will be atomic.
-	tempPath := filepath.Join(filepath.Dir(path), tempPrefix+filepath.Base(path))
-	tempFile, err := os.OpenFile(tempPath, os.O_WRONLY|os.O_CREATE, filePermissions)
+	// Write the data to a temporary file, sync it, then rename into place
+	// so that the operation will be atomic.
+	dirPath := filepath.Dir(path)
+	tempFile, err := ioutil.TempFile(dirPath, tempPrefix)
 	if err != nil {
 		return err
 	}
-	defer os.Remove(tempPath)
+	defer os.Remove(tempFile.Name())
 
+	// TempFile() creates the file with mode 0600.  Change it to 0644.
+	if err = tempFile.Chmod(filePermissions); err != nil {
+		tempFile.Close()
+		return err
+	}
 	if _, err = tempFile.Write(data); err != nil {
 		tempFile.Close()
 		return err
 	}
+	if err = tempFile.Sync(); err != nil {
+		tempFile.Close()
+		return err
+	}
 	if err = tempFile.Close(); err != nil {
 		return err
 	}
 
-	return os.Rename(tempPath, path)
+	if err = os.Rename(tempFile.Name(), path); err != nil {
+		return err
+	}
+	// Ensure the rename has been persisted before returning success.
+	return syncDirectory(dirPath)
 }
 
 // addMetadata writes the metadata structure to the file with the specified
-- 
2.39.5