aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--Documentation/MANPAGE.md2
-rw-r--r--daemonize.go2
-rw-r--r--internal/fusefrontend_reverse/node.go19
-rw-r--r--internal/fusefrontend_reverse/node_helpers.go26
-rw-r--r--internal/fusefrontend_reverse/root_node.go24
-rw-r--r--internal/fusefrontend_reverse/virtualnode.go11
-rw-r--r--internal/generation_num/generation_num.go11
-rw-r--r--internal/inomap/inomap.go41
-rw-r--r--internal/inomap/inomap_test.go13
-rw-r--r--internal/syscallcompat/sys_common.go5
-rw-r--r--tests/reverse/correctness_test.go35
-rw-r--r--tests/reverse/inomap_test.go6
12 files changed, 129 insertions, 66 deletions
diff --git a/Documentation/MANPAGE.md b/Documentation/MANPAGE.md
index 44b2f93..5b3508f 100644
--- a/Documentation/MANPAGE.md
+++ b/Documentation/MANPAGE.md
@@ -499,7 +499,7 @@ Creating a filesystem with no pin verification:
gocryptfs -init -fido2 DEVICE_PATH -fido2-assert-option pin=false CIPHERDIR
-Creating a filesystem with both user presence and pin verification:
+Creating a filesystem with both user verification and pin verification:
gocryptfs -init -fido2 DEVICE_PATH -fido2-assert-option uv=true -fido2-assert-option pin=true CIPHERDIR
diff --git a/daemonize.go b/daemonize.go
index 097a544..c9b0f0e 100644
--- a/daemonize.go
+++ b/daemonize.go
@@ -90,7 +90,7 @@ func redirectStdFds() {
if err != nil {
tlog.Warn.Printf("redirectStdFds: stderr dup error: %v\n", err)
}
- // Our stout and stderr point to "pw". We can close the extra copy.
+ // Our stdout and stderr point to "pw". We can close the original copy.
pw.Close()
// Redirect stdin to /dev/null
nullFd, err := os.Open("/dev/null")
diff --git a/internal/fusefrontend_reverse/node.go b/internal/fusefrontend_reverse/node.go
index 170410f..22ad975 100644
--- a/internal/fusefrontend_reverse/node.go
+++ b/internal/fusefrontend_reverse/node.go
@@ -68,6 +68,25 @@ func (n *Node) Lookup(ctx context.Context, cName string, out *fuse.EntryOut) (ch
if t == typeReal {
n.translateSize(d.dirfd, cName, d.pName, &out.Attr)
}
+
+ // Usually we always create a new Node ID by always incrementing the generation
+ // number.
+ //
+ // If we already have a child node that matches what we found on disk*
+ // (as reflected in `ch`), return it here.
+ //
+ // This keeps the Node ID for each directory entry stable
+ // (until forgotten), preventing extra FORGETs from the kernel.
+ //
+ // *We compare `cName`, `Ino`, `Mode` (but not `Gen`!)
+ old := n.Inode.GetChild(cName)
+ if old != nil &&
+ old.StableAttr().Ino == ch.StableAttr().Ino &&
+ // `Mode` has already been masked with syscall.S_IFMT by n.newChild()
+ old.StableAttr().Mode == ch.StableAttr().Mode {
+ return old, 0
+ }
+
return ch, 0
}
diff --git a/internal/fusefrontend_reverse/node_helpers.go b/internal/fusefrontend_reverse/node_helpers.go
index 96c3c2d..6bba097 100644
--- a/internal/fusefrontend_reverse/node_helpers.go
+++ b/internal/fusefrontend_reverse/node_helpers.go
@@ -91,18 +91,17 @@ func (n *Node) prepareAtSyscall(child string) (d *dirfdPlus, errno syscall.Errno
// newChild attaches a new child inode to n.
// The passed-in `st` will be modified to get a unique inode number.
+//
+// This function is not used for virtual files. See lookupLongnameName(),
+// lookupDiriv() instead.
func (n *Node) newChild(ctx context.Context, st *syscall.Stat_t, out *fuse.EntryOut) *fs.Inode {
- isOtherFilesystem := (uint64(st.Dev) != n.rootNode().rootDev)
- // Get unique inode number
rn := n.rootNode()
+ isOtherFilesystem := (uint64(st.Dev) != rn.rootDev)
+ // Get unique inode number
rn.inoMap.TranslateStat(st)
out.Attr.FromStat(st)
// Create child node
- id := fs.StableAttr{
- Mode: uint32(st.Mode),
- Gen: 1,
- Ino: st.Ino,
- }
+ id := rn.uniqueStableAttr(uint32(st.Mode), st.Ino)
node := &Node{
isOtherFilesystem: isOtherFilesystem,
}
@@ -153,7 +152,7 @@ func (n *Node) lookupLongnameName(ctx context.Context, nameFile string, out *fus
}
out.Attr = vf.attr
// Create child node
- id := fs.StableAttr{Mode: uint32(vf.attr.Mode), Gen: 1, Ino: vf.attr.Ino}
+ id := rn.uniqueStableAttr(uint32(vf.attr.Mode), vf.attr.Ino)
ch = n.NewInode(ctx, vf, id)
return
@@ -161,7 +160,8 @@ func (n *Node) lookupLongnameName(ctx context.Context, nameFile string, out *fus
// lookupDiriv returns a new Inode for a gocryptfs.diriv file inside `n`.
func (n *Node) lookupDiriv(ctx context.Context, out *fuse.EntryOut) (ch *fs.Inode, errno syscall.Errno) {
- if rn := n.rootNode(); rn.args.DeterministicNames {
+ rn := n.rootNode()
+ if rn.args.DeterministicNames {
log.Panic("BUG: lookupDiriv called but DeterministicNames is set")
}
@@ -183,7 +183,7 @@ func (n *Node) lookupDiriv(ctx context.Context, out *fuse.EntryOut) (ch *fs.Inod
}
out.Attr = vf.attr
// Create child node
- id := fs.StableAttr{Mode: uint32(vf.attr.Mode), Gen: 1, Ino: vf.attr.Ino}
+ id := rn.uniqueStableAttr(uint32(vf.attr.Mode), vf.attr.Ino)
ch = n.NewInode(ctx, vf, id)
return
}
@@ -202,11 +202,7 @@ func (n *Node) lookupConf(ctx context.Context, out *fuse.EntryOut) (ch *fs.Inode
rn.inoMap.TranslateStat(&st)
out.Attr.FromStat(&st)
// Create child node
- id := fs.StableAttr{
- Mode: uint32(st.Mode),
- Gen: 1,
- Ino: st.Ino,
- }
+ id := rn.uniqueStableAttr(uint32(st.Mode), st.Ino)
node := &VirtualConfNode{path: p}
ch = n.NewInode(ctx, node, id)
return
diff --git a/internal/fusefrontend_reverse/root_node.go b/internal/fusefrontend_reverse/root_node.go
index 8a2afd9..1a68ffd 100644
--- a/internal/fusefrontend_reverse/root_node.go
+++ b/internal/fusefrontend_reverse/root_node.go
@@ -5,6 +5,7 @@ import (
"os"
"path/filepath"
"strings"
+ "sync/atomic"
"syscall"
"github.com/rfjakob/gocryptfs/v2/internal/exitcodes"
@@ -45,6 +46,13 @@ type RootNode struct {
// If a file name length is shorter than shortNameMax, there is no need to
// hash it.
shortNameMax int
+ // gen is the node generation number. Normally, it is always set to 1,
+ // but reverse mode, like -sharestorage, uses an incrementing counter for new nodes.
+ // This makes each directory entry unique (even hard links),
+ // makes go-fuse hand out separate FUSE Node IDs for each, and prevents
+ // bizarre problems when inode numbers are reused behind our back,
+ // like this one: https://github.com/rfjakob/gocryptfs/issues/802
+ gen uint64
}
// NewRootNode returns an encrypted FUSE overlay filesystem.
@@ -149,3 +157,19 @@ func (rn *RootNode) excludeDirEntries(d *dirfdPlus, entries []fuse.DirEntry) (fi
}
return filtered
}
+
+// uniqueStableAttr returns a fs.StableAttr struct with a unique generation number,
+// preventing files to appear hard-linked, even when they have the same inode number.
+//
+// This is good because inode numbers can be reused behind our back, which could make
+// unrelated files appear hard-linked.
+// Example: https://github.com/rfjakob/gocryptfs/issues/802
+func (rn *RootNode) uniqueStableAttr(mode uint32, ino uint64) fs.StableAttr {
+ return fs.StableAttr{
+ Mode: mode,
+ Ino: ino,
+ // Make each directory entry a unique node by using a unique generation
+ // value. Also see the comment at RootNode.gen for details.
+ Gen: atomic.AddUint64(&rn.gen, 1),
+ }
+}
diff --git a/internal/fusefrontend_reverse/virtualnode.go b/internal/fusefrontend_reverse/virtualnode.go
index 688f536..922cfa7 100644
--- a/internal/fusefrontend_reverse/virtualnode.go
+++ b/internal/fusefrontend_reverse/virtualnode.go
@@ -86,8 +86,15 @@ func (n *Node) newVirtualMemNode(content []byte, parentStat *syscall.Stat_t, ino
// Adjust inode number and size
rn := n.rootNode()
st := parentStat
- q := inomap.NewQIno(uint64(st.Dev), inoTag, uint64(st.Ino))
- st.Ino = rn.inoMap.Translate(q)
+ if inoTag == inoTagNameFile {
+ // No stable mapping for gocryptfs.longname.*.name files, instead use an
+ // incrementing counter. We don't want two of those files to ever have the
+ // same inode number, even for hard-linked files.
+ st.Ino = rn.inoMap.NextSpillIno()
+ } else {
+ q := inomap.NewQIno(uint64(st.Dev), inoTag, uint64(st.Ino))
+ st.Ino = rn.inoMap.Translate(q)
+ }
st.Size = int64(len(content))
st.Mode = virtualFileMode
st.Nlink = 1
diff --git a/internal/generation_num/generation_num.go b/internal/generation_num/generation_num.go
deleted file mode 100644
index 298db14..0000000
--- a/internal/generation_num/generation_num.go
+++ /dev/null
@@ -1,11 +0,0 @@
-package generation_num
-
-import (
- "sync/atomic"
-)
-
-var gen uint64
-
-func Next() uint64 {
- return atomic.AddUint64(&gen, 1)
-}
diff --git a/internal/inomap/inomap.go b/internal/inomap/inomap.go
index 0f7ade3..b4dbf27 100644
--- a/internal/inomap/inomap.go
+++ b/internal/inomap/inomap.go
@@ -3,8 +3,8 @@
//
// Format of the returned inode numbers:
//
-// [spill bit = 0][15 bit namespace id][48 bit passthru inode number]
-// [spill bit = 1][63 bit spill inode number ]
+// [spill bit = 0][15 bit namespace id][48 bit passthru inode number] = 64 bit translated inode number
+// [spill bit = 1][63 bit counter ] = 64 bit spill inode number
//
// Each (Dev, Tag) tuple gets a namespace id assigned. The original inode
// number is then passed through in the lower 48 bits.
@@ -16,7 +16,9 @@ package inomap
import (
"log"
+ "math"
"sync"
+ "sync/atomic"
"syscall"
"github.com/rfjakob/gocryptfs/v2/internal/tlog"
@@ -27,10 +29,8 @@ const (
maxNamespaceId = 1<<15 - 1
// max value of 48 bit passthru inode number
maxPassthruIno = 1<<48 - 1
- // max value of 63 bit spill inode number
- maxSpillIno = 1<<63 - 1
- // bit 63 is used as the spill bit
- spillBit = 1 << 63
+ // the spill inode number space starts at 0b10000...0.
+ spillSpaceStart = 1 << 63
)
// InoMap stores the maps using for inode number translation.
@@ -57,7 +57,7 @@ func New(rootDev uint64) *InoMap {
namespaceMap: make(map[namespaceData]uint16),
namespaceNext: 0,
spillMap: make(map[QIno]uint64),
- spillNext: 0,
+ spillNext: spillSpaceStart,
}
if rootDev > 0 {
// Reserve namespace 0 for rootDev
@@ -69,23 +69,32 @@ func New(rootDev uint64) *InoMap {
var spillWarn sync.Once
+// NextSpillIno returns a fresh inode number from the spill pool without adding it to
+// spillMap.
+// Reverse mode NextSpillIno() for gocryptfs.longname.*.name files where a stable
+// mapping is not needed.
+func (m *InoMap) NextSpillIno() (out uint64) {
+ if m.spillNext == math.MaxUint64 {
+ log.Panicf("spillMap overflow: spillNext = 0x%x", m.spillNext)
+ }
+ return atomic.AddUint64(&m.spillNext, 1) - 1
+}
+
func (m *InoMap) spill(in QIno) (out uint64) {
- spillWarn.Do(func() { tlog.Warn.Printf("InoMap: opening spillMap for %v", in) })
+ spillWarn.Do(func() { tlog.Warn.Printf("InoMap: opening spillMap for %#v", in) })
out, found := m.spillMap[in]
if found {
- return out | spillBit
- }
- if m.spillNext >= maxSpillIno {
- log.Panicf("spillMap overflow: spillNext = 0x%x", m.spillNext)
+ return out
}
- out = m.spillNext
- m.spillNext++
+
+ out = m.NextSpillIno()
m.spillMap[in] = out
- return out | spillBit
+
+ return out
}
-// Translate maps the passed-in (device, inode) pair to a unique inode number.
+// Translate maps the passed-in (device, tag, inode) tuple to a unique inode number.
func (m *InoMap) Translate(in QIno) (out uint64) {
m.Lock()
defer m.Unlock()
diff --git a/internal/inomap/inomap_test.go b/internal/inomap/inomap_test.go
index 9ec2932..ce5b880 100644
--- a/internal/inomap/inomap_test.go
+++ b/internal/inomap/inomap_test.go
@@ -5,6 +5,11 @@ import (
"testing"
)
+const (
+ // bit 63 is used as the spill bit
+ spillBit = 1 << 63
+)
+
func TestTranslate(t *testing.T) {
m := New(0)
q := QIno{Ino: 1}
@@ -102,6 +107,9 @@ func TestSpill(t *testing.T) {
if out1&spillBit == 0 {
t.Error("spill bit not set")
}
+ if out1 != spillSpaceStart {
+ t.Errorf("unexpected first spill inode number %d", out1)
+ }
out2 := m.Translate(q)
if out2&spillBit == 0 {
t.Error("spill bit not set")
@@ -109,6 +117,11 @@ func TestSpill(t *testing.T) {
if out1 != out2 {
t.Errorf("unstable mapping: %d vs %d", out1, out2)
}
+ q.Ino = maxPassthruIno + 2
+ out3 := m.Translate(q)
+ if out3 != out1+1 {
+ t.Errorf("unexpected 2nd spill inode number %d", out1)
+ }
}
// TestUniqueness checks that unique (Dev, Flags, Ino) tuples get unique inode
diff --git a/internal/syscallcompat/sys_common.go b/internal/syscallcompat/sys_common.go
index 50b8180..1aa6a6e 100644
--- a/internal/syscallcompat/sys_common.go
+++ b/internal/syscallcompat/sys_common.go
@@ -60,6 +60,11 @@ func Openat(dirfd int, path string, flags int, mode uint32) (fd int, err error)
flags |= syscall.O_NOFOLLOW
}
}
+
+ // os/exec expects all fds to have O_CLOEXEC or it will leak fds to subprocesses.
+ // In our case, that would be logger(1), and we did leak fds to it.
+ flags |= syscall.O_CLOEXEC
+
fd, err = retryEINTR2(func() (int, error) {
return unix.Openat(dirfd, path, flags, mode)
})
diff --git a/tests/reverse/correctness_test.go b/tests/reverse/correctness_test.go
index 8747a11..b335456 100644
--- a/tests/reverse/correctness_test.go
+++ b/tests/reverse/correctness_test.go
@@ -4,7 +4,6 @@ import (
"bytes"
"fmt"
"io/ioutil"
- "log"
"os"
"path/filepath"
"strings"
@@ -296,25 +295,13 @@ func TestSeekData(t *testing.T) {
f.Close()
}
-func findInum(dir string, inum uint64) (name string) {
- entries, err := os.ReadDir(dir)
- if err != nil {
- panic(err)
- }
- for _, e := range entries {
- info, _ := e.Info()
- st := info.Sys().(*syscall.Stat_t)
- if st.Ino == inum {
- return dir + "/" + e.Name()
- }
- }
- log.Panicf("inum %d not found", inum)
- return ""
-}
-
// gocryptfs.longname.*.name of hardlinked files should not appear hardlinked (as the
// contents are different).
//
+// This means that
+// 1) They have a different NodeID, hence the kernel knows it's different files
+// 2) They have a different inode number, hence userspace knows they are not hard-linked.
+//
// https://github.com/rfjakob/gocryptfs/issues/802
func TestHardlinkedLongname(t *testing.T) {
if plaintextnames {
@@ -339,7 +326,7 @@ func TestHardlinkedLongname(t *testing.T) {
if err := syscall.Stat(workdir, &st); err != nil {
t.Fatal(err)
}
- cWorkdir := findInum(dirB, st.Ino)
+ cWorkdir := dirB + "/" + findIno(dirB, st.Ino)
t.Logf("workdir=%q cWorkdir=%q", workdir, cWorkdir)
matches, err := filepath.Glob(cWorkdir + "/gocryptfs.longname.*.name")
@@ -352,4 +339,16 @@ func TestHardlinkedLongname(t *testing.T) {
if test_helpers.Md5fn(matches[0]) == test_helpers.Md5fn(matches[1]) {
t.Errorf("Files %q are identical - that's wrong!", matches)
}
+
+ var st0 syscall.Stat_t
+ if err := syscall.Stat(matches[0], &st0); err != nil {
+ t.Fatal(err)
+ }
+ var st1 syscall.Stat_t
+ if err := syscall.Stat(matches[1], &st1); err != nil {
+ t.Fatal(err)
+ }
+ if st0.Ino == st1.Ino {
+ t.Errorf("Files %q have the same inode number - that's wrong!", matches)
+ }
}
diff --git a/tests/reverse/inomap_test.go b/tests/reverse/inomap_test.go
index a79ddd2..ff78f4c 100644
--- a/tests/reverse/inomap_test.go
+++ b/tests/reverse/inomap_test.go
@@ -2,6 +2,7 @@ package reverse_test
import (
"bytes"
+ "log"
"os"
"strings"
"syscall"
@@ -30,6 +31,7 @@ func findIno(dir string, ino uint64) string {
return entry
}
}
+ log.Panicf("ino %d not found", ino)
return ""
}
@@ -141,7 +143,7 @@ func TestVirtualFileIno(t *testing.T) {
if origInos.child == cipherInos.name {
t.Errorf("name ino collision: %d == %d", origInos.child, cipherInos.name)
}
- if origInos.child&mask != cipherInos.name&mask {
- t.Errorf("name ino mismatch: %#x vs %#x", origInos.child, cipherInos.name)
+ if cipherInos.name < 1<<63 {
+ t.Errorf("name ino should be in spill space, but is actually %#x", cipherInos.name)
}
}
generated by cgit v1.2.3 (git 2.39.1) at 2024-05-18 17:51:54 +0000