fusefrontend: take ContentLock for truncate through node.SetattrHEADmaster

node.Setattr opens its own file handle and called truncate directly,
without ContentLock. truncate ends in doWrite, which documents that the
caller holds that lock, and file.Setattr and Allocate both take it.

Besides leaving a path truncate unserialized against concurrent writes,
this skipped the write-operation counter that ContentLock.Lock()
increments. isConsecutiveWrite() reads that counter to notice foreign
modifications, so an already-open handle kept assuming its next write
appends, skipped writePadHole() and grew the file past a short last
block. That block no longer decrypted, and every later operation on the
file failed with EIO.

Fixes #1024

3 files changed, 39 insertions, 0 deletions

diff --git a/internal/fusefrontend/file_allocate_truncate.go b/internal/fusefrontend/file_allocate_truncate.go
index bfd11e1..f4a078c 100644
--- a/internal/fusefrontend/file_allocate_truncate.go
+++ b/internal/fusefrontend/file_allocate_truncate.go
@@ -97,6 +97,8 @@ func (f *File) Allocate(ctx context.Context, off uint64, sz uint64, mode uint32)
 }
 
 // truncate - called from node.Setattr and file.Setattr.
+//
+// The caller must hold f.fileTableEntry.ContentLock
 func (f *File) truncate(newSize uint64) (errno syscall.Errno) {
 	var err error
 	// Common case first: Truncate to zero
diff --git a/internal/fusefrontend/node.go b/internal/fusefrontend/node.go
index 28ebbd5..59f6b2f 100644
--- a/internal/fusefrontend/node.go
+++ b/internal/fusefrontend/node.go
@@ -246,6 +246,8 @@ func (n *Node) Setattr(ctx context.Context, f fs.FileHandle, in *fuse.SetAttrIn,
 		}
 		f2 := f.(*File)
 		defer f2.Release(ctx)
+		f2.fileTableEntry.ContentLock.Lock()
+		defer f2.fileTableEntry.ContentLock.Unlock()
 		errno = syscall.Errno(f2.truncate(sz))
 		if errno != 0 {
 			return errno
diff --git a/tests/defaults/main_test.go b/tests/defaults/main_test.go
index a19f079..237dbd1 100644
--- a/tests/defaults/main_test.go
+++ b/tests/defaults/main_test.go
@@ -554,3 +554,38 @@ func TestSeekDir(t *testing.T) {
 		t.Error("Seek did not have any effect")
 	}
 }
+
+// Regression test for https://github.com/rfjakob/gocryptfs/issues/1024
+//
+// truncate(2) goes through node.Setattr, which opens its own file handle. That
+// handle must take ContentLock like file.Setattr does: the lock doubles as the
+// global write-operation counter that isConsecutiveWrite() uses to notice that
+// somebody else changed the file. Without it, an already-open handle keeps
+// believing its next write appends, skips writePadHole(), and leaves the last
+// block short while the file grows past it - the block then fails to decrypt.
+func TestConcurrentTruncateViaPath(t *testing.T) {
+	path := test_helpers.DefaultPlainDir + "/" + t.Name()
+	f, err := os.Create(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f.Close()
+
+	b := make([]byte, 4096)
+	_, err = f.Write(b)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Truncate via path used to not increment writeOpCount...
+	if err = os.Truncate(path, 8); err != nil {
+		t.Fatal(err)
+	}
+	// ...which means this write will not call writePadHole.
+	if _, err = f.Write([]byte("foo")); err != nil {
+		t.Fatal(err)
+	}
+	// First block is corrupt now.
+	if _, err = f.ReadAt(b, 0); err != nil {
+		t.Fatal(err)
+	}
+}