vfs: fix crash when multiple mounts or servers share the same VFS

The VFS is shared between users with the same remote and options, for example two mounts created over the rc, or a mount and an NFS server. Each node has a single Sys() slot which mount, mount2 and serve nfs all used to attach their per-node data. With a shared VFS the users overwrote each other's data: at best churning the cached FUSE nodes, and since the slot was an atomic.Value, panicking with "store of inconsistently typed value" as soon as two users stored different types on the same node. This change gives each node auxiliary values keyed by owner, set with SetAux and read with Aux, so each user of the VFS has an independent slot. The mounts now cache their FUSE nodes under their own key, leaving Sys - which is read through the os.FileInfo interface - reserved for users like serve nfs which need to control what that returns. Nodes with nothing attached use less memory than before (one pointer instead of an atomic.Value) and reads remain lock free. Bug discovered while thinking about #9617
2026-07-27 14:55:20 +01:00
parent b2d0642825
commit e006d7c13f
7 changed files with 168 additions and 28 deletions
@@ -81,7 +81,7 @@ func (d *Dir) Lookup(ctx context.Context, req *fuse.LookupRequest, resp *fuse.Lo
 	resp.EntryValid = time.Duration(d.fsys.opt.AttrTimeout)
 	// Check the mnode to see if it has a fuse Node cached
 	// We must return the same fuse nodes for vfs Nodes
-	node, ok := mnode.Sys().(fusefs.Node)
+	node, ok := mnode.Aux(d.fsys).(fusefs.Node)
 	if ok {
 		return node, nil
 	}
@@ -94,7 +94,7 @@ func (d *Dir) Lookup(ctx context.Context, req *fuse.LookupRequest, resp *fuse.Lo
 		panic("bad type")
 	}
 	// Cache the node for later
-	mnode.SetSys(node)
+	mnode.SetAux(d.fsys, node)
 	return node, nil
 }

@@ -158,7 +158,7 @@ func (d *Dir) Create(ctx context.Context, req *fuse.CreateRequest, resp *fuse.Cr
 		return nil, nil, translateError(err)
 	}
 	node = &File{file, d.fsys}
-	file.SetSys(node) // cache the FUSE node for later
+	file.SetAux(d.fsys, node) // cache the FUSE node for later
 	return node, &FileHandle{fh}, err
 }

@@ -172,7 +172,7 @@ func (d *Dir) Mkdir(ctx context.Context, req *fuse.MkdirRequest) (node fusefs.No
 		return nil, translateError(err)
 	}
 	node = &Dir{dir, d.fsys}
-	dir.SetSys(node) // cache the FUSE node for later
+	dir.SetAux(d.fsys, node) // cache the FUSE node for later
 	return node, nil
 }

@@ -30,7 +30,7 @@ var _ fusefs.InodeEmbedder = (*Node)(nil)
 func newNode(fsys *FS, vfsNode vfs.Node) (node *Node) {
 	// Check the vfsNode to see if it has a fuse Node cached
 	// We must return the same fuse nodes for vfs Nodes
-	node, ok := vfsNode.Sys().(*Node)
+	node, ok := vfsNode.Aux(fsys).(*Node)
 	if ok {
 		return node
 	}
@@ -39,7 +39,7 @@ func newNode(fsys *FS, vfsNode vfs.Node) (node *Node) {
 		fsys: fsys,
 	}
 	// Cache the node for later
-	vfsNode.SetSys(node)
+	vfsNode.SetAux(fsys, node)
 	return node
 }

@@ -0,0 +1,85 @@
+package vfs
+
+import "sync/atomic"
+
+// auxEntry associates an owner with a value attached to a node.
+type auxEntry struct {
+	owner, value any
+}
+
+// aux holds auxiliary values attached to a node, keyed by owner.
+//
+// It is embedded in Dir and File to provide the Aux, SetAux, Sys and
+// SetSys methods of the Node interface.
+//
+// Reads are lock free. Writes copy the entry list, which is assumed
+// to be very short.
+type aux struct {
+	entries atomic.Pointer[[]auxEntry]
+}
+
+// sysOwner is the owner Sys and SetSys attach their value under.
+type sysOwner struct{}
+
+// Aux returns the value attached to the node for owner, or nil if
+// none is attached.
+func (a *aux) Aux(owner any) any {
+	if entries := a.entries.Load(); entries != nil {
+		for _, entry := range *entries {
+			if entry.owner == owner {
+				return entry.value
+			}
+		}
+	}
+	return nil
+}
+
+// SetAux attaches value to the node for owner, replacing any value
+// owner attached before. Attaching nil removes owner's value.
+//
+// Values attached by different owners are independent, so multiple
+// users of a shared VFS do not conflict. Owner must be comparable and
+// unique to the user - a pointer to the user's filesystem struct is a
+// good choice.
+func (a *aux) SetAux(owner, value any) {
+	for {
+		old := a.entries.Load()
+		var entries []auxEntry
+		if old != nil {
+			entries = make([]auxEntry, 0, len(*old)+1)
+			for _, entry := range *old {
+				if entry.owner != owner {
+					entries = append(entries, entry)
+				}
+			}
+		}
+		if value != nil {
+			entries = append(entries, auxEntry{owner: owner, value: value})
+		}
+		var next *[]auxEntry
+		if len(entries) != 0 {
+			next = &entries
+		}
+		if a.entries.CompareAndSwap(old, next) {
+			return
+		}
+	}
+}
+
+// Sys returns the value set with SetSys (can be nil) - satisfies
+// os.FileInfo.
+//
+// This is what callers reading the node through os.FileInfo (e.g. the
+// NFS server library) will see. It is reserved for users which need
+// to control that; users caching their own data on the node should
+// use Aux and SetAux instead. As the VFS may be shared, the value
+// stored here should be derived from the node alone so that all users
+// store the same value.
+func (a *aux) Sys() any {
+	return a.Aux(sysOwner{})
+}
+
+// SetSys sets the value returned by Sys - see Sys for the contract.
+func (a *aux) SetSys(x any) {
+	a.SetAux(sysOwner{}, x)
+}
@@ -0,0 +1,73 @@
+package vfs
+
+import (
+	"fmt"
+	"sync"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestAux(t *testing.T) {
+	var a aux
+	owner1, owner2 := new(int), new(int)
+
+	// Nothing attached yet
+	assert.Nil(t, a.Aux(owner1))
+	assert.Nil(t, a.Sys())
+
+	// Values attached by different owners are independent even if
+	// they have different types
+	a.SetAux(owner1, "potato")
+	a.SetAux(owner2, 2)
+	assert.Equal(t, "potato", a.Aux(owner1))
+	assert.Equal(t, 2, a.Aux(owner2))
+
+	// Replace a value
+	a.SetAux(owner1, "sausage")
+	assert.Equal(t, "sausage", a.Aux(owner1))
+
+	// Remove a value
+	a.SetAux(owner1, nil)
+	assert.Nil(t, a.Aux(owner1))
+	assert.Equal(t, 2, a.Aux(owner2))
+
+	// Sys is independent of the other owners
+	assert.Nil(t, a.Sys())
+	a.SetSys(42)
+	assert.Equal(t, 42, a.Sys())
+	assert.Equal(t, 2, a.Aux(owner2))
+
+	// Changing the type of the value stored must not panic
+	a.SetSys("42")
+	assert.Equal(t, "42", a.Sys())
+
+	// Remove the remaining values
+	a.SetSys(nil)
+	a.SetAux(owner2, nil)
+	assert.Nil(t, a.entries.Load())
+}
+
+func TestAuxConcurrent(t *testing.T) {
+	const (
+		owners     = 4
+		iterations = 100
+	)
+	var (
+		a  aux
+		wg sync.WaitGroup
+	)
+	for i := range owners {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			owner := &i
+			for j := range iterations {
+				value := fmt.Sprintf("%d-%d", i, j)
+				a.SetAux(owner, value)
+				assert.Equal(t, value, a.Aux(owner))
+			}
+		}()
+	}
+	wg.Wait()
+}
@@ -24,6 +24,7 @@ import (

 // Dir represents a directory entry
 type Dir struct {
+	aux                      // values attached by users of the VFS
 	vfs          *VFS        // read only
 	inode        uint64      // read only: inode number
 	f            fs.Fs       // read only
@@ -36,7 +37,6 @@ type Dir struct {
 	read    time.Time         // time directory entry last read
 	items   map[string]Node   // directory entries - can be empty but not nil
 	virtual map[string]vState // virtual directory entries - may be nil
-	sys     atomic.Value      // user defined info to be attached here

 	modTimeMu sync.Mutex // protects the following
 	modTime   time.Time
@@ -181,16 +181,6 @@ func (d *Dir) Path() (name string) {
 	return d.path
 }

-// Sys returns underlying data source (can be nil) - satisfies Node interface
-func (d *Dir) Sys() any {
-	return d.sys.Load()
-}
-
-// SetSys sets the underlying data source (can be nil) - satisfies Node interface
-func (d *Dir) SetSys(x any) {
-	d.sys.Store(x)
-}
-
 // Inode returns the inode number - satisfies Node interface
 func (d *Dir) Inode() uint64 {
 	return d.inode
@@ -40,6 +40,7 @@ import (

 // File represents a file or a symlink
 type File struct {
+	aux                   // values attached by users of the VFS
 	inode uint64          // inode number - read only
 	size  atomic.Int64    // size of file
 	ctx   context.Context // context for VFS operations - read only
@@ -55,7 +56,6 @@ type File struct {
 	virtualModTime   *time.Time                      // modtime for backends with Precision == fs.ModTimeNotSupported
 	pendingModTime   time.Time                       // will be applied once o becomes available, i.e. after file was written
 	pendingRenameFun func(ctx context.Context) error // will be run/renamed after all writers close
-	sys              atomic.Value                    // user defined info to be attached here
 	nwriters         atomic.Int32                    // len(writers)
 	appendMode       bool                            // file was opened with O_APPEND
 	isLink           bool                            // file represents a symlink
@@ -184,16 +184,6 @@ func (f *File) CachePath() string {
 	return f._cachePath()
 }

-// Sys returns underlying data source (can be nil) - satisfies Node interface
-func (f *File) Sys() any {
-	return f.sys.Load()
-}
-
-// SetSys sets the underlying data source (can be nil) - satisfies Node interface
-func (f *File) SetSys(x any) {
-	f.sys.Store(x)
-}
-
 // Inode returns the inode number - satisfies Node interface
 func (f *File) Inode() uint64 {
 	return f.inode
@@ -69,6 +69,8 @@ type Node interface {
 	Truncate(size int64) error
 	Path() string
 	SetSys(any)
+	Aux(owner any) any
+	SetAux(owner, value any)
 }

 // Check interfaces