Skip to content

Commit

Permalink
reverse: use unique generation number for all nodes
Browse files Browse the repository at this point in the history
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 1bc1db6 did
for forward mode with -sharedstorage.

Fixes #802
  • Loading branch information
rfjakob committed May 1, 2024
1 parent 210c5c5 commit ed0a12b
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 15 deletions.
19 changes: 19 additions & 0 deletions internal/fusefrontend_reverse/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
26 changes: 11 additions & 15 deletions internal/fusefrontend_reverse/node_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down Expand Up @@ -153,15 +152,16 @@ 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

}

// 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")
}

Expand All @@ -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
}
Expand All @@ -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
Expand Down
24 changes: 24 additions & 0 deletions internal/fusefrontend_reverse/root_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"
"path/filepath"
"strings"
"sync/atomic"
"syscall"

"github.com/rfjakob/gocryptfs/v2/internal/exitcodes"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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),
}
}

0 comments on commit ed0a12b

Please sign in to comment.