diff options
| -rw-r--r-- | internal/fusefrontend/file.go | 16 | ||||
| -rw-r--r-- | internal/fusefrontend/file_lock.go | 29 | ||||
| -rw-r--r-- | internal/syscallcompat/sys_darwin.go | 4 | ||||
| -rw-r--r-- | internal/syscallcompat/sys_linux.go | 4 | ||||
| -rw-r--r-- | tests/cluster/cluster_test.go | 75 | ||||
| -rw-r--r-- | tests/cluster/poc_test.go | 47 |
6 files changed, 175 insertions, 0 deletions
diff --git a/internal/fusefrontend/file.go b/internal/fusefrontend/file.go index 64c6ca0..40dce6d 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" @@ -90,6 +92,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 { @@ -266,18 +275,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 52d4e0f..17e6a9e 100644 --- a/internal/syscallcompat/sys_darwin.go +++ b/internal/syscallcompat/sys_darwin.go @@ -33,6 +33,10 @@ const ( // On Darwin we use O_SYMLINK which allows opening a symlink itself. // On Linux, we only have O_NOFOLLOW. OpenatFlagNofollowSymlink = unix.O_SYMLINK + + // F_OFD_SETLKW only exists on Linux. On Darwin, fall back to F_SETLKW as a + // flawed replacement. + F_OFD_SETLKW = unix.F_SETLKW ) // Unfortunately fsetattrlist does not have a syscall wrapper yet. diff --git a/internal/syscallcompat/sys_linux.go b/internal/syscallcompat/sys_linux.go index a850ba1..51a2c09 100644 --- a/internal/syscallcompat/sys_linux.go +++ b/internal/syscallcompat/sys_linux.go @@ -35,6 +35,10 @@ const ( // Only defined on Linux ENODATA = unix.ENODATA + + // F_OFD_SETLKW only exists on Linux. On Darwin, fall back to F_SETLKW as a + // flawed replacement. + 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 af93bc4..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" @@ -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") + } +} |