vfs: establish lock ordering for FilesystemImpl.PrependPath

- Add type parameter Filesystem to vfs/genericfstree, which is required to
  provide `ancestryMu sync.RWMutex`, and add such a RWMutex to all FSImpls that
  use genericfstree.

- Modify genericfstree.PrependPath() and genericfstree.IsDescendant() to use
  ancestryMu to ensure atomicity. For callers of genericfstree.PrependPath(),
  this means that (broader) FSImpl locks no longer need to be held during the
  call. For callers of genericfstree.IsDescendant(), this means that we can
  remove documentation warnings about its non-atomicity.

- Minor cleanup: Remove useless variable `start`, which is always 0, from
  MM.ReadMaps/SmapsDataInto().

PiperOrigin-RevId: 696713993
This commit is contained in:
Jamie Liu
2024-11-14 18:14:27 -08:00
committed by gVisor bot
parent 2dfab3e180
commit 0659b6035a
29 changed files with 239 additions and 209 deletions
+27 -18
View File
@@ -26,6 +26,7 @@ go_template_instance(
template = "//pkg/sentry/vfs/genericfstree:generic_fstree",
types = {
"Dentry": "dentry",
"Filesystem": "filesystem",
},
)
@@ -40,25 +41,11 @@ go_template_instance(
},
)
declare_mutex(
name = "inode_mutex",
out = "inode_mutex.go",
declare_rwmutex(
name = "ancestry_mutex",
out = "ancestry_mutex.go",
package = "tmpfs",
prefix = "inode",
)
declare_mutex(
name = "pages_used_mutex",
out = "pages_used_mutex.go",
package = "tmpfs",
prefix = "pagesUsed",
)
declare_mutex(
name = "iter_mutex",
out = "iter_mutex.go",
package = "tmpfs",
prefix = "iter",
prefix = "ancestry",
)
declare_rwmutex(
@@ -68,9 +55,31 @@ declare_rwmutex(
prefix = "filesystem",
)
declare_mutex(
name = "inode_mutex",
out = "inode_mutex.go",
package = "tmpfs",
prefix = "inode",
)
declare_mutex(
name = "iter_mutex",
out = "iter_mutex.go",
package = "tmpfs",
prefix = "iter",
)
declare_mutex(
name = "pages_used_mutex",
out = "pages_used_mutex.go",
package = "tmpfs",
prefix = "pagesUsed",
)
go_library(
name = "tmpfs",
srcs = [
"ancestry_mutex.go",
"dentry_list.go",
"device_file.go",
"directory.go",
+1 -2
View File
@@ -60,8 +60,7 @@ func (fs *filesystem) newDirectory(kuid auth.KUID, kgid auth.KGID, mode linux.Fi
// - filesystem.mu must be locked for writing.
// - dir must not already contain a child with the given name.
func (dir *directory) insertChildLocked(child *dentry, name string) {
child.parent.Store(&dir.dentry)
child.name = name
genericSetParentAndName(dir.dentry.inode.fs, child, &dir.dentry, name)
if dir.childMap == nil {
dir.childMap = make(map[string]*dentry)
}
+23 -34
View File
@@ -580,7 +580,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa
// mount point then we want to rename the mount point, not anything in the
// mounted filesystem.
if renamed.inode.isDir() {
if renamed == &newParentDir.dentry || genericIsAncestorDentry(renamed, &newParentDir.dentry) {
if renamed == &newParentDir.dentry || genericIsAncestorDentry(fs, renamed, &newParentDir.dentry) {
return linuxerr.EINVAL
}
if oldParentDir != newParentDir {
@@ -936,37 +936,31 @@ func (fs *filesystem) RemoveXattrAt(ctx context.Context, rp *vfs.ResolvingPath,
// PrependPath implements vfs.FilesystemImpl.PrependPath.
func (fs *filesystem) PrependPath(ctx context.Context, vfsroot, vd vfs.VirtualDentry, b *fspath.Builder) error {
fs.mu.RLock()
defer fs.mu.RUnlock()
mnt := vd.Mount()
d := vd.Dentry().Impl().(*dentry)
for {
if mnt == vfsroot.Mount() && &d.vfsd == vfsroot.Dentry() {
return vfs.PrependPathAtVFSRootError{}
if d.parent.Load() == nil {
fs.ancestryMu.Lock()
name := d.name
fs.ancestryMu.Unlock()
if name != "" {
// This file must have been created by
// newUnlinkedRegularFileDescription(). In Linux,
// mm/shmem.c:__shmem_file_setup() =>
// fs/file_table.c:alloc_file_pseudo() sets the created
// dentry's dentry_operations to anon_ops, for which d_dname ==
// simple_dname. fs/d_path.c:simple_dname() defines the
// dentry's pathname to be its name, prefixed with "/" and
// suffixed with " (deleted)".
b.PrependComponent("/" + name)
b.AppendString(" (deleted)")
return vfs.PrependPathSyntheticError{}
}
if mnt != nil && &d.vfsd == mnt.Root() {
return nil
}
parent := d.parent.Load()
if parent == nil {
if d.name != "" {
// This file must have been created by
// newUnlinkedRegularFileDescription(). In Linux,
// mm/shmem.c:__shmem_file_setup() =>
// fs/file_table.c:alloc_file_pseudo() sets the created
// dentry's dentry_operations to anon_ops, for which d_dname ==
// simple_dname. fs/d_path.c:simple_dname() defines the
// dentry's pathname to be its name, prefixed with "/" and
// suffixed with " (deleted)".
b.PrependComponent("/" + d.name)
b.AppendString(" (deleted)")
return vfs.PrependPathSyntheticError{}
}
return vfs.PrependPathAtNonMountRootError{}
}
b.PrependComponent(d.name)
d = parent
}
return genericPrependPath(fs, vfsroot, vd.Mount(), d, b)
}
// IsDescendant implements vfs.FilesystemImpl.IsDescendant.
func (fs *filesystem) IsDescendant(vfsroot, vd vfs.VirtualDentry) bool {
return genericIsDescendant(fs, vfsroot.Dentry(), vd.Dentry().Impl().(*dentry))
}
// MountOptions implements vfs.FilesystemImpl.MountOptions.
@@ -974,11 +968,6 @@ func (fs *filesystem) MountOptions() string {
return fs.mopts
}
// IsDescendant implements vfs.FilesystemImpl.IsDescendant.
func (fs *filesystem) IsDescendant(vfsroot, vd vfs.VirtualDentry) bool {
return genericIsDescendant(vfsroot.Dentry(), vd.Dentry().Impl().(*dentry))
}
// adjustPageAcct adjusts the accounting done against filesystem size limit in
// case there is any discrepancy between the number of pages reserved vs the
// number of pages actually allocated.
+7 -1
View File
@@ -20,11 +20,12 @@
// filesystem.mu
// inode.mu
// regularFileFD.offMu
// *** "memmap.Mappable locks" below this point
// *** "memmap.Mappable/MappingIdentity locks" below this point
// regularFile.mapsMu
// *** "memmap.Mappable locks taken by Translate" below this point
// regularFile.dataMu
// fs.pagesUsedMu
// filesystem.ancestryMu
// directory.iterMu
package tmpfs
@@ -46,6 +47,7 @@ import (
"gvisor.dev/gvisor/pkg/sentry/usage"
"gvisor.dev/gvisor/pkg/sentry/vfs"
"gvisor.dev/gvisor/pkg/sentry/vfs/memxattr"
"gvisor.dev/gvisor/pkg/sync"
)
// Name is the default filesystem name.
@@ -83,6 +85,10 @@ type filesystem struct {
// mu serializes changes to the Dentry tree.
mu filesystemRWMutex `state:"nosave"`
// ancestryMu additionally protects dentry.parent and dentry.name as
// required by genericfstree.
ancestryMu sync.RWMutex `state:"nosave"`
nextInoMinusOne atomicbitops.Uint64 // accessed using atomic memory operations
root *dentry