From 2b55090a58136308bf8feb0f6b8d143bce8da2f1 Mon Sep 17 00:00:00 2001
From: Etienne Perot <eperot@google.com>
Date: Sat, 23 Nov 2024 01:25:18 -0800
Subject: [PATCH] Do not crash when creating thread group with already-exceeded
 soft CPU limit.

Reported-by: syzbot+da9595a72d0762aaa48d@syzkaller.appspotmail.com
PiperOrigin-RevId: 699425946
---
 pkg/sentry/kernel/task_start.go   | 13 ++++++++++++-
 pkg/sentry/kernel/thread_group.go |  1 -
 test/syscalls/linux/timers.cc     | 13 ++++++++-----
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/pkg/sentry/kernel/task_start.go b/pkg/sentry/kernel/task_start.go
index ded77133f..84260ba1e 100644
--- a/pkg/sentry/kernel/task_start.go
+++ b/pkg/sentry/kernel/task_start.go
@@ -213,6 +213,17 @@ func (ts *TaskSet) newTask(ctx context.Context, cfg *TaskConfig) (*Task, error)
 		}
 	}
 
+	// If the task was the first to be added to the thread group, check if
+	// it needs to be notified of CPU limits being exceeded.
+	// We use a defer here because we need to do this without holding the
+	// TaskSet or signalHandlers lock.
+	var isFirstTask bool
+	defer func() {
+		if isFirstTask {
+			tg.notifyRlimitCPUUpdated(t)
+		}
+	}()
+
 	// Make the new task (and possibly thread group) visible to the rest of
 	// the system atomically.
 	ts.mu.Lock()
@@ -259,7 +270,7 @@ func (ts *TaskSet) newTask(ctx context.Context, cfg *TaskConfig) (*Task, error)
 	t.EnterInitialCgroups(srcT, cfg.InitialCgroups)
 	committed = true
 
-	if tg.leader == nil {
+	if isFirstTask = tg.leader == nil; isFirstTask {
 		// New thread group.
 		tg.leader = t
 		if parentPG := tg.parentPG(); parentPG == nil {
diff --git a/pkg/sentry/kernel/thread_group.go b/pkg/sentry/kernel/thread_group.go
index c06485dac..26566de04 100644
--- a/pkg/sentry/kernel/thread_group.go
+++ b/pkg/sentry/kernel/thread_group.go
@@ -305,7 +305,6 @@ func (k *Kernel) NewThreadGroup(pidns *PIDNamespace, sh *SignalHandlers, termina
 	tg.rlimitCPUSoftListener.tg = tg
 	tg.rlimitCPUHardTimer.Init(&tg.appSysCPUClock, &tg.rlimitCPUHardListener)
 	tg.rlimitCPUHardListener.tg = tg
-	tg.notifyRlimitCPUUpdated(nil)
 	tg.oldRSeqCritical.Store(&OldRSeqCriticalRegion{})
 	return tg
 }
diff --git a/test/syscalls/linux/timers.cc b/test/syscalls/linux/timers.cc
index 4b215bf10..6260d7b04 100644
--- a/test/syscalls/linux/timers.cc
+++ b/test/syscalls/linux/timers.cc
@@ -238,15 +238,18 @@ TEST(TimerTest, RlimitCpuInheritedAcrossFork) {
     sigemptyset(&new_action.sa_mask);
     TEST_PCHECK(sigaction(SIGXCPU, &new_action, nullptr) == 0);
 
-    // Set both soft and hard limits to expire a short time from now. (Since we
-    // may not be able to raise RLIMIT_CPU again, this must happen in a
-    // disposable child of the test process.)
     constexpr int kDelaySeconds = 2;
     struct timespec ts;
     TEST_PCHECK(clock_gettime(CLOCK_PROCESS_CPUTIME_ID, &ts) == 0);
     struct rlimit cpu_limits;
+    // Set soft limit to 0 to expire immediately. This should cause
+    // a SIGXCPU to be sent to the grandchild immediately on fork.
+    cpu_limits.rlim_cur = 0;
+    // Set hard limit to expire a short time from now. (Since we
+    // may not be able to raise RLIMIT_CPU again, this must happen in a
+    // disposable child of the test process.)
     // +1 to round up, presuming that ts.tv_nsec > 0.
-    cpu_limits.rlim_cur = cpu_limits.rlim_max = ts.tv_sec + kDelaySeconds + 1;
+    cpu_limits.rlim_max = ts.tv_sec + kDelaySeconds + 1;
     TEST_PCHECK(setrlimit(RLIMIT_CPU, &cpu_limits) == 0);
     MaybeSave();
 
@@ -271,7 +274,7 @@ TEST(TimerTest, RlimitCpuInheritedAcrossFork) {
         // block in waitid().
         // TODO: b/315388929 - remove this
         if (x % 16384 == 0) {
-          TEST_PCHECK(ppoll(&pfd, 1, &timeout, nullptr) == 0);
+          TEST_PCHECK(RetryEINTR(ppoll)(&pfd, 1, &timeout, nullptr) == 0);
         }
       }
     }