From a5fd293b3981fad0cf2cc0f918e8c864fad2c961 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 8 Feb 2026 21:37:09 +0100 Subject: tests/reverse: add TestXattrGetMountpoint Fails right now: gocryptfs/tests/reverse$ go test -run TestXattrGetMountpoint --- FAIL: TestXattrGetMountpoint (0.00s) xattr_test.go:78: LGet: xattr.get /var/tmp/gocryptfs-test-parent-1026/3269496664/x.3903415542.mnt user.foo453465324: invalid argument xattr_test.go:82: LList: err= FAIL testcases[0] = struct { plaintextnames bool; deterministic_names bool }{plaintextnames:false, deterministic_names:false} failed exit status 1 FAIL github.com/rfjakob/gocryptfs/v2/tests/reverse 0.091s --- tests/reverse/xattr_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'tests') diff --git a/tests/reverse/xattr_test.go b/tests/reverse/xattr_test.go index 406d055..b6a7b34 100644 --- a/tests/reverse/xattr_test.go +++ b/tests/reverse/xattr_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/pkg/xattr" + "golang.org/x/sys/unix" ) func xattrSupported(path string) bool { @@ -65,3 +66,18 @@ func TestXattrList(t *testing.T) { } } } + +// Shouldn't get EINVAL when querying the mountpoint. +func TestXattrGetMountpoint(t *testing.T) { + _, err := xattr.LGet(dirB, "user.foo453465324") + if err == nil { + return + } + e2 := err.(*xattr.Error) + if e2.Unwrap() == unix.EINVAL { + t.Errorf("LGet: %v", err) + } + // Let's see what LList says + _, err = xattr.LList(dirB) + t.Logf("LList: err=%v", err) +} -- cgit v1.2.3 From d22d6b3c69f95ed0034af04851807e7125811b87 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 7 Feb 2026 21:19:14 +0100 Subject: darwin: syscallcompat: Openat: use O_SYMLINK Also add tests that opening a symlink (using unix.O_PATH | unix.O_NOFOLLOW) works. https://github.com/rfjakob/gocryptfs/issues/993 --- internal/fusefrontend/root_node.go | 8 +++--- internal/syscallcompat/sys_common.go | 8 +++--- internal/syscallcompat/sys_darwin.go | 4 +++ internal/syscallcompat/sys_linux.go | 4 +++ tests/matrix/symlink_darwin_test.go | 39 ++++++++++++++++++++++++++++++ tests/matrix/symlink_linux_test.go | 47 ++++++++++++++++++++++++++++++++++++ 6 files changed, 102 insertions(+), 8 deletions(-) create mode 100644 tests/matrix/symlink_darwin_test.go create mode 100644 tests/matrix/symlink_linux_test.go (limited to 'tests') diff --git a/internal/fusefrontend/root_node.go b/internal/fusefrontend/root_node.go index 489e3c6..362290d 100644 --- a/internal/fusefrontend/root_node.go +++ b/internal/fusefrontend/root_node.go @@ -124,11 +124,11 @@ func (rn *RootNode) mangleOpenFlags(flags uint32) (newFlags int) { // accesses. Running xfstests generic/013 on ext4 used to trigger lots of // EINVAL errors due to missing alignment. Just fall back to buffered IO. newFlags = newFlags &^ syscallcompat.O_DIRECT - // Create and Open are two separate FUSE operations, so O_CREAT should not - // be part of the open flags. + // Create and Open are two separate FUSE operations, so O_CREAT should usually not + // be part of the Open() flags. Create() will add O_CREAT back itself. newFlags = newFlags &^ syscall.O_CREAT - // We always want O_NOFOLLOW to be safe against symlink races - newFlags |= syscall.O_NOFOLLOW + // We always want O_NOFOLLOW/O_SYMLINK to be safe against symlink races + newFlags |= syscallcompat.OpenatFlagNofollowSymlink return newFlags } diff --git a/internal/syscallcompat/sys_common.go b/internal/syscallcompat/sys_common.go index 70ee633..3cb9ffa 100644 --- a/internal/syscallcompat/sys_common.go +++ b/internal/syscallcompat/sys_common.go @@ -54,10 +54,10 @@ func Openat(dirfd int, path string, flags int, mode uint32) (fd int, err error) flags |= syscall.O_EXCL } } else { - // If O_CREAT is not used, we should use O_NOFOLLOW - if flags&syscall.O_NOFOLLOW == 0 { - tlog.Warn.Printf("Openat: O_NOFOLLOW missing: flags = %#x", flags) - flags |= syscall.O_NOFOLLOW + // If O_CREAT is not used, we should use O_NOFOLLOW or O_SYMLINK + if flags&(unix.O_NOFOLLOW|OpenatFlagNofollowSymlink) == 0 { + tlog.Warn.Printf("Openat: O_NOFOLLOW/O_SYMLINK missing: flags = %#x", flags) + flags |= unix.O_NOFOLLOW } } diff --git a/internal/syscallcompat/sys_darwin.go b/internal/syscallcompat/sys_darwin.go index 0ebdd3b..ef19f24 100644 --- a/internal/syscallcompat/sys_darwin.go +++ b/internal/syscallcompat/sys_darwin.go @@ -27,6 +27,10 @@ const ( // Only exists on Linux. Define here to fix build failure, even though // we will never see this flag. RENAME_WHITEOUT = 1 << 30 + + // On Darwin we use O_SYMLINK which allows opening a symlink itself. + // On Linux, we only have O_NOFOLLOW. + OpenatFlagNofollowSymlink = unix.O_SYMLINK ) // Unfortunately fsetattrlist does not have a syscall wrapper yet. diff --git a/internal/syscallcompat/sys_linux.go b/internal/syscallcompat/sys_linux.go index 19d2c56..71478af 100644 --- a/internal/syscallcompat/sys_linux.go +++ b/internal/syscallcompat/sys_linux.go @@ -28,6 +28,10 @@ const ( RENAME_NOREPLACE = unix.RENAME_NOREPLACE RENAME_WHITEOUT = unix.RENAME_WHITEOUT RENAME_EXCHANGE = unix.RENAME_EXCHANGE + + // On Darwin we use O_SYMLINK which allows opening a symlink itself. + // On Linux, we only have O_NOFOLLOW. + OpenatFlagNofollowSymlink = unix.O_NOFOLLOW ) var preallocWarn sync.Once diff --git a/tests/matrix/symlink_darwin_test.go b/tests/matrix/symlink_darwin_test.go new file mode 100644 index 0000000..be28d9d --- /dev/null +++ b/tests/matrix/symlink_darwin_test.go @@ -0,0 +1,39 @@ +package matrix + +import ( + "os" + "testing" + + "golang.org/x/sys/unix" + + "github.com/rfjakob/gocryptfs/v2/tests/test_helpers" +) + +// TestOpenSymlinkDarwin checks that a symlink can be opened +// using O_SYMLINK. +func TestOpenSymlinkDarwin(t *testing.T) { + path := test_helpers.DefaultPlainDir + "/TestOpenSymlink" + target := "/target/does/not/exist" + err := os.Symlink(target, path) + if err != nil { + t.Fatal(err) + } + fd, err := unix.Open(path, unix.O_RDONLY|unix.O_SYMLINK, 0) + if err != nil { + t.Fatal(err) + } + defer unix.Close(fd) + var st unix.Stat_t + if err := unix.Fstat(fd, &st); err != nil { + t.Fatal(err) + } + if st.Size != int64(len(target)) { + t.Errorf("wrong size: have=%d want=%d", st.Size, len(target)) + } + if err := unix.Unlink(path); err != nil { + t.Fatal(err) + } + if err := unix.Fstat(fd, &st); err != nil { + t.Error(err) + } +} diff --git a/tests/matrix/symlink_linux_test.go b/tests/matrix/symlink_linux_test.go new file mode 100644 index 0000000..fdb7051 --- /dev/null +++ b/tests/matrix/symlink_linux_test.go @@ -0,0 +1,47 @@ +package matrix + +import ( + "os" + "testing" + + "golang.org/x/sys/unix" + + "github.com/rfjakob/gocryptfs/v2/tests/test_helpers" +) + +// TestOpenSymlinkLinux checks that a symlink can be opened +// using O_PATH. +// Only works on Linux because is uses O_PATH and AT_EMPTY_PATH. +// MacOS has O_SYMLINK instead (see TestOpenSymlinkDarwin). +func TestOpenSymlinkLinux(t *testing.T) { + path := test_helpers.DefaultPlainDir + "/TestOpenSymlink" + target := "/target/does/not/exist" + err := os.Symlink(target, path) + if err != nil { + t.Fatal(err) + } + how := unix.OpenHow{ + Flags: unix.O_PATH | unix.O_NOFOLLOW, + } + fd, err := unix.Openat2(unix.AT_FDCWD, path, &how) + if err != nil { + t.Fatal(err) + } + defer unix.Close(fd) + var st unix.Stat_t + if err := unix.Fstatat(fd, "", &st, unix.AT_EMPTY_PATH); err != nil { + t.Fatal(err) + } + if st.Size != int64(len(target)) { + t.Errorf("wrong size: have=%d want=%d", st.Size, len(target)) + } + if err := unix.Unlink(path); err != nil { + t.Fatal(err) + } + if err = unix.Fstatat(fd, "", &st, unix.AT_EMPTY_PATH); err != nil { + // That's a bug, but I have never heard of a use case that would break because of this. + // Also I don't see how to fix it, as gocryptfs does not get informed about the earlier + // Openat2(). + t.Logf("posix compliance issue: deleted symlink cannot be accessed: Fstatat: %v", err) + } +} -- cgit v1.2.3 From b024e3696d215e6b7a0b8d17ca3eb28af20dd504 Mon Sep 17 00:00:00 2001 From: Jared Van Bortel Date: Fri, 6 Feb 2026 15:16:40 -0500 Subject: enable falloc on btrfs if CoW is disabled --- internal/fusefrontend/root_node.go | 4 ++- internal/syscallcompat/quirks_linux.go | 40 +++++++++++++++++++-- tests/root_test/btrfs_test.go | 63 +++++++++++++++++++++++++++++----- 3 files changed, 95 insertions(+), 12 deletions(-) (limited to 'tests') diff --git a/internal/fusefrontend/root_node.go b/internal/fusefrontend/root_node.go index aa26b9c..38d070d 100644 --- a/internal/fusefrontend/root_node.go +++ b/internal/fusefrontend/root_node.go @@ -92,7 +92,9 @@ func NewRootNode(args Args, c *contentenc.ContentEnc, n *nametransform.NameTrans } // Suppress the message if the user has already specified -noprealloc if rn.quirks&syscallcompat.QuirkBtrfsBrokenFalloc != 0 && !args.NoPrealloc { - syscallcompat.LogQuirk("Btrfs detected, forcing -noprealloc. See https://github.com/rfjakob/gocryptfs/issues/395 for why.") + syscallcompat.LogQuirk("Btrfs detected, forcing -noprealloc. " + + "Use \"chattr +C\" on the backing directory to enable NOCOW and allow preallocation. " + + "See https://github.com/rfjakob/gocryptfs/issues/395 for details.") } if statErr == nil { rn.inoMap.TranslateStat(&st) diff --git a/internal/syscallcompat/quirks_linux.go b/internal/syscallcompat/quirks_linux.go index 87af03d..35f754d 100644 --- a/internal/syscallcompat/quirks_linux.go +++ b/internal/syscallcompat/quirks_linux.go @@ -1,11 +1,38 @@ package syscallcompat import ( + "syscall" + "golang.org/x/sys/unix" "github.com/rfjakob/gocryptfs/v2/internal/tlog" ) +// FS_NOCOW_FL is the flag set by "chattr +C" to disable copy-on-write on +// btrfs. Not exported by golang.org/x/sys/unix, value from linux/fs.h. +const FS_NOCOW_FL = 0x00800000 + +// dirHasNoCow checks whether the directory at the given path has the +// NOCOW (No Copy-on-Write) attribute set (i.e. "chattr +C"). +// When a directory has this attribute, files created within it inherit +// NOCOW, which makes fallocate work correctly on btrfs because writes +// go in-place rather than through COW. +func dirHasNoCow(path string) bool { + fd, err := syscall.Open(path, syscall.O_RDONLY|syscall.O_DIRECTORY, 0) + if err != nil { + tlog.Debug.Printf("dirHasNoCow: Open %q failed: %v", path, err) + return false + } + defer syscall.Close(fd) + + flags, err := unix.IoctlGetInt(fd, unix.FS_IOC_GETFLAGS) + if err != nil { + tlog.Debug.Printf("dirHasNoCow: FS_IOC_GETFLAGS on %q failed: %v", path, err) + return false + } + return flags&FS_NOCOW_FL != 0 +} + // DetectQuirks decides if there are known quirks on the backing filesystem // that need to be workarounded. // @@ -21,10 +48,19 @@ func DetectQuirks(cipherdir string) (q uint64) { // Preallocation on Btrfs is broken ( https://github.com/rfjakob/gocryptfs/issues/395 ) // and slow ( https://github.com/rfjakob/gocryptfs/issues/63 ). // + // The root cause is that btrfs COW allocates new blocks on write even for + // preallocated extents, defeating the purpose of fallocate. However, if the + // backing directory has the NOCOW attribute (chattr +C), writes go in-place + // and fallocate works correctly. + // // Cast to uint32 avoids compile error on arm: "constant 2435016766 overflows int32" if uint32(st.Type) == unix.BTRFS_SUPER_MAGIC { - // LogQuirk is called in fusefrontend/root_node.go - q |= QuirkBtrfsBrokenFalloc + if dirHasNoCow(cipherdir) { + tlog.Debug.Printf("DetectQuirks: Btrfs detected but cipherdir has NOCOW attribute (chattr +C), fallocate should work correctly") + } else { + // LogQuirk is called in fusefrontend/root_node.go + q |= QuirkBtrfsBrokenFalloc + } } return q diff --git a/tests/root_test/btrfs_test.go b/tests/root_test/btrfs_test.go index 72b7751..898cd39 100644 --- a/tests/root_test/btrfs_test.go +++ b/tests/root_test/btrfs_test.go @@ -12,12 +12,20 @@ import ( "github.com/rfjakob/gocryptfs/v2/tests/test_helpers" ) -// TestBtrfsQuirks needs root permissions because it creates a loop disk -func TestBtrfsQuirks(t *testing.T) { +// createBtrfsImage creates a btrfs image file, formats it, and mounts it. +// Returns the mount path and a cleanup function. +func createBtrfsImage(t *testing.T) (mnt string, cleanup func()) { + t.Helper() + if os.Getuid() != 0 { t.Skip("must run as root") } + _, err := exec.LookPath("mkfs.btrfs") + if err != nil { + t.Skip("mkfs.btrfs not found, skipping test") + } + img := filepath.Join(test_helpers.TmpDir, t.Name()+".img") f, err := os.Create(img) if err != nil { @@ -31,10 +39,6 @@ func TestBtrfsQuirks(t *testing.T) { } // Format as Btrfs - _, err = exec.LookPath("mkfs.btrfs") - if err != nil { - t.Skip("mkfs.btrfs not found, skipping test") - } cmd := exec.Command("mkfs.btrfs", img) out, err := cmd.CombinedOutput() if err != nil { @@ -44,7 +48,7 @@ func TestBtrfsQuirks(t *testing.T) { } // Mount - mnt := img + ".mnt" + mnt = img + ".mnt" err = os.Mkdir(mnt, 0600) if err != nil { t.Fatal(err) @@ -55,11 +59,52 @@ func TestBtrfsQuirks(t *testing.T) { t.Log(string(out)) t.Fatal(err) } - defer syscall.Unlink(img) - defer syscall.Unmount(mnt, 0) + + cleanup = func() { + syscall.Unmount(mnt, 0) + syscall.Unlink(img) + } + return mnt, cleanup +} + +// TestBtrfsQuirks needs root permissions because it creates a loop disk +func TestBtrfsQuirks(t *testing.T) { + mnt, cleanup := createBtrfsImage(t) + defer cleanup() quirk := syscallcompat.DetectQuirks(mnt) if quirk != syscallcompat.QuirkBtrfsBrokenFalloc { t.Errorf("wrong quirk: %v", quirk) } } + +// TestBtrfsQuirksNoCow verifies that when the backing directory has +// the NOCOW attribute (chattr +C), the QuirkBtrfsBrokenFalloc quirk +// is NOT set, because fallocate works correctly with NOCOW. +func TestBtrfsQuirksNoCow(t *testing.T) { + mnt, cleanup := createBtrfsImage(t) + defer cleanup() + + _, err := exec.LookPath("chattr") + if err != nil { + t.Skip("chattr not found, skipping test") + } + + // Create a subdirectory with NOCOW attribute + nocowDir := filepath.Join(mnt, "nocow") + err = os.Mkdir(nocowDir, 0700) + if err != nil { + t.Fatal(err) + } + cmd := exec.Command("chattr", "+C", nocowDir) + out, err := cmd.CombinedOutput() + if err != nil { + t.Log(string(out)) + t.Fatal(err) + } + + quirk := syscallcompat.DetectQuirks(nocowDir) + if quirk&syscallcompat.QuirkBtrfsBrokenFalloc != 0 { + t.Errorf("QuirkBtrfsBrokenFalloc should not be set on NOCOW directory, got quirks: %v", quirk) + } +} -- cgit v1.2.3