From 637730674343c9848985d033dc7e6612d348cf71 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Thu, 31 Mar 2022 11:52:30 -0700 Subject: [PATCH] Fix circular locking of kernel.taskMutex and kernfs.filesystemRWMutex mntns.DecRef can be called without holding task.Mu. panic: WARNING: circular locking detected: kernel.taskMutex -> kernfs.filesystemRWMutex: goroutine 68 [running]: gvisor.dev/gvisor/pkg/log.Stacks(0x40) pkg/log/log.go:313 +0x9e gvisor.dev/gvisor/pkg/sync/locking.checkLock(0xc0000b2780, 0xc0000b3b80, {0x0, 0x0, 0x739539}) pkg/sync/locking/lockdep.go:53 +0x14a gvisor.dev/gvisor/pkg/sync/locking.AddGLock(0xc0000b2780, 0x0) pkg/sync/locking/lockdep.go:102 +0x27d gvisor.dev/gvisor/pkg/sentry/fsimpl/kernfs.(*filesystemRWMutex).Lock(0xc0001e0230) bazel-out/k8-fastbuild-ST-1a50d7a562c6/bin/pkg/sentry/fsimpl/kernfs/filesystem_mutex.go:16 +0x34 gvisor.dev/gvisor/pkg/sentry/fsimpl/kernfs.(*Filesystem).Release(0xc0001e01e0, {0x158ef50, 0xc00046ca80}) pkg/sentry/fsimpl/kernfs/filesystem.go:266 +0x67 gvisor.dev/gvisor/pkg/sentry/fsimpl/proc.(*filesystem).Release(0xc0001e01e0, {0x158ef50, 0xc00046ca80}) pkg/sentry/fsimpl/proc/filesystem.go:104 +0x6b gvisor.dev/gvisor/pkg/sentry/vfs.(*Filesystem).DecRef.func1() pkg/sentry/vfs/filesystem.go:81 +0xbd gvisor.dev/gvisor/pkg/sentry/vfs.(*FilesystemRefs).DecRef(0xc00046ca80, 0xc0005bbd00) bazel-out/k8-fastbuild-ST-1a50d7a562c6/bin/pkg/sentry/vfs/filesystem_refs.go:131 +0x74 gvisor.dev/gvisor/pkg/sentry/vfs.(*Filesystem).DecRef(0xc000512500, {0x158ef50, 0xc00046ca80}) pkg/sentry/vfs/filesystem.go:77 +0x5b gvisor.dev/gvisor/pkg/sentry/vfs.(*Mount).destroy(0xc000312d80, {0x158ef50, 0xc00046ca80}) pkg/sentry/vfs/mount.go:544 +0x159 gvisor.dev/gvisor/pkg/sentry/vfs.(*Mount).DecRef(0xc000312d80, {0x158ef50, 0xc00046ca80}) pkg/sentry/vfs/mount.go:528 +0x99 gvisor.dev/gvisor/pkg/sentry/vfs.(*MountNamespace).DecRef.func1() pkg/sentry/vfs/mount.go:583 +0x157 gvisor.dev/gvisor/pkg/sentry/vfs.(*MountNamespaceRefs).DecRef(0x0, 0xc0005bbed0) bazel-out/k8-fastbuild-ST-1a50d7a562c6/bin/pkg/sentry/vfs/mount_namespace_refs.go:131 +0x74 gvisor.dev/gvisor/pkg/sentry/vfs.(*MountNamespace).DecRef(0xc00046d238, {0x158ef50, 0xc00046ca80}) pkg/sentry/vfs/mount.go:571 +0x87 gvisor.dev/gvisor/pkg/sentry/kernel.(*runExitMain).execute(0xc0004932a8, 0xc00046ca80) pkg/sentry/kernel/task_exit.go:256 +0x27f gvisor.dev/gvisor/pkg/sentry/kernel.(*Task).run(0xc00046ca80, 0x1) pkg/sentry/kernel/task_run.go:95 +0x1ec created by gvisor.dev/gvisor/pkg/sentry/kernel.(*Task).Start pkg/sentry/kernel/task_start.go:345 +0x106 known lock chain: kernfs.filesystemRWMutex -> kernel.taskMutex ====== kernfs.filesystemRWMutex -> kernel.taskMutex ===== goroutine 68 [running]: gvisor.dev/gvisor/pkg/log.Stacks(0x80) pkg/log/log.go:313 +0x9e gvisor.dev/gvisor/pkg/sync/locking.AddGLock(0xc0000b3b80, 0x0) pkg/sync/locking/lockdep.go:105 +0x2b5 gvisor.dev/gvisor/pkg/sentry/kernel.(*taskMutex).Lock(0xc00046d238) bazel-out/k8-fastbuild-ST-1a50d7a562c6/bin/pkg/sentry/kernel/task_mutex.go:16 +0x34 gvisor.dev/gvisor/pkg/sentry/kernel.(*Task).WithMuLocked(0xc00046ca80, 0xc0005bad78) pkg/sentry/kernel/task.go:818 +0x3e gvisor.dev/gvisor/pkg/sentry/fsimpl/proc.getMM(0x6245f69b) pkg/sentry/fsimpl/proc/task_files.go:48 +0x4d gvisor.dev/gvisor/pkg/sentry/fsimpl/proc.(*taskOwnedInode).getOwner(0x1000000001ff, 0x0) pkg/sentry/fsimpl/proc/task.go:207 +0xb2 gvisor.dev/gvisor/pkg/sentry/fsimpl/proc.(*taskOwnedInode).Stat(0xc00049d2a8, {0x158ef50, 0xc00046ca80}, 0x0, {0x4, 0x0}) pkg/sentry/fsimpl/proc/task.go:174 +0x117 gvisor.dev/gvisor/pkg/sentry/fsimpl/kernfs.(*Filesystem).StatAt(0xc0001e01e0, {0x158ef50, 0xc00046ca80}, 0xc00058c758, {0xb2a80, 0xc0}) pkg/sentry/fsimpl/kernfs/filesystem.go:837 +0x262 gvisor.dev/gvisor/pkg/sentry/vfs.(*VirtualFilesystem).StatAt(0xc0005bb6f0, {0x158ef50, 0xc00046ca80}, 0xc0005bb6f0, 0xd9f54f, 0xc0005bb6c0) pkg/sentry/vfs/vfs.go:598 +0xe3 gvisor.dev/gvisor/pkg/sentry/syscalls/linux/vfs2.fstatat(0xc00046ca80, 0x7, 0x0, 0xc0005bbc14, 0x100) pkg/sentry/syscalls/linux/vfs2/stat.go:104 +0x5d1 gvisor.dev/gvisor/pkg/sentry/syscalls/linux/vfs2.Newfstatat(0x65, {{0x7}, {0x56043e7212c0}, {0x56043e721230}, {0x100}, {0x100}, {0x1}}) pkg/sentry/syscalls/linux/vfs2/stat.go:50 +0x5f gvisor.dev/gvisor/pkg/sentry/kernel.(*Task).executeSyscall(0xc00046ca80, 0x106, {{0x7}, {0x56043e7212c0}, {0x56043e721230}, {0x100}, {0x100}, {0x1}}) pkg/sentry/kernel/task_syscall.go:103 +0x2d7 gvisor.dev/gvisor/pkg/sentry/kernel.(*Task).doSyscallInvoke(0xc00046ca80, 0xf6a220, {{0x7}, {0x56043e7212c0}, {0x56043e721230}, {0x100}, {0x100}, {0x1}}) pkg/sentry/kernel/task_syscall.go:238 +0x57 gvisor.dev/gvisor/pkg/sentry/kernel.(*Task).doSyscallEnter(0xc0000c9c70, 0x15a3038, {{0x7}, {0x56043e7212c0}, {0x56043e721230}, {0x100}, {0x100}, {0x1}}) pkg/sentry/kernel/task_syscall.go:198 +0x85 gvisor.dev/gvisor/pkg/sentry/kernel.(*Task).doSyscall(0xc00046ca80) pkg/sentry/kernel/task_syscall.go:173 +0x38e gvisor.dev/gvisor/pkg/sentry/kernel.(*runApp).execute(0xc0004932a8, 0xc00046ca80) pkg/sentry/kernel/task_run.go:254 +0x10fb gvisor.dev/gvisor/pkg/sentry/kernel.(*Task).run(0xc00046ca80, 0x1) pkg/sentry/kernel/task_run.go:95 +0x1ec created by gvisor.dev/gvisor/pkg/sentry/kernel.(*Task).Start pkg/sentry/kernel/task_start.go:345 +0x106 --- pkg/sentry/kernel/task_clone.go | 6 +++++- pkg/sentry/kernel/task_exit.go | 12 +++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/pkg/sentry/kernel/task_clone.go b/pkg/sentry/kernel/task_clone.go index fe43b0bd6..a84b88661 100644 --- a/pkg/sentry/kernel/task_clone.go +++ b/pkg/sentry/kernel/task_clone.go @@ -464,6 +464,7 @@ func (t *Task) Unshare(flags int32) error { // new user namespace is used if there is one. t.utsns = t.utsns.Clone(creds.UserNamespace) } + var oldIPCNS *IPCNamespace if flags&linux.CLONE_NEWIPC != 0 { if !haveCapSysAdmin { t.mu.Unlock() @@ -471,7 +472,7 @@ func (t *Task) Unshare(flags int32) error { } // Note that "If CLONE_NEWIPC is set, then create the process in a new IPC // namespace" - t.ipcns.DecRef(t) + oldIPCNS = t.ipcns t.ipcns = NewIPCNamespace(creds.UserNamespace) if VFS2Enabled { t.ipcns.InitPosixQueues(t, t.k.VFS(), creds) @@ -488,6 +489,9 @@ func (t *Task) Unshare(flags int32) error { t.fsContext = oldFSContext.Fork() } t.mu.Unlock() + if oldIPCNS != nil { + oldIPCNS.DecRef(t) + } if oldFDTable != nil { oldFDTable.DecRef(t) } diff --git a/pkg/sentry/kernel/task_exit.go b/pkg/sentry/kernel/task_exit.go index 822dfb45a..a74638711 100644 --- a/pkg/sentry/kernel/task_exit.go +++ b/pkg/sentry/kernel/task_exit.go @@ -252,12 +252,14 @@ func (*runExitMain) execute(t *Task) taskRunState { t.LeaveCgroups() t.mu.Lock() - if t.mountNamespaceVFS2 != nil { - t.mountNamespaceVFS2.DecRef(t) - t.mountNamespaceVFS2 = nil - } - t.ipcns.DecRef(t) + mntns := t.mountNamespaceVFS2 + t.mountNamespaceVFS2 = nil + ipcns := t.ipcns t.mu.Unlock() + if mntns != nil { + mntns.DecRef(t) + } + ipcns.DecRef(t) // If this is the last task to exit from the thread group, release the // thread group's resources.