From 6c14d25d442a819bf37e228d936e6a2a05de747d Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Wed, 3 May 2023 20:56:36 +0200
Subject: [PATCH 01/15] tests: TestParseCliOpts: de-uglify testcase list

---
 cli_args_test.go | 70 +++++++++++++++++++++++-------------------------
 1 file changed, 34 insertions(+), 36 deletions(-)

diff --git a/cli_args_test.go b/cli_args_test.go
index 74255f2..3220196 100644
--- a/cli_args_test.go
+++ b/cli_args_test.go
@@ -131,48 +131,46 @@ func TestParseCliOpts(t *testing.T) {
 		o argContainer
 	}
 
-	var testcases []testcaseContainer
-
-	testcases = append(testcases, testcaseContainer{
-		i: []string{"gocryptfs"},
-		o: defaultArgs,
-	})
+	testcases := []testcaseContainer{
+		{
+			i: []string{"gocryptfs"},
+			o: defaultArgs,
+		},
+	}
 
 	o := defaultArgs
 	o.quiet = true
-	testcases = append(testcases, testcaseContainer{
-		i: []string{"gocryptfs", "-q"},
-		o: o,
-	})
-	testcases = append(testcases, testcaseContainer{
-		i: []string{"gocryptfs", "--q"},
-		o: o,
-	})
-	testcases = append(testcases, testcaseContainer{
-		i: []string{"gocryptfs", "-quiet"},
-		o: o,
-	})
-	testcases = append(testcases, testcaseContainer{
-		i: []string{"gocryptfs", "--quiet"},
-		o: o,
-	})
+	testcases = append(testcases, []testcaseContainer{
+		{
+			i: []string{"gocryptfs", "-q"},
+			o: o,
+		}, {
+			i: []string{"gocryptfs", "--q"},
+			o: o,
+		}, {
+			i: []string{"gocryptfs", "-quiet"},
+			o: o,
+		}, {
+			i: []string{"gocryptfs", "--quiet"},
+			o: o,
+		},
+	}...)
 
 	o = defaultArgs
 	o.exclude = []string{"foo", "bar"}
-	testcases = append(testcases, testcaseContainer{
-		i: []string{"gocryptfs", "-e", "foo", "-e", "bar"},
-		o: o,
-	})
-	testcases = append(testcases, testcaseContainer{
-		i: []string{"gocryptfs", "--exclude", "foo", "--exclude", "bar"},
-		o: o,
-	})
-	/* TODO BROKEN
-	testcases = append(testcases, testcaseContainer{
-		i: []string{"gocryptfs", "--exclude", "foo", "-e", "bar"},
-		o: o,
-	})
-	*/
+	testcases = append(testcases, []testcaseContainer{
+		{
+			i: []string{"gocryptfs", "-e", "foo", "-e", "bar"},
+			o: o,
+		}, {
+			i: []string{"gocryptfs", "--exclude", "foo", "--exclude", "bar"},
+			o: o,
+		}, /* TODO BROKEN {
+			i: []string{"gocryptfs", "--exclude", "foo", "-e", "bar"},
+			o: o,
+		},*/
+	}...)
+
 	for _, tc := range testcases {
 		o := parseCliOpts(tc.i)
 		if !reflect.DeepEqual(o, tc.o) {

From aa1d8a0f90a1046b89dfdd9e58fb1407c76ff27e Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Wed, 3 May 2023 21:14:53 +0200
Subject: [PATCH 02/15] cli: don't split multiple-strings flags on comma

Looks like I used StringSliceVar (which splits on comma)
where I should have always used StringArrayVar (which does not).

Bug report contains this example of misbehavoir:

	#gocryptfs -extpass 'echo abc,123' -init testdir
	Reading password from extpass program "echo abc", arguments: ["123"]
	extpass cmd start failed: exec: "echo abc": executable file not found in $PATH

Fixes https://github.com/rfjakob/gocryptfs/issues/730
---
 cli_args.go      | 16 ++++++++--------
 cli_args_test.go |  6 +++---
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/cli_args.go b/cli_args.go
index 91bd56e..48f1cdd 100644
--- a/cli_args.go
+++ b/cli_args.go
@@ -210,16 +210,16 @@ func parseCliOpts(osArgs []string) (args argContainer) {
 	flagSet.StringVar(&args.fido2, "fido2", "", "Protect the masterkey using a FIDO2 token instead of a password")
 
 	// Exclusion options
-	flagSet.StringSliceVar(&args.exclude, "e", nil, "Alias for -exclude")
-	flagSet.StringSliceVar(&args.exclude, "exclude", nil, "Exclude relative path from reverse view")
-	flagSet.StringSliceVar(&args.excludeWildcard, "ew", nil, "Alias for -exclude-wildcard")
-	flagSet.StringSliceVar(&args.excludeWildcard, "exclude-wildcard", nil, "Exclude path from reverse view, supporting wildcards")
-	flagSet.StringSliceVar(&args.excludeFrom, "exclude-from", nil, "File from which to read exclusion patterns (with -exclude-wildcard syntax)")
+	flagSet.StringArrayVar(&args.exclude, "e", nil, "Alias for -exclude")
+	flagSet.StringArrayVar(&args.exclude, "exclude", nil, "Exclude relative path from reverse view")
+	flagSet.StringArrayVar(&args.excludeWildcard, "ew", nil, "Alias for -exclude-wildcard")
+	flagSet.StringArrayVar(&args.excludeWildcard, "exclude-wildcard", nil, "Exclude path from reverse view, supporting wildcards")
+	flagSet.StringArrayVar(&args.excludeFrom, "exclude-from", nil, "File from which to read exclusion patterns (with -exclude-wildcard syntax)")
 
 	// multipleStrings options ([]string)
-	flagSet.StringSliceVar(&args.extpass, "extpass", nil, "Use external program for the password prompt")
-	flagSet.StringSliceVar(&args.badname, "badname", nil, "Glob pattern invalid file names that should be shown")
-	flagSet.StringSliceVar(&args.passfile, "passfile", nil, "Read password from file")
+	flagSet.StringArrayVar(&args.extpass, "extpass", nil, "Use external program for the password prompt")
+	flagSet.StringArrayVar(&args.badname, "badname", nil, "Glob pattern invalid file names that should be shown")
+	flagSet.StringArrayVar(&args.passfile, "passfile", nil, "Read password from file")
 
 	flagSet.Uint8Var(&args.longnamemax, "longnamemax", 255, "Hash encrypted names that are longer than this")
 
diff --git a/cli_args_test.go b/cli_args_test.go
index 3220196..4fb01ac 100644
--- a/cli_args_test.go
+++ b/cli_args_test.go
@@ -157,13 +157,13 @@ func TestParseCliOpts(t *testing.T) {
 	}...)
 
 	o = defaultArgs
-	o.exclude = []string{"foo", "bar"}
+	o.exclude = []string{"foo", "bar", "baz,boe"}
 	testcases = append(testcases, []testcaseContainer{
 		{
-			i: []string{"gocryptfs", "-e", "foo", "-e", "bar"},
+			i: []string{"gocryptfs", "-e", "foo", "-e", "bar", "-e", "baz,boe"},
 			o: o,
 		}, {
-			i: []string{"gocryptfs", "--exclude", "foo", "--exclude", "bar"},
+			i: []string{"gocryptfs", "--exclude", "foo", "--exclude", "bar", "--exclude", "baz,boe"},
 			o: o,
 		}, /* TODO BROKEN {
 			i: []string{"gocryptfs", "--exclude", "foo", "-e", "bar"},

From 1a866b73731ce58b0ba31d80a2a84c84737c7d30 Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Fri, 12 May 2023 09:55:54 +0200
Subject: [PATCH 03/15] canonical-benchmarks.bash: drop page cache of "zero"
 file

For the streaming read benchmark, we don't want to benchmark
the page cache.
---
 tests/canonical-benchmarks.bash | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tests/canonical-benchmarks.bash b/tests/canonical-benchmarks.bash
index 195effe..4c1a357 100755
--- a/tests/canonical-benchmarks.bash
+++ b/tests/canonical-benchmarks.bash
@@ -35,7 +35,13 @@ function etime {
 }
 
 echo -n "WRITE: "
-dd if=/dev/zero of=zero bs=131072 count=2000 2>&1 | tail -n 1
+dd if=/dev/zero of=zero bs=131072 count=2000 conv=fsync 2>&1 | tail -n 1
+
+# Drop cache of file "zero", otherwise we are benchmarking the
+# page cache. Borrowed from
+# https://www.gnu.org/software/coreutils/manual/html_node/dd-invocation.html#index-nocache
+dd if=zero iflag=nocache count=0 status=none
+
 sleep 0.1
 echo -n "READ:  "
 dd if=zero of=/dev/null bs=131072 count=2000 2>&1 | tail -n 1

From 76d0f3ca7c491b39197e1a69c52b63c39455acfc Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Wed, 17 May 2023 15:20:20 +0200
Subject: [PATCH 04/15] tests: root_test: use TMPDIR=/var/tmp

Otherwise we fail like this on my Fedora 38 box:

	=== RUN   TestOverlay
	DetectQuirks: tmpfs detected, no extended attributes except acls will work.
	    root_test.go:379: No user xattrs! overlay mount will likely fail.
	15:15:57.957960 Unimplemented opcode OPCODE-51
	    root_test.go:398: mount: /tmp/gocryptfs-test-parent-0/3652394902/TestOverlay.2374697046.mnt/merged: wrong fs type, bad option, bad superblock on overlay, missing codepage or helper program, or other error.
	               dmesg(1) may have more information after failed mount system call.

	    root_test.go:399: exit status 32
	--- FAIL: TestOverlay (0.04s)
	FAIL

Also fix the messed-up DetectQuirks bit test.
---
 Makefile                     | 3 ++-
 tests/root_test/root_test.go | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index d69894b..8cf583a 100644
--- a/Makefile
+++ b/Makefile
@@ -10,7 +10,8 @@ test:
 .phony: root_test
 root_test:
 	./build.bash
-	cd tests/root_test && go test -c && sudo ./root_test.test -test.v
+	# Need to use TMPDIR=/var/tmp as TestOverlay fails on tmpfs.
+	cd tests/root_test && go test -c && sudo TMPDIR=/var/tmp ./root_test.test -test.v
 
 .phony: format
 format:
diff --git a/tests/root_test/root_test.go b/tests/root_test/root_test.go
index c4ed7db..23b44d0 100644
--- a/tests/root_test/root_test.go
+++ b/tests/root_test/root_test.go
@@ -375,7 +375,7 @@ func TestOverlay(t *testing.T) {
 		t.Skip("must run as root")
 	}
 	cDir := test_helpers.InitFS(t)
-	if syscallcompat.DetectQuirks(cDir)|syscallcompat.QuirkNoUserXattr != 0 {
+	if syscallcompat.DetectQuirks(cDir)&syscallcompat.QuirkNoUserXattr != 0 {
 		t.Logf("No user xattrs! overlay mount will likely fail.")
 	}
 	os.Chmod(cDir, 0755)

From d7a3d7b97d0879853afa38cacf56e9582ad2a59d Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Wed, 17 May 2023 15:31:52 +0200
Subject: [PATCH 05/15] tests: add TestDirectMount

This is in preparation of adding directmount capability.

It also check that FsName is set correctly, which is
in preparation for the next patch.
---
 Makefile                      |  5 ++-
 tests/cli/directmount_test.go | 61 +++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 1 deletion(-)
 create mode 100644 tests/cli/directmount_test.go

diff --git a/Makefile b/Makefile
index 8cf583a..beccc5f 100644
--- a/Makefile
+++ b/Makefile
@@ -10,9 +10,12 @@ test:
 .phony: root_test
 root_test:
 	./build.bash
-	# Need to use TMPDIR=/var/tmp as TestOverlay fails on tmpfs.
+
+# Need to use TMPDIR=/var/tmp as TestOverlay fails on tmpfs.
 	cd tests/root_test && go test -c && sudo TMPDIR=/var/tmp ./root_test.test -test.v
 
+	cd tests/cli && go test -c && sudo ./cli.test -test.v -test.run=TestDirectMount
+
 .phony: format
 format:
 	go fmt ./...
diff --git a/tests/cli/directmount_test.go b/tests/cli/directmount_test.go
new file mode 100644
index 0000000..da78039
--- /dev/null
+++ b/tests/cli/directmount_test.go
@@ -0,0 +1,61 @@
+package cli
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/moby/sys/mountinfo"
+
+	"github.com/rfjakob/gocryptfs/v2/tests/test_helpers"
+)
+
+// TestDirectMount checks that the effective mount options are what we expect.
+//
+// This test should be run twice:
+// 1) As a normal user (uses fusermount):          make test
+// 2) As root (mount syscall is called directly):  make root_test
+func TestDirectMount(t *testing.T) {
+	type testCase struct {
+		allow_other bool
+	}
+	table := []testCase{
+		{allow_other: false},
+		{allow_other: true},
+	}
+
+	dir := test_helpers.InitFS(t)
+	mnt := dir + ".mnt"
+
+	doTestMountInfo := func(t *testing.T, row testCase) {
+		test_helpers.MountOrFatal(t, dir, mnt, "-extpass=echo test", fmt.Sprintf("-allow_other=%v", row.allow_other))
+		defer test_helpers.UnmountErr(mnt)
+
+		mounts, err := mountinfo.GetMounts(mountinfo.SingleEntryFilter(mnt))
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(mounts) != 1 {
+			t.Fatalf("Could not find mountpoint %q in /proc/self/mountinfo", mnt)
+		}
+		info := mounts[0]
+
+		if info.FSType != "fuse.gocryptfs" {
+			t.Errorf("wrong FSType: %q", info.FSType)
+		}
+		if info.Source != dir {
+			t.Errorf("wrong Source: have %q, want %q", info.Source, dir)
+		}
+		if !strings.Contains(info.VFSOptions, "max_read=") {
+			t.Errorf("VFSOptions is missing max_read")
+		}
+		if row.allow_other && !strings.Contains(info.VFSOptions, "allow_other") {
+			t.Errorf("VFSOptions is missing allow_other")
+		}
+	}
+
+	for _, row := range table {
+		doTestMountInfo(t, row)
+	}
+
+}

From 199a74bc1ae49cdda486095b8daa8034510943a6 Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Wed, 17 May 2023 15:35:01 +0200
Subject: [PATCH 06/15] mount: set FsName via go-fuse

go-fuse now handles setting FsName, including DirectMount,
so use that instead of our own solution.

Regression-tested in TestDirectMount.
---
 mount.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mount.go b/mount.go
index 4d72778..5cd90a7 100644
--- a/mount.go
+++ b/mount.go
@@ -427,7 +427,7 @@ func initGoFuse(rootNode fs.InodeEmbedder, args *argContainer) *fuse.Server {
 		tlog.Warn.Printf("Warning: %q will be displayed as %q in \"df -T\"", fsname, fsname2)
 		fsname = fsname2
 	}
-	mOpts.Options = append(mOpts.Options, "fsname="+fsname)
+	mOpts.FsName = fsname
 	// Second column, "Type", will be shown as "fuse." + Name
 	mOpts.Name = "gocryptfs"
 	if args.reverse {

From b4defa636b901d992165e2a828872dd410c48292 Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Wed, 17 May 2023 15:37:11 +0200
Subject: [PATCH 07/15] mount: drop "max_read="

go-fuse now sets this internally.

Regression-tested in TestDirectMount.
---
 mount.go | 2 --
 1 file changed, 2 deletions(-)

diff --git a/mount.go b/mount.go
index 5cd90a7..a229245 100644
--- a/mount.go
+++ b/mount.go
@@ -2,7 +2,6 @@ package main
 
 import (
 	"bytes"
-	"fmt"
 	"log"
 	"log/syslog"
 	"math"
@@ -388,7 +387,6 @@ func initGoFuse(rootNode fs.InodeEmbedder, args *argContainer) *fuse.Server {
 		// have it >128kiB. We cannot handle more than 128kiB, so we tell
 		// the kernel to limit the size explicitly.
 		MaxWrite: fuse.MAX_KERNEL_WRITE,
-		Options:  []string{fmt.Sprintf("max_read=%d", fuse.MAX_KERNEL_WRITE)},
 		Debug:    args.fusedebug,
 		// The kernel usually submits multiple read requests in parallel,
 		// which means we serve them in any order. Out-of-order reads are

From 8d3b992824072a3b487214812655a35fd28ee4dc Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Wed, 17 May 2023 16:08:49 +0200
Subject: [PATCH 08/15] tests: TestDirectMount: also check dev, suid

---
 tests/cli/directmount_test.go | 42 +++++++++++++++++++++++++++++------
 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/tests/cli/directmount_test.go b/tests/cli/directmount_test.go
index da78039..3e23e6a 100644
--- a/tests/cli/directmount_test.go
+++ b/tests/cli/directmount_test.go
@@ -2,6 +2,7 @@ package cli
 
 import (
 	"fmt"
+	"os"
 	"strings"
 	"testing"
 
@@ -18,17 +19,42 @@ import (
 func TestDirectMount(t *testing.T) {
 	type testCase struct {
 		allow_other bool
+		noexec      bool
+		suid        bool
+		dev         bool
 	}
 	table := []testCase{
-		{allow_other: false},
+		{ /* all false */ },
 		{allow_other: true},
+		{noexec: true},
+		{suid: true},
+		{dev: true},
 	}
 
 	dir := test_helpers.InitFS(t)
 	mnt := dir + ".mnt"
 
+	checkOptionPresent := func(t *testing.T, opts string, option string, want bool) {
+		split := strings.Split(opts, ",")
+		have := false
+		for _, v := range split {
+			if strings.HasPrefix(v, option) {
+				have = true
+				break
+			}
+		}
+		if want != have {
+			t.Errorf("checkOptionPresent: %s: want=%v have=%v. Full string: %s", option, want, have, opts)
+		}
+	}
+
 	doTestMountInfo := func(t *testing.T, row testCase) {
-		test_helpers.MountOrFatal(t, dir, mnt, "-extpass=echo test", fmt.Sprintf("-allow_other=%v", row.allow_other))
+		test_helpers.MountOrFatal(t, dir, mnt,
+			"-extpass=echo test",
+			fmt.Sprintf("-allow_other=%v", row.allow_other),
+			fmt.Sprintf("-noexec=%v", row.noexec),
+			fmt.Sprintf("-dev=%v", row.dev),
+			fmt.Sprintf("-suid=%v", row.suid))
 		defer test_helpers.UnmountErr(mnt)
 
 		mounts, err := mountinfo.GetMounts(mountinfo.SingleEntryFilter(mnt))
@@ -46,11 +72,13 @@ func TestDirectMount(t *testing.T) {
 		if info.Source != dir {
 			t.Errorf("wrong Source: have %q, want %q", info.Source, dir)
 		}
-		if !strings.Contains(info.VFSOptions, "max_read=") {
-			t.Errorf("VFSOptions is missing max_read")
-		}
-		if row.allow_other && !strings.Contains(info.VFSOptions, "allow_other") {
-			t.Errorf("VFSOptions is missing allow_other")
+		checkOptionPresent(t, info.VFSOptions, "max_read=", true)
+		checkOptionPresent(t, info.VFSOptions, "allow_other", row.allow_other)
+		checkOptionPresent(t, info.Options, "noexec", row.noexec)
+		// Enabling suid and dev only works as root
+		if os.Getuid() == 0 {
+			checkOptionPresent(t, info.Options, "nosuid", !row.suid)
+			checkOptionPresent(t, info.Options, "nodev", !row.dev)
 		}
 	}
 

From a40e9a8622b0d79ba0c18361104c47375ebb89a1 Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Wed, 17 May 2023 16:47:22 +0200
Subject: [PATCH 09/15] mount: set DirectMount: true

Attempt to directly call mount(2) before trying fusermount. This means we
can do without fusermount if running as root.

https://github.com/rfjakob/gocryptfs/issues/697
---
 README.md | 5 +++++
 mount.go  | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/README.md b/README.md
index 6ffa1ff..c7164c3 100644
--- a/README.md
+++ b/README.md
@@ -195,6 +195,11 @@ RM:    2,367
 Changelog
 ---------
 
+#### vNEXT, in progress
+* Attempt to directly call mount(2) before trying fusermount. This means we
+  can do without fusermount if running as root or in a root-like namespace
+  ([#697](https://github.com/rfjakob/gocryptfs/issues/697)).
+
 #### v2.3.2, 2023-04-29
 * Fix incorrect file size reported after hard link creation
   ([#724](https://github.com/rfjakob/gocryptfs/issues/724))
diff --git a/mount.go b/mount.go
index a229245..d856e49 100644
--- a/mount.go
+++ b/mount.go
@@ -396,6 +396,9 @@ func initGoFuse(rootNode fs.InodeEmbedder, args *argContainer) *fuse.Server {
 		// Setting SyncRead disables FUSE_CAP_ASYNC_READ. This makes the kernel
 		// do everything in-order without parallelism.
 		SyncRead: args.serialize_reads,
+		// Attempt to directly call mount(2) before trying fusermount. This means we
+		// can do without fusermount if running as root.
+		DirectMount: true,
 	}
 
 	mOpts := &fuseOpts.MountOptions

From 7d1e48d195b827ee00bfd0ab0421a395b1ec0ea7 Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Wed, 17 May 2023 17:22:39 +0200
Subject: [PATCH 10/15] go.mod: add test dependency
 github.com/moby/sys/mountinfo

---
 go.mod | 1 +
 1 file changed, 1 insertion(+)

diff --git a/go.mod b/go.mod
index d6b0a52..6aebae9 100644
--- a/go.mod
+++ b/go.mod
@@ -5,6 +5,7 @@ go 1.16
 require (
 	github.com/aperturerobotics/jacobsa-crypto v1.0.0
 	github.com/hanwen/go-fuse/v2 v2.3.0
+	github.com/moby/sys/mountinfo v0.6.2
 	github.com/pkg/xattr v0.4.3
 	github.com/rfjakob/eme v1.1.2
 	github.com/sabhiram/go-gitignore v0.0.0-20201211210132-54b8a0bf510f

From 09954c4bdecf0ca6da65776f176dc934ffced2b0 Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Wed, 17 May 2023 23:26:56 +0200
Subject: [PATCH 11/15] fusefrontend: implement our own Access()

Not having Access() means go-fuse emulates it by looking at Getattr().
This works fine most of the time, but breaks down on sshfs, where
sshfs-benchmark.bash shows this:

	gocryptfs/tests$ ./sshfs-benchmark.bash nuetzlich.net
	working directory: /tmp/sshfs-benchmark.bash.JQC
	sshfs mounted: nuetzlich.net:/tmp -> sshfs.mnt
	gocryptfs mounted: sshfs.mnt/sshfs-benchmark.bash.Wrz/gocryptfs.crypt -> gocryptfs.mnt

	sshfs-benchmark.bash:    sshfs  gocryptfs-on-sshfs
	git init                  3.98                6.80
	rsync                     7.71               10.84
	rm -R                     4.30rm: descend into write-protected directory 'gocryptfs.mnt/git1'?

The go-fuse emulation gets it wrong here because sshfs reports
permissions but does not enforce them.

Implement it ourselves properly.
---
 internal/fusefrontend/node.go | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/internal/fusefrontend/node.go b/internal/fusefrontend/node.go
index 274123b..687a386 100644
--- a/internal/fusefrontend/node.go
+++ b/internal/fusefrontend/node.go
@@ -99,6 +99,17 @@ func (n *Node) Getattr(ctx context.Context, f fs.FileHandle, out *fuse.AttrOut)
 	return 0
 }
 
+func (n *Node) Access(ctx context.Context, mode uint32) syscall.Errno {
+	dirfd, cName, errno := n.prepareAtSyscallMyself()
+	if errno != 0 {
+		return errno
+	}
+	defer syscall.Close(dirfd)
+
+	err := syscallcompat.Faccessat(dirfd, cName, mode)
+	return fs.ToErrno(err)
+}
+
 // Unlink - FUSE call. Delete a file.
 //
 // Symlink-safe through use of Unlinkat().

From c67454464aaabec7c594eeb84983287057ab4231 Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Thu, 18 May 2023 10:14:21 +0200
Subject: [PATCH 12/15] tests: TestDirectMount: check for default_permissions

---
 tests/cli/directmount_test.go | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/cli/directmount_test.go b/tests/cli/directmount_test.go
index 3e23e6a..7ecbf80 100644
--- a/tests/cli/directmount_test.go
+++ b/tests/cli/directmount_test.go
@@ -74,6 +74,8 @@ func TestDirectMount(t *testing.T) {
 		}
 		checkOptionPresent(t, info.VFSOptions, "max_read=", true)
 		checkOptionPresent(t, info.VFSOptions, "allow_other", row.allow_other)
+		// gocryptfs enables default_permissions when allow_other is enabled
+		checkOptionPresent(t, info.VFSOptions, "default_permissions", row.allow_other)
 		checkOptionPresent(t, info.Options, "noexec", row.noexec)
 		// Enabling suid and dev only works as root
 		if os.Getuid() == 0 {

From b725de5ec300aec208908c6a3bf5443cee7cfa81 Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Fri, 19 May 2023 13:17:24 +0200
Subject: [PATCH 13/15] fsstress-gocryptfs.bash: improve header comment

I maybe should have noted that this is xfstests generic/013.
---
 tests/stress_tests/fsstress-gocryptfs.bash | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/stress_tests/fsstress-gocryptfs.bash b/tests/stress_tests/fsstress-gocryptfs.bash
index 03352f2..30ea456 100755
--- a/tests/stress_tests/fsstress-gocryptfs.bash
+++ b/tests/stress_tests/fsstress-gocryptfs.bash
@@ -1,9 +1,13 @@
 #!/bin/bash
 #
-# Mount a go-fuse loopback filesystem in /tmp and run fsstress against it
+# Mount a gocryptfs filesystem in /var/tmp and run fsstress against it
 # in an infinite loop, only exiting on errors.
 #
-# When called as "fsstress-gocryptfs.bash", a gocryptfs filesystem is tested
+# Replicates what xfstests generic/013 does
+# ( https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/tests/generic/013 ),
+# but in an infinite loop.
+#
+# When called as "fsstress-loopback.bash", a go-fuse loopback filesystem is tested
 # instead.
 #
 # This test used to fail on older go-fuse versions after a few iterations with

From 3058b7978fd8dabd3e8565c9be816b1367bd196a Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Tue, 30 May 2023 09:43:45 +0200
Subject: [PATCH 14/15] tests: add cluster test

finds out what happens if multiple
gocryptfs mounts write to one file concurrently
(usually, nothing good).

This use case is relevant for HPC clusters.
---
 tests/cluster/cluster_test.go | 111 ++++++++++++++++++++++++++++++++++
 1 file changed, 111 insertions(+)
 create mode 100644 tests/cluster/cluster_test.go

diff --git a/tests/cluster/cluster_test.go b/tests/cluster/cluster_test.go
new file mode 100644
index 0000000..2e969ce
--- /dev/null
+++ b/tests/cluster/cluster_test.go
@@ -0,0 +1,111 @@
+// package cluster_test finds out what happens if multiple
+// gocryptfs mounts write to one file concurrently
+// (usually, nothing good).
+//
+// This use case is relevant for HPC clusters.
+package cluster_test
+
+import (
+	"bytes"
+	"math/rand"
+	"os"
+	"sync"
+	"testing"
+
+	"github.com/rfjakob/gocryptfs/v2/tests/test_helpers"
+)
+
+// This test passes on XFS but fails on ext4 and tmpfs!!!
+//
+// Quoting https://lists.samba.org/archive/samba-technical/2019-March/133050.html
+//
+// > It turns out that xfs respects POSIX w.r.t "atomic read/write" and
+// > this is implemented by taking a file-wide shared lock on every
+// > buffered read.
+// > This behavior is unique to XFS on Linux and is not optional.
+// > Other Linux filesystems only guaranty page level atomicity for
+// > 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.
+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" +
+			"Run it like this: ENABLE_CLUSTER_TEST=1 go test\n" +
+			"Choose a backing directory by setting TMPDIR.")
+	}
+
+	const blocksize = 4096
+	const fileSize = 25 * blocksize // 100 kiB
+
+	cDir := test_helpers.InitFS(t)
+	mnt1 := cDir + ".mnt1"
+	mnt2 := cDir + ".mnt2"
+	test_helpers.MountOrFatal(t, cDir, mnt1, "-extpass=echo test", "-wpanic=0")
+	defer test_helpers.UnmountPanic(mnt1)
+	test_helpers.MountOrFatal(t, cDir, mnt2, "-extpass=echo test", "-wpanic=0")
+	defer test_helpers.UnmountPanic(mnt2)
+
+	f1, err := os.Create(mnt1 + "/foo")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f1.Close()
+	// Preallocate space
+	_, err = f1.WriteAt(make([]byte, fileSize), 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	f2, err := os.OpenFile(mnt2+"/foo", os.O_RDWR, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f2.Close()
+
+	var wg sync.WaitGroup
+
+	const loops = 10000
+	writeThread := func(f *os.File) {
+		defer wg.Done()
+		buf := make([]byte, blocksize)
+		for i := 0; i < loops; i++ {
+			if t.Failed() {
+				return
+			}
+			off := rand.Int63n(fileSize / blocksize)
+			_, err := f.WriteAt(buf, off)
+			if err != nil {
+				t.Errorf("writeThread iteration %d: WriteAt failed: %v", i, err)
+				return
+			}
+		}
+	}
+	readThread := func(f *os.File) {
+		defer wg.Done()
+		zeroBlock := make([]byte, blocksize)
+		buf := make([]byte, blocksize)
+		for i := 0; i < loops; i++ {
+			if t.Failed() {
+				return
+			}
+			off := rand.Int63n(fileSize / blocksize)
+			_, err := f.ReadAt(buf, off)
+			if err != nil {
+				t.Errorf("readThread iteration %d: ReadAt failed: %v", i, err)
+				return
+			}
+			if !bytes.Equal(buf, zeroBlock) {
+				t.Errorf("readThread iteration %d: data mismatch", i)
+				return
+			}
+		}
+	}
+
+	wg.Add(4)
+	go writeThread(f1)
+	go writeThread(f2)
+	go readThread(f1)
+	go readThread(f2)
+	wg.Wait()
+}

From 8979cca43ea2ed15cf6ff577619298b9473d2882 Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Sat, 10 Jun 2023 16:31:49 +0200
Subject: [PATCH 15/15] README: update changelog for v2.4.0

---
 README.md | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/README.md b/README.md
index c7164c3..f5783a4 100644
--- a/README.md
+++ b/README.md
@@ -195,10 +195,13 @@ RM:    2,367
 Changelog
 ---------
 
-#### vNEXT, in progress
-* Attempt to directly call mount(2) before trying fusermount. This means we
-  can do without fusermount if running as root or in a root-like namespace
-  ([#697](https://github.com/rfjakob/gocryptfs/issues/697)).
+#### v2.4.0, 2023-06-10
+* Try the `mount(2)` syscall before falling back to `fusermount(1)`. This means we
+  don't need `fusermount(1)` at all if running as root or in a root-like namespace
+  ([#697](https://github.com/rfjakob/gocryptfs/issues/697))
+* Fix `-extpass` mis-parsing commas ([#730](https://github.com/rfjakob/gocryptfs/issues/730))
+* Fix `rm -R` mis-reporting `write-protected directory` on gocryptfs on sshfs
+  ([commit](https://github.com/rfjakob/gocryptfs/commit/09954c4bdecf0ca6da65776f176dc934ffced2b0))
 
 #### v2.3.2, 2023-04-29
 * Fix incorrect file size reported after hard link creation