From ed0a12b7337c2d88c027329f64e73070da17d5b3 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Wed, 1 May 2024 22:26:37 +0200 Subject: reverse: use unique generation number for all nodes We used to present gocryptfs.longname.*.name files for hardlinked files as hardlinked to the kernel (same Node ID) which is wrong. Fix this by using a unique generation number for all nodes, which also fixes possible issues with inode reuse. Basically what 1bc1db620b061aabf59469a5eb4fb60e3e1701a3 did for forward mode with -sharedstorage. Fixes https://github.com/rfjakob/gocryptfs/issues/802 --- internal/fusefrontend_reverse/node.go | 19 +++++++++++++++++++ internal/fusefrontend_reverse/node_helpers.go | 26 +++++++++++--------------- internal/fusefrontend_reverse/root_node.go | 24 ++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 15 deletions(-) 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), + } +} -- cgit v1.2.3