diff --git a/pkg/sentry/kernel/task_exit.go b/pkg/sentry/kernel/task_exit.go index 26c3679f8..b5c2ff6c4 100644 --- a/pkg/sentry/kernel/task_exit.go +++ b/pkg/sentry/kernel/task_exit.go @@ -737,13 +737,13 @@ func (t *Task) exitNotifyLocked(fromPtraceDetach bool) { ns.deleteTask(t) } t.userCounters.decRLimitNProc() - t.tg.exitedCPUStats.Accumulate(t.CPUStats()) - t.tg.ioUsage.Accumulate(t.ioUsage) t.tg.signalHandlers.mu.Lock() t.tg.tasks.Remove(t) t.tg.tasksCount-- tc := t.tg.tasksCount + t.tg.exitedCPUStats.Accumulate(t.CPUStats()) t.tg.signalHandlers.mu.Unlock() + t.tg.ioUsage.Accumulate(t.ioUsage) if tc == 1 && t != t.tg.leader { // Our fromPtraceDetach doesn't matter here (in Linux terms, this // is via a call to release_task()). diff --git a/pkg/sentry/kernel/task_sched.go b/pkg/sentry/kernel/task_sched.go index 8d6270d41..3a63e64cb 100644 --- a/pkg/sentry/kernel/task_sched.go +++ b/pkg/sentry/kernel/task_sched.go @@ -204,7 +204,7 @@ func (tg *ThreadGroup) CPUStats() usage.CPUStats { } // Preconditions: Same as TaskGoroutineSchedInfo.userTicksAt, plus: -// - The TaskSet mutex must be locked. +// - Either the TaskSet mutex or the signal mutex must be locked. func (tg *ThreadGroup) cpuStatsAtLocked(now uint64) usage.CPUStats { stats := tg.exitedCPUStats // Account for live tasks. @@ -375,12 +375,8 @@ func (k *Kernel) runCPUClockTicker() { continue } - k.tasks.mu.RLock() - if tg.leader == nil { - // No tasks have ever run in this thread group. - k.tasks.mu.RUnlock() - continue - } + sh := tg.signalLock() + // Accumulate thread group CPU stats, and randomly select running tasks // using reservoir sampling to receive CPU timer signals. var virtReceiver *Task @@ -415,7 +411,6 @@ func (k *Kernel) runCPUClockTicker() { // All of the following are standard (not real-time) signals, which are // automatically deduplicated, so we ignore the number of expirations. - tg.signalHandlers.mu.Lock() // It should only be possible for these timers to advance if we found // at least one running task. if virtReceiver != nil { @@ -445,9 +440,8 @@ func (k *Kernel) runCPUClockTicker() { profReceiver.sendSignalLocked(SignalInfoPriv(linux.SIGKILL), true) } } - tg.signalHandlers.mu.Unlock() - k.tasks.mu.RUnlock() + sh.mu.Unlock() } k.cpuClockMu.Unlock() diff --git a/pkg/sentry/kernel/thread_group.go b/pkg/sentry/kernel/thread_group.go index 5ec71a7c1..336a4e481 100644 --- a/pkg/sentry/kernel/thread_group.go +++ b/pkg/sentry/kernel/thread_group.go @@ -204,7 +204,11 @@ type ThreadGroup struct { nextTimerID linux.TimerID // exitedCPUStats is the CPU usage for all exited tasks in the thread - // group. exitedCPUStats is protected by the TaskSet mutex. + // group. exitedCPUStats is protected by both the TaskSet mutex and the + // signal mutex. Mutating it requires that the TaskSet mutex is locked for + // writing *and* that the signal mutex is locked. Reading it requires + // locking the TaskSet mutex (for reading or writing) *or* locking the + // signal mutex. exitedCPUStats usage.CPUStats // childCPUStats is the CPU usage of all joined descendants of this thread