From e56c2e44cc4dd1484cae78956f8ff72a9eea42f2 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Fri, 2 Jun 2023 14:24:44 +0200 Subject: fusefrontend: sharedstorage: use byte-range lock on file header creation Multiple host writing to the same empty file at the same time could have overwritten each other's newly created file header, leading to data corruption. Fix the race by placing a byte-range lock on the file when creating the file header. --- tests/cluster/cluster_test.go | 75 +++++++++++++++++++++++++++++++++++++++++++ tests/cluster/poc_test.go | 47 +++++++++++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 tests/cluster/poc_test.go (limited to 'tests') 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") + } +} -- cgit v1.2.3