diff options
| -rw-r--r-- | pkg/sentry/fsimpl/mqfs/mqfs.go | 5 | ||||
| -rw-r--r-- | pkg/sentry/fsimpl/mqfs/registry.go | 23 | ||||
| -rw-r--r-- | pkg/sentry/fsimpl/mqfs/root.go | 9 | ||||
| -rw-r--r-- | pkg/sentry/vfs/filesystem_type.go | 2 | ||||
| -rw-r--r-- | test/syscalls/linux/BUILD | 1 | ||||
| -rw-r--r-- | test/syscalls/linux/mq.cc | 22 |
6 files changed, 27 insertions, 35 deletions
diff --git a/pkg/sentry/fsimpl/mqfs/mqfs.go b/pkg/sentry/fsimpl/mqfs/mqfs.go index c2b53c9d0..9b3704217 100644 --- a/pkg/sentry/fsimpl/mqfs/mqfs.go +++ b/pkg/sentry/fsimpl/mqfs/mqfs.go @@ -75,6 +75,7 @@ func (ft FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.VirtualF impl.fs.MaxCachedDentries = maxCachedDentries impl.fs.VFSFilesystem().IncRef() + impl.root.IncRef() return impl.fs.VFSFilesystem(), impl.root.VFSDentry(), nil } @@ -100,10 +101,6 @@ func maxCachedDentries(ctx context.Context, mopts map[string]string) (_ uint64, type filesystem struct { kernfs.Filesystem devMinor uint32 - - // root is the filesystem's root dentry. Since we take a reference on it in - // GetFilesystem, we should release it when the fs is released. - root *kernfs.Dentry } // Release implements vfs.FilesystemImpl.Release. diff --git a/pkg/sentry/fsimpl/mqfs/registry.go b/pkg/sentry/fsimpl/mqfs/registry.go index c8fbe4d33..e470ffadc 100644 --- a/pkg/sentry/fsimpl/mqfs/registry.go +++ b/pkg/sentry/fsimpl/mqfs/registry.go @@ -57,16 +57,19 @@ func NewRegistryImpl(ctx context.Context, vfsObj *vfs.VirtualFilesystem, creds * return nil, err } - var dentry kernfs.Dentry fs := &filesystem{ devMinor: devMinor, - root: &dentry, } fs.VFSFilesystem().Init(vfsObj, &FilesystemType{}, fs) vfsfs := fs.VFSFilesystem() + // NewDisconnectedMount will obtain a ref on dentry and vfsfs which is + // transferred to mount. vfsfs was initiated with 1 ref already. So get rid + // of the extra ref. + defer vfsfs.DecRef(ctx) + // dentry is initialized with 1 ref which is transferred to fs. + var dentry kernfs.Dentry dentry.InitRoot(&fs.Filesystem, fs.newRootInode(ctx, creds)) - defer vfsfs.DecRef(ctx) // NewDisconnectedMount will obtain a ref on success. mount, err := vfsObj.NewDisconnectedMount(vfsfs, dentry.VFSDentry(), &vfs.MountOptions{}) if err != nil { @@ -82,7 +85,7 @@ func NewRegistryImpl(ctx context.Context, vfsObj *vfs.VirtualFilesystem, creds * // Get implements mq.RegistryImpl.Get. func (r *RegistryImpl) Get(ctx context.Context, name string, access mq.AccessType, block bool, flags uint32) (*vfs.FileDescription, bool, error) { - inode, err := r.lookup(ctx, name) + inode, err := r.root.Inode().(*rootInode).Lookup(ctx, name) if err != nil { return nil, false, nil } @@ -120,7 +123,7 @@ func (r *RegistryImpl) Unlink(ctx context.Context, name string) error { } root := r.root.Inode().(*rootInode) - inode, err := r.lookup(ctx, name) + inode, err := root.Lookup(ctx, name) if err != nil { return err } @@ -133,16 +136,6 @@ func (r *RegistryImpl) Destroy(ctx context.Context) { r.mount.DecRef(ctx) } -// lookup retreives a kernfs.Inode using a name. -func (r *RegistryImpl) lookup(ctx context.Context, name string) (kernfs.Inode, error) { - inode := r.root.Inode().(*rootInode) - lookup, err := inode.Lookup(ctx, name) - if err != nil { - return nil, err - } - return lookup, nil -} - // newFD returns a new file description created using the given queue and inode. func (r *RegistryImpl) newFD(q *mq.Queue, inode *queueInode, access mq.AccessType, block bool, flags uint32) (*vfs.FileDescription, error) { view, err := mq.NewView(q, access, block) diff --git a/pkg/sentry/fsimpl/mqfs/root.go b/pkg/sentry/fsimpl/mqfs/root.go index 37b5749fb..922e669e2 100644 --- a/pkg/sentry/fsimpl/mqfs/root.go +++ b/pkg/sentry/fsimpl/mqfs/root.go @@ -34,7 +34,6 @@ type rootInode struct { kernfs.InodeNotSymlink kernfs.InodeTemporary kernfs.OrderedChildren - implStatFS locks vfs.FileLocks } @@ -77,13 +76,7 @@ func (*rootInode) SetStat(context.Context, *vfs.Filesystem, *auth.Credentials, v return linuxerr.EPERM } -// implStatFS provides an implementation of kernfs.Inode.StatFS for message -// queues to be embedded in inodes. -// -// +stateify savable -type implStatFS struct{} - // StatFS implements kernfs.Inode.StatFS. -func (*implStatFS) StatFS(context.Context, *vfs.Filesystem) (linux.Statfs, error) { +func (*rootInode) StatFS(context.Context, *vfs.Filesystem) (linux.Statfs, error) { return vfs.GenericStatFS(linux.MQUEUE_MAGIC), nil } diff --git a/pkg/sentry/vfs/filesystem_type.go b/pkg/sentry/vfs/filesystem_type.go index 9d54cc4ed..70d6eb85e 100644 --- a/pkg/sentry/vfs/filesystem_type.go +++ b/pkg/sentry/vfs/filesystem_type.go @@ -28,7 +28,7 @@ import ( type FilesystemType interface { // GetFilesystem returns a Filesystem configured by the given options, // along with its mount root. A reference is taken on the returned - // Filesystem and Dentry. + // Filesystem and Dentry whose ownership is transferred to the caller. GetFilesystem(ctx context.Context, vfsObj *VirtualFilesystem, creds *auth.Credentials, source string, opts GetFilesystemOptions) (*Filesystem, *Dentry, error) // Name returns the name of this FilesystemType. diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 9d975c614..6217ff4dc 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -4209,6 +4209,7 @@ cc_binary( linkstatic = 1, deps = [ "//test/util:capability_util", + "//test/util:cleanup", "//test/util:fs_util", "//test/util:mount_util", "//test/util:posix_error", diff --git a/test/syscalls/linux/mq.cc b/test/syscalls/linux/mq.cc index 839877a67..013994fd9 100644 --- a/test/syscalls/linux/mq.cc +++ b/test/syscalls/linux/mq.cc @@ -22,6 +22,7 @@ #include <string> #include "test/util/capability_util.h" +#include "test/util/cleanup.h" #include "test/util/fs_util.h" #include "test/util/mount_util.h" #include "test/util/posix_error.h" @@ -286,18 +287,25 @@ TEST(MqTest, Mount) { TEST(MqTest, MountSeveral) { SKIP_IF(IsRunningWithVFS1() || !ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); + constexpr int numMounts = 3; + // mountDirs should outlive mountCUs and queue so that its destructor succeeds + // in unlinking the mountpoints and does not interfere with queue destruction. + testing::TempPath mountDirs[numMounts]; + testing::Cleanup mountCUs[numMounts]; PosixQueue queue = ASSERT_NO_ERRNO_AND_VALUE( MqOpen(O_RDWR | O_CREAT | O_EXCL, 0777, nullptr)); - auto const dir1 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); - // Assign the pointer so it doesn't get destroyed before the second mount is - // created. - auto mnt = - ASSERT_NO_ERRNO_AND_VALUE(Mount("none", dir1.path(), "mqueue", 0, "", 0)); + for (int i = 0; i < numMounts; ++i) { + mountDirs[i] = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + mountCUs[i] = ASSERT_NO_ERRNO_AND_VALUE( + Mount("none", mountDirs[i].path(), "mqueue", 0, "", 0)); + } - auto const dir2 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); - ASSERT_NO_ERRNO(Mount("none", dir2.path(), "mqueue", 0, "", 0)); + // Ensure that queue is visible from all mounts. + for (int i = 0; i < numMounts; ++i) { + ASSERT_NO_ERRNO(Stat(JoinPath(mountDirs[i].path(), queue.name()))); + } } // Test mounting mqueue and opening a queue as normal file. |