From a5ebd5de4b22de8299d8b025cd2c098ef1eb327e Mon Sep 17 00:00:00 2001
From: Michael Eischer <michael.eischer@fau.de>
Date: Sun, 7 Aug 2022 17:56:14 +0200
Subject: [PATCH] restorer: Fix race condition in partialFile.WriteAt

The restorer can issue multiple calls to WriteAt in parallel. This can
result in unexpected orderings of the Truncate and WriteAt calls and
sometimes too short restored files.
---
 internal/restorer/fileswriter.go | 35 ++++++++++++++++----------------
 internal/restorer/sparsewrite.go | 17 +++-------------
 2 files changed, 20 insertions(+), 32 deletions(-)

diff --git a/internal/restorer/fileswriter.go b/internal/restorer/fileswriter.go
index d7483cd84..47fb5572c 100644
--- a/internal/restorer/fileswriter.go
+++ b/internal/restorer/fileswriter.go
@@ -24,8 +24,7 @@ type filesWriterBucket struct {
 
 type partialFile struct {
 	*os.File
-	size   int64 // File size, tracked for sparse writes (not on Windows).
-	users  int   // Reference count.
+	users  int // Reference count.
 	sparse bool
 }
 
@@ -64,24 +63,24 @@ func (w *filesWriter) writeToFile(path string, blob []byte, offset int64, create
 		}
 
 		wr := &partialFile{File: f, users: 1, sparse: sparse}
-		if createSize < 0 {
-			info, err := f.Stat()
-			if err != nil {
-				return nil, err
-			}
-			wr.size = info.Size()
-		}
 		bucket.files[path] = wr
 
-		if createSize >= 0 && !sparse {
-			err := preallocateFile(wr.File, createSize)
-			if err != nil {
-				// Just log the preallocate error but don't let it cause the restore process to fail.
-				// Preallocate might return an error if the filesystem (implementation) does not
-				// support preallocation or our parameters combination to the preallocate call
-				// This should yield a syscall.ENOTSUP error, but some other errors might also
-				// show up.
-				debug.Log("Failed to preallocate %v with size %v: %v", path, createSize, err)
+		if createSize >= 0 {
+			if sparse {
+				err = f.Truncate(createSize)
+				if err != nil {
+					return nil, err
+				}
+			} else {
+				err := preallocateFile(wr.File, createSize)
+				if err != nil {
+					// Just log the preallocate error but don't let it cause the restore process to fail.
+					// Preallocate might return an error if the filesystem (implementation) does not
+					// support preallocation or our parameters combination to the preallocate call
+					// This should yield a syscall.ENOTSUP error, but some other errors might also
+					// show up.
+					debug.Log("Failed to preallocate %v with size %v: %v", path, createSize, err)
+				}
 			}
 		}
 
diff --git a/internal/restorer/sparsewrite.go b/internal/restorer/sparsewrite.go
index dec95d784..9dec4bfa3 100644
--- a/internal/restorer/sparsewrite.go
+++ b/internal/restorer/sparsewrite.go
@@ -13,7 +13,6 @@ func (f *partialFile) WriteAt(p []byte, offset int64) (n int, err error) {
 	}
 
 	n = len(p)
-	end := offset + int64(n)
 
 	// Skip the longest all-zero prefix of p.
 	// If it's long enough, we can punch a hole in the file.
@@ -22,26 +21,16 @@ func (f *partialFile) WriteAt(p []byte, offset int64) (n int, err error) {
 	offset += int64(skipped)
 
 	switch {
-	case len(p) == 0 && end > f.size:
-		// We need to do a Truncate, as WriteAt with length-0 input
-		// doesn't actually extend the file.
-		err = f.Truncate(end)
-		if err != nil {
-			return 0, err
-		}
-
 	case len(p) == 0:
 		// All zeros, file already big enough. A previous WriteAt or
 		// Truncate will have produced the zeros in f.File.
 
 	default:
-		n, err = f.File.WriteAt(p, offset)
+		var n2 int
+		n2, err = f.File.WriteAt(p, offset)
+		n = skipped + n2
 	}
 
-	end = offset + int64(n)
-	if end > f.size {
-		f.size = end
-	}
 	return n, err
 }