From 574e988f2bc6060078a17f37a377441703c52a22 Mon Sep 17 00:00:00 2001 From: Fabricio Voznika Date: Mon, 23 Dec 2019 17:31:20 -0800 Subject: Fix deadlock in kernfs.Filesystem.revalidateChildLocked It was calling Dentry.InsertChild with the dentry's mutex already locked. Updates #1035 PiperOrigin-RevId: 286962742 --- pkg/sentry/fsimpl/kernfs/filesystem.go | 2 +- pkg/sentry/fsimpl/kernfs/kernfs.go | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) (limited to 'pkg/sentry/fsimpl/kernfs') diff --git a/pkg/sentry/fsimpl/kernfs/filesystem.go b/pkg/sentry/fsimpl/kernfs/filesystem.go index a6f9fced5..79759e0fc 100644 --- a/pkg/sentry/fsimpl/kernfs/filesystem.go +++ b/pkg/sentry/fsimpl/kernfs/filesystem.go @@ -122,7 +122,7 @@ func (fs *Filesystem) revalidateChildLocked(ctx context.Context, vfsObj *vfs.Vir return nil, err } // Reference on childVFSD dropped by a corresponding Valid. - parent.InsertChild(name, childVFSD) + parent.insertChildLocked(name, childVFSD) } return childVFSD.Impl().(*Dentry), nil } diff --git a/pkg/sentry/fsimpl/kernfs/kernfs.go b/pkg/sentry/fsimpl/kernfs/kernfs.go index bb01c3d01..ac802218d 100644 --- a/pkg/sentry/fsimpl/kernfs/kernfs.go +++ b/pkg/sentry/fsimpl/kernfs/kernfs.go @@ -239,14 +239,22 @@ func (d *Dentry) destroy() { // // Precondition: d must represent a directory inode. func (d *Dentry) InsertChild(name string, child *vfs.Dentry) { + d.dirMu.Lock() + d.insertChildLocked(name, child) + d.dirMu.Unlock() +} + +// insertChildLocked is equivalent to InsertChild, with additional +// preconditions. +// +// Precondition: d.dirMu must be locked. +func (d *Dentry) insertChildLocked(name string, child *vfs.Dentry) { if !d.isDir() { panic(fmt.Sprintf("InsertChild called on non-directory Dentry: %+v.", d)) } vfsDentry := d.VFSDentry() vfsDentry.IncRef() // DecRef in child's Dentry.destroy. - d.dirMu.Lock() vfsDentry.InsertChild(child, name) - d.dirMu.Unlock() } // The Inode interface maps filesystem-level operations that operate on paths to -- cgit v1.2.3