From b99fd8711f824724968bf61409076499bef74469 Mon Sep 17 00:00:00 2001
From: Jamie Liu <jamieliu@google.com>
Date: Tue, 1 Oct 2024 16:06:06 -0700
Subject: [PATCH] kernel: fix lock order inversion in ThreadGroup.Release()

PiperOrigin-RevId: 681199251
---
 pkg/sentry/kernel/kernel.go       | 13 +++++++------
 pkg/sentry/kernel/thread_group.go | 19 +++++++++++++------
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go
index 68f640ee41..41b32eeb97 100644
--- a/pkg/sentry/kernel/kernel.go
+++ b/pkg/sentry/kernel/kernel.go
@@ -19,12 +19,13 @@
 // Lock order (outermost locks must be taken first):
 //
 //	Kernel.extMu
-//		ThreadGroup.timerMu
-//		  ktime.Timer.mu (for IntervalTimer) and Kernel.cpuClockMu
-//		    TaskSet.mu
-//		      SignalHandlers.mu
-//		        Task.mu
-//		    runningTasksMu
+//	  TTY.mu
+//	  ThreadGroup.timerMu
+//	    ktime.Timer.mu (for IntervalTimer) and Kernel.cpuClockMu
+//	      TaskSet.mu
+//	        SignalHandlers.mu
+//	          Task.mu
+//	      runningTasksMu
 //
 // Locking SignalHandlers.mu in multiple SignalHandlers requires locking
 // TaskSet.mu exclusively first. Locking Task.mu in multiple Tasks at the same
diff --git a/pkg/sentry/kernel/thread_group.go b/pkg/sentry/kernel/thread_group.go
index 336a4e4814..a78e0fbffa 100644
--- a/pkg/sentry/kernel/thread_group.go
+++ b/pkg/sentry/kernel/thread_group.go
@@ -343,15 +343,22 @@ func (tg *ThreadGroup) Release(ctx context.Context) {
 	}
 	clear(tg.timers) // nil maps can't be saved
 	// Disassociate from the tty if we have one.
+	var tty *TTY
 	if tg.tty != nil {
-		tg.tty.mu.Lock() // FIXME(b/370763686)
-		if tg.tty.tg == tg {
-			tg.tty.tg = nil
-		}
-		tg.tty.mu.Unlock()
-		tg.tty = nil
+		// Can't lock tty.mu due to lock ordering.
+		tty = tg.tty
 	}
 	tg.signalHandlers.mu.Unlock()
+	if tty != nil {
+		tty.mu.Lock()
+		tg.signalHandlers.mu.Lock()
+		tg.tty = nil
+		if tty.tg == tg {
+			tty.tg = nil
+		}
+		tg.signalHandlers.mu.Unlock()
+		tty.mu.Unlock()
+	}
 	for _, it := range its {
 		it.DestroyTimer()
 	}