aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--internal/fusefrontend/file.go16
-rw-r--r--internal/fusefrontend/file_lock.go29
-rw-r--r--internal/syscallcompat/sys_darwin.go4
-rw-r--r--internal/syscallcompat/sys_linux.go1
-rw-r--r--tests/cluster/cluster_test.go79
-rw-r--r--tests/cluster/poc_test.go47
6 files changed, 174 insertions, 2 deletions
diff --git a/internal/fusefrontend/file.go b/internal/fusefrontend/file.go
index 0e25de3..5be0872 100644
--- a/internal/fusefrontend/file.go
+++ b/internal/fusefrontend/file.go
@@ -14,6 +14,8 @@ import (
"sync"
"syscall"
+ "golang.org/x/sys/unix"
+
"github.com/hanwen/go-fuse/v2/fs"
"github.com/hanwen/go-fuse/v2/fuse"
@@ -91,6 +93,13 @@ func (f *File) readFileID() ([]byte, error) {
// and not only the header. A header-only file will be considered empty.
// This makes File ID poisoning more difficult.
readLen := contentenc.HeaderLen + 1
+ if f.rootNode.args.SharedStorage {
+ // With -sharedstorage, we consider a header-only file as valid, because
+ // another gocryptfs process may have either:
+ // 1) just created the header, and not written further data yet.
+ // 2) truncated the file down to just the header.
+ readLen = contentenc.HeaderLen
+ }
buf := make([]byte, readLen)
n, err := f.fd.ReadAt(buf, 0)
if err != nil {
@@ -267,18 +276,25 @@ func (f *File) doWrite(data []byte, off int64) (uint32, syscall.Errno) {
//
// If the file ID is not cached, read it from disk
if f.fileTableEntry.ID == nil {
+ if err := f.LockSharedStorage(unix.F_WRLCK, 0, contentenc.HeaderLen); err != nil {
+ return 0, fs.ToErrno(err)
+ }
var err error
fileID, err := f.readFileID()
// Write a new file header if the file is empty
if err == io.EOF {
fileID, err = f.createHeader()
fileWasEmpty = true
+ // Having the unlock three times is ugly. But every other way I tried is even uglier.
+ f.LockSharedStorage(unix.F_UNLCK, 0, contentenc.HeaderLen)
} else if err != nil {
// Other errors mean readFileID() found a corrupt header
tlog.Warn.Printf("doWrite %d: corrupt header: %v", f.qIno.Ino, err)
+ f.LockSharedStorage(unix.F_UNLCK, 0, contentenc.HeaderLen)
return 0, syscall.EIO
}
if err != nil {
+ f.LockSharedStorage(unix.F_UNLCK, 0, contentenc.HeaderLen)
return 0, fs.ToErrno(err)
}
f.fileTableEntry.ID = fileID
diff --git a/internal/fusefrontend/file_lock.go b/internal/fusefrontend/file_lock.go
new file mode 100644
index 0000000..6f92cfe
--- /dev/null
+++ b/internal/fusefrontend/file_lock.go
@@ -0,0 +1,29 @@
+package fusefrontend
+
+import (
+ "golang.org/x/sys/unix"
+
+ "github.com/rfjakob/gocryptfs/v2/internal/syscallcompat"
+)
+
+// SharedStorageLock conveniently wraps F_OFD_SETLKW
+// See https://man7.org/linux/man-pages/man2/fcntl.2.html -> "Open file description locks (non-POSIX)"
+//
+// lkType is one of:
+// * unix.F_RDLCK (shared read lock)
+// * unix.F_WRLCK (exclusive write lock)
+// * unix.F_UNLCK (unlock)
+//
+// This function is a no-op if args.SharedStorage == false.
+func (f *File) LockSharedStorage(lkType int16, lkStart int64, lkLen int64) error {
+ if !f.rootNode.args.SharedStorage {
+ return nil
+ }
+ lk := unix.Flock_t{
+ Type: lkType,
+ Whence: unix.SEEK_SET,
+ Start: lkStart,
+ Len: lkLen,
+ }
+ return unix.FcntlFlock(uintptr(f.intFd()), syscallcompat.F_OFD_SETLKW, &lk)
+}
diff --git a/internal/syscallcompat/sys_darwin.go b/internal/syscallcompat/sys_darwin.go
index 06f09f0..b13b6a5 100644
--- a/internal/syscallcompat/sys_darwin.go
+++ b/internal/syscallcompat/sys_darwin.go
@@ -27,6 +27,10 @@ const (
RENAME_EXCHANGE = 2
RENAME_WHITEOUT = 4
+ // F_OFD_SETLKW only exists on Linux. On Darwin, fall back to F_SETLKW as a
+ // flawed replacement.
+ F_OFD_SETLKW = unix.F_SETLKW
+
// KAUTH_UID_NONE and KAUTH_GID_NONE are special values to
// revert permissions to the process credentials.
KAUTH_UID_NONE = ^uint32(0) - 100
diff --git a/internal/syscallcompat/sys_linux.go b/internal/syscallcompat/sys_linux.go
index a64b27e..3714b75 100644
--- a/internal/syscallcompat/sys_linux.go
+++ b/internal/syscallcompat/sys_linux.go
@@ -32,6 +32,7 @@ const (
RENAME_NOREPLACE = unix.RENAME_NOREPLACE
RENAME_WHITEOUT = unix.RENAME_WHITEOUT
RENAME_EXCHANGE = unix.RENAME_EXCHANGE
+ F_OFD_SETLKW = unix.F_OFD_SETLKW
)
var preallocWarn sync.Once
diff --git a/tests/cluster/cluster_test.go b/tests/cluster/cluster_test.go
index 2e969ce..f9d0903 100644
--- a/tests/cluster/cluster_test.go
+++ b/tests/cluster/cluster_test.go
@@ -7,9 +7,12 @@ package cluster_test
import (
"bytes"
+ "errors"
+ "io"
"math/rand"
"os"
"sync"
+ "syscall"
"testing"
"github.com/rfjakob/gocryptfs/v2/tests/test_helpers"
@@ -27,8 +30,8 @@ import (
// > buffered read/write.
//
// See also:
-// * https://lore.kernel.org/linux-xfs/20190325001044.GA23020@dastard/
-// Dave Chinner: XFS is the only linux filesystem that provides this behaviour.
+// - https://lore.kernel.org/linux-xfs/20190325001044.GA23020@dastard/
+// Dave Chinner: XFS is the only linux filesystem that provides this behaviour.
func TestClusterConcurrentRW(t *testing.T) {
if os.Getenv("ENABLE_CLUSTER_TEST") != "1" {
t.Skipf("This test is disabled by default because it fails unless on XFS.\n" +
@@ -109,3 +112,75 @@ func TestClusterConcurrentRW(t *testing.T) {
go readThread(f2)
wg.Wait()
}
+
+// Multiple hosts creating the same file at the same time could
+// overwrite each other's file header, leading to data
+// corruption. Passing "-sharedstorage" should prevent this.
+func TestConcurrentCreate(t *testing.T) {
+ cDir := test_helpers.InitFS(t)
+ mnt1 := cDir + ".mnt1"
+ mnt2 := cDir + ".mnt2"
+ test_helpers.MountOrFatal(t, cDir, mnt1, "-extpass=echo test", "-wpanic=0", "-sharedstorage")
+ defer test_helpers.UnmountPanic(mnt1)
+ test_helpers.MountOrFatal(t, cDir, mnt2, "-extpass=echo test", "-wpanic=0", "-sharedstorage")
+ defer test_helpers.UnmountPanic(mnt2)
+
+ var wg sync.WaitGroup
+ const loops = 10000
+
+ createOrOpen := func(path string) (f *os.File, err error) {
+ // Use the high-level os.Create/OpenFile instead of syscall.Open because we
+ // *want* Go's EINTR retry logic. glibc open(2) has similar logic.
+ f, err = os.OpenFile(path, os.O_CREATE|os.O_RDWR|os.O_EXCL, 0600)
+ if err == nil {
+ return
+ }
+ if !errors.Is(err, os.ErrExist) {
+ t.Logf("POSIX compliance issue: exclusive create failed with unexpected error: err=%v", errors.Unwrap(err))
+ }
+ f, err = os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0600)
+ if err == nil {
+ return
+ }
+ t.Logf("POSIX compliance issue: non-exlusive create failed with err=%v", errors.Unwrap(err))
+ return
+ }
+
+ workerThread := func(path string) {
+ defer wg.Done()
+ buf := make([]byte, 10)
+ for i := 0; i < loops; i++ {
+ if t.Failed() {
+ return
+ }
+ f, err := createOrOpen(path)
+ if err != nil {
+ // retry
+ continue
+ }
+ defer f.Close()
+ _, err = f.WriteAt(buf, 0)
+ if err != nil {
+ t.Errorf("iteration %d: Pwrite: %v", i, err)
+ return
+ }
+ buf2 := make([]byte, len(buf)+1)
+ n, err := f.ReadAt(buf2, 0)
+ if err != nil && err != io.EOF {
+ t.Errorf("iteration %d: ReadAt: %v", i, err)
+ return
+ }
+ buf2 = buf2[:n]
+ if !bytes.Equal(buf, buf2) {
+ t.Errorf("iteration %d: corrupt data received: %x", i, buf2)
+ return
+ }
+ syscall.Unlink(path)
+ }
+ }
+
+ wg.Add(2)
+ go workerThread(mnt1 + "/foo")
+ go workerThread(mnt2 + "/foo")
+ wg.Wait()
+}
diff --git a/tests/cluster/poc_test.go b/tests/cluster/poc_test.go
new file mode 100644
index 0000000..4d7182a
--- /dev/null
+++ b/tests/cluster/poc_test.go
@@ -0,0 +1,47 @@
+package cluster
+
+// poc_test.go contains proof of concept tests for the byte-range locking logic.
+// This goes directly to an underlying filesystem without going through gocryptfs.
+
+import (
+ "syscall"
+ "testing"
+
+ "golang.org/x/sys/unix"
+
+ "github.com/rfjakob/gocryptfs/v2/tests/test_helpers"
+)
+
+// Check that byte-range locks work on an empty file
+func TestPoCFcntlFlock(t *testing.T) {
+ path := test_helpers.TmpDir + "/" + t.Name()
+
+ fd1, err := syscall.Open(path, syscall.O_CREAT|syscall.O_WRONLY|syscall.O_EXCL, 0600)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer syscall.Close(fd1)
+
+ // F_OFD_SETLK locks on the same fd always succeed, so we have to
+ // open a 2nd time.
+ fd2, err := syscall.Open(path, syscall.O_RDWR, 0)
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer syscall.Close(fd2)
+
+ lk := unix.Flock_t{
+ Type: unix.F_WRLCK,
+ Whence: unix.SEEK_SET,
+ Start: 0,
+ Len: 0,
+ }
+ err = unix.FcntlFlock(uintptr(fd1), unix.F_OFD_SETLK, &lk)
+ if err != nil {
+ t.Fatal(err)
+ }
+ err = unix.FcntlFlock(uintptr(fd2), unix.F_OFD_SETLK, &lk)
+ if err == nil {
+ t.Fatal("double-lock succeeded but should have failed")
+ }
+}
generated by cgit v1.2.3 (git 2.39.1) at 2024-12-23 17:16:41 +0000