From a248c63cd5df307b32bb2a7526bb2648249c4cfb Mon Sep 17 00:00:00 2001
From: Lucas Manning <lucasmanning@google.com>
Date: Mon, 9 Jan 2023 10:22:37 -0800
Subject: [PATCH] Fix circular lock between filesystemRWMutex and
 taskSetRWMutex.

PiperOrigin-RevId: 500747195
---
 pkg/sentry/kernel/seccheck.go   | 40 ++++++++++++++++++++++++---------
 pkg/sentry/kernel/task_clone.go |  7 ++++--
 pkg/sentry/kernel/task_exit.go  |  4 +++-
 pkg/sentry/seccheck/metadata.go | 39 +++++++++++++++++++++++++++++---
 4 files changed, 74 insertions(+), 16 deletions(-)

diff --git a/pkg/sentry/kernel/seccheck.go b/pkg/sentry/kernel/seccheck.go
index 2e2a86c60..aab2694c2 100644
--- a/pkg/sentry/kernel/seccheck.go
+++ b/pkg/sentry/kernel/seccheck.go
@@ -19,17 +19,44 @@ import (
 	pb "gvisor.dev/gvisor/pkg/sentry/seccheck/points/points_go_proto"
 )
 
+func getTaskCurrentWorkingDirectory(t *Task) string {
+	// Grab the filesystem context first since it needs tasks.mu to be locked.
+	// It's safe to unlock and use the values obtained here as long as there's
+	// no way to modify root and wd from a separate task.
+	t.k.tasks.mu.RLock()
+	root := t.FSContext().RootDirectory()
+	wd := t.FSContext().WorkingDirectory()
+	t.k.tasks.mu.RUnlock()
+
+	// Perform VFS operations outside of task mutex to avoid circular locking with
+	// filesystem mutexes.
+	var cwd string
+	if root.Ok() {
+		defer root.DecRef(t)
+		if wd.Ok() {
+			defer wd.DecRef(t)
+			vfsObj := root.Mount().Filesystem().VirtualFilesystem()
+			cwd, _ = vfsObj.PathnameWithDeleted(t, root, wd)
+		}
+	}
+	return cwd
+}
+
 // LoadSeccheckData sets info from the task based on mask.
 func LoadSeccheckData(t *Task, mask seccheck.FieldMask, info *pb.ContextData) {
+	var cwd string
+	if mask.Contains(seccheck.FieldCtxtCwd) {
+		cwd = getTaskCurrentWorkingDirectory(t)
+	}
 	t.k.tasks.mu.RLock()
 	defer t.k.tasks.mu.RUnlock()
-	LoadSeccheckDataLocked(t, mask, info)
+	LoadSeccheckDataLocked(t, mask, info, cwd)
 }
 
 // LoadSeccheckDataLocked sets info from the task based on mask.
 //
 // Preconditions: The TaskSet mutex must be locked.
-func LoadSeccheckDataLocked(t *Task, mask seccheck.FieldMask, info *pb.ContextData) {
+func LoadSeccheckDataLocked(t *Task, mask seccheck.FieldMask, info *pb.ContextData, cwd string) {
 	if mask.Contains(seccheck.FieldCtxtTime) {
 		info.TimeNs = t.k.RealtimeClock().Now().Nanoseconds()
 	}
@@ -49,14 +76,7 @@ func LoadSeccheckDataLocked(t *Task, mask seccheck.FieldMask, info *pb.ContextDa
 		info.ContainerId = t.tg.leader.ContainerID()
 	}
 	if mask.Contains(seccheck.FieldCtxtCwd) {
-		if root := t.FSContext().RootDirectory(); root.Ok() {
-			defer root.DecRef(t)
-			if wd := t.FSContext().WorkingDirectory(); wd.Ok() {
-				defer wd.DecRef(t)
-				vfsObj := root.Mount().Filesystem().VirtualFilesystem()
-				info.Cwd, _ = vfsObj.PathnameWithDeleted(t, root, wd)
-			}
-		}
+		info.Cwd = cwd
 	}
 	if mask.Contains(seccheck.FieldCtxtProcessName) {
 		info.ProcessName = t.Name()
diff --git a/pkg/sentry/kernel/task_clone.go b/pkg/sentry/kernel/task_clone.go
index 5b33e75d1..91815c398 100644
--- a/pkg/sentry/kernel/task_clone.go
+++ b/pkg/sentry/kernel/task_clone.go
@@ -310,7 +310,10 @@ func (t *Task) Clone(args *linux.CloneArgs) (ThreadID, *SyscallControl, error) {
 
 func getCloneSeccheckInfo(t, nt *Task, flags uint64) (seccheck.FieldSet, *pb.CloneInfo) {
 	fields := seccheck.Global.GetFieldSet(seccheck.PointClone)
-
+	var cwd string
+	if fields.Context.Contains(seccheck.FieldCtxtCwd) {
+		cwd = getTaskCurrentWorkingDirectory(t)
+	}
 	t.k.tasks.mu.RLock()
 	defer t.k.tasks.mu.RUnlock()
 	info := &pb.CloneInfo{
@@ -322,7 +325,7 @@ func getCloneSeccheckInfo(t, nt *Task, flags uint64) (seccheck.FieldSet, *pb.Clo
 
 	if !fields.Context.Empty() {
 		info.ContextData = &pb.ContextData{}
-		LoadSeccheckDataLocked(t, fields.Context, info.ContextData)
+		LoadSeccheckDataLocked(t, fields.Context, info.ContextData, cwd)
 	}
 
 	return fields, info
diff --git a/pkg/sentry/kernel/task_exit.go b/pkg/sentry/kernel/task_exit.go
index 97d8751f7..ef4d9556c 100644
--- a/pkg/sentry/kernel/task_exit.go
+++ b/pkg/sentry/kernel/task_exit.go
@@ -748,7 +748,9 @@ func getExitNotifyParentSeccheckInfo(t *Task) (seccheck.FieldSet, *pb.ExitNotify
 	}
 	if !fields.Context.Empty() {
 		info.ContextData = &pb.ContextData{}
-		LoadSeccheckDataLocked(t, fields.Context, info.ContextData)
+		// cwd isn't used for notifyExit seccheck so it's ok to pass an empty
+		// string.
+		LoadSeccheckDataLocked(t, fields.Context, info.ContextData, "")
 	}
 
 	return fields, info
diff --git a/pkg/sentry/seccheck/metadata.go b/pkg/sentry/seccheck/metadata.go
index 077751b11..fa2e55227 100644
--- a/pkg/sentry/seccheck/metadata.go
+++ b/pkg/sentry/seccheck/metadata.go
@@ -243,9 +243,42 @@ func init() {
 		ContextFields: defaultContextFields,
 	})
 	registerPoint(PointDesc{
-		ID:            PointExitNotifyParent,
-		Name:          "sentry/exit_notify_parent",
-		ContextFields: defaultContextFields,
+		ID:   PointExitNotifyParent,
+		Name: "sentry/exit_notify_parent",
+		ContextFields: []FieldDesc{
+			{
+				ID:   FieldCtxtTime,
+				Name: "time",
+			},
+			{
+				ID:   FieldCtxtThreadID,
+				Name: "thread_id",
+			},
+			{
+				ID:   FieldCtxtThreadStartTime,
+				Name: "task_start_time",
+			},
+			{
+				ID:   FieldCtxtThreadGroupID,
+				Name: "group_id",
+			},
+			{
+				ID:   FieldCtxtThreadGroupStartTime,
+				Name: "thread_group_start_time",
+			},
+			{
+				ID:   FieldCtxtContainerID,
+				Name: "container_id",
+			},
+			{
+				ID:   FieldCtxtCredentials,
+				Name: "credentials",
+			},
+			{
+				ID:   FieldCtxtProcessName,
+				Name: "process_name",
+			},
+		},
 	})
 	registerPoint(PointDesc{
 		ID:            PointTaskExit,