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 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'internal/fusefrontend/root_node.go') 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 } -- cgit v1.2.3 From 8936a544a062d8ba56e050d45f30fc2d3b9d9d09 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 8 Feb 2026 20:25:47 +0100 Subject: fusefrontend: convert mangleOpenFlags method to a function No need to attach it to the root node. Also rename it to mangleOpenCreateFlags. --- internal/fusefrontend/node_open_create.go | 29 +++++++++++++++++++++++++++-- internal/fusefrontend/root_node.go | 25 ------------------------- 2 files changed, 27 insertions(+), 27 deletions(-) (limited to 'internal/fusefrontend/root_node.go') diff --git a/internal/fusefrontend/node_open_create.go b/internal/fusefrontend/node_open_create.go index 9598559..622d5dc 100644 --- a/internal/fusefrontend/node_open_create.go +++ b/internal/fusefrontend/node_open_create.go @@ -2,6 +2,7 @@ package fusefrontend import ( "context" + "os" "syscall" "github.com/hanwen/go-fuse/v2/fs" @@ -12,6 +13,30 @@ import ( "github.com/rfjakob/gocryptfs/v2/internal/tlog" ) +// mangleOpenCreateFlags is used by Create() and Open() to convert the open flags the user +// wants to the flags we internally use to open the backing file using Openat(). +// The returned flags always contain O_NOFOLLOW/O_SYMLINK. +func mangleOpenCreateFlags(flags uint32) (newFlags int) { + newFlags = int(flags) + // Convert WRONLY to RDWR. We always need read access to do read-modify-write cycles. + if (newFlags & syscall.O_ACCMODE) == syscall.O_WRONLY { + newFlags = newFlags ^ os.O_WRONLY | os.O_RDWR + } + // We also cannot open the file in append mode, we need to seek back for RMW + newFlags = newFlags &^ os.O_APPEND + // O_DIRECT accesses must be aligned in both offset and length. Due to our + // crypto header, alignment will be off, even if userspace makes aligned + // 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 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/O_SYMLINK to be safe against symlink races + newFlags |= syscallcompat.OpenatFlagNofollowSymlink + return newFlags +} + // Open - FUSE call. Open already-existing file. // // Symlink-safe through Openat(). @@ -23,7 +48,7 @@ func (n *Node) Open(ctx context.Context, flags uint32) (fh fs.FileHandle, fuseFl defer syscall.Close(dirfd) rn := n.rootNode() - newFlags := rn.mangleOpenFlags(flags) + newFlags := mangleOpenCreateFlags(flags) // Taking this lock makes sure we don't race openWriteOnlyFile() rn.openWriteOnlyLock.RLock() defer rn.openWriteOnlyLock.RUnlock() @@ -71,7 +96,7 @@ func (n *Node) Create(ctx context.Context, name string, flags uint32, mode uint3 if !rn.args.PreserveOwner { ctx = nil } - newFlags := rn.mangleOpenFlags(flags) + newFlags := mangleOpenCreateFlags(flags) // Handle long file name ctx2 := toFuseCtx(ctx) if !rn.args.PlaintextNames && nametransform.IsLongContent(cName) { diff --git a/internal/fusefrontend/root_node.go b/internal/fusefrontend/root_node.go index 362290d..aa26b9c 100644 --- a/internal/fusefrontend/root_node.go +++ b/internal/fusefrontend/root_node.go @@ -1,7 +1,6 @@ package fusefrontend import ( - "os" "strings" "sync" "sync/atomic" @@ -108,30 +107,6 @@ func (rn *RootNode) AfterUnmount() { rn.dirCache.stats() } -// mangleOpenFlags is used by Create() and Open() to convert the open flags the user -// wants to the flags we internally use to open the backing file. -// The returned flags always contain O_NOFOLLOW. -func (rn *RootNode) mangleOpenFlags(flags uint32) (newFlags int) { - newFlags = int(flags) - // Convert WRONLY to RDWR. We always need read access to do read-modify-write cycles. - if (newFlags & syscall.O_ACCMODE) == syscall.O_WRONLY { - newFlags = newFlags ^ os.O_WRONLY | os.O_RDWR - } - // We also cannot open the file in append mode, we need to seek back for RMW - newFlags = newFlags &^ os.O_APPEND - // O_DIRECT accesses must be aligned in both offset and length. Due to our - // crypto header, alignment will be off, even if userspace makes aligned - // 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 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/O_SYMLINK to be safe against symlink races - newFlags |= syscallcompat.OpenatFlagNofollowSymlink - return newFlags -} - // reportMitigatedCorruption is used to report a corruption that was transparently // mitigated and did not return an error to the user. Pass the name of the corrupt // item (filename for OpenDir(), xattr name for ListXAttr() etc). -- 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 'internal/fusefrontend/root_node.go') 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