Skip to content

Commit 8221e47

Browse files
nlacassegvisor-bot
authored andcommitted
Protect Task.ptraceSeized with TaskSet mutex and Signal mutex.
Previously Task.ptraceSeized was protected only by TaskSet mutex, but some pathways incorrectly modified the field without holding the lock, leading to data races. This CL changes the locking so that TaskSet and Signal mutexes are required to write Task.ptraceSeized, but only one of them must be held to read Task.ptraceSeized. Reported-by: [email protected] PiperOrigin-RevId: 742317413
1 parent d06b27e commit 8221e47

File tree

2 files changed

+9
-4
lines changed

2 files changed

+9
-4
lines changed

pkg/sentry/kernel/ptrace.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -525,8 +525,8 @@ func (t *Task) ptraceAttach(target *Task, seize bool, opts uintptr) error {
525525
}
526526
target.ptraceTracer.Store(t)
527527
t.ptraceTracees[target] = struct{}{}
528-
target.ptraceSeized = seize
529528
target.tg.signalHandlers.mu.Lock()
529+
target.ptraceSeized = seize
530530
// "Unlike PTRACE_ATTACH, PTRACE_SEIZE does not stop the process." -
531531
// ptrace(2)
532532
if !seize {
@@ -601,17 +601,19 @@ func (t *Task) exitPtraceLocked() {
601601
//
602602
// Preconditions: The TaskSet mutex must be locked for writing.
603603
func (t *Task) forgetTracerLocked() {
604-
t.ptraceSeized = false
605604
t.ptraceOpts = ptraceOptions{}
606605
t.ptraceSyscallMode = ptraceSyscallNone
607606
t.ptraceSinglestep = false
608607
t.ptraceTracer.Store(nil)
609608
if t.exitTracerNotified && !t.exitTracerAcked {
610609
t.exitTracerAcked = true
610+
// Don't hold signalHandlers.mu while calling exitNotifyLocked,
611+
// since it takes that mutex in some pathways.
611612
t.exitNotifyLocked(true)
612613
}
613614
t.tg.signalHandlers.mu.Lock()
614615
defer t.tg.signalHandlers.mu.Unlock()
616+
t.ptraceSeized = false
615617
// Unset t.trapStopPending, which might have been set by PTRACE_INTERRUPT. If
616618
// it wasn't, it will be reset via t.groupStopPending after the following.
617619
t.trapStopPending = false
@@ -803,6 +805,7 @@ func (t *Task) ptraceClone(kind ptraceCloneKind, child *Task, args *linux.CloneA
803805
if tracer != nil {
804806
child.ptraceTracer.Store(tracer)
805807
tracer.ptraceTracees[child] = struct{}{}
808+
child.tg.signalHandlers.mu.Lock()
806809
// "The "seized" behavior ... is inherited by children that are
807810
// automatically attached using PTRACE_O_TRACEFORK,
808811
// PTRACE_O_TRACEVFORK, and PTRACE_O_TRACECLONE." - ptrace(2)
@@ -811,7 +814,6 @@ func (t *Task) ptraceClone(kind ptraceCloneKind, child *Task, args *linux.CloneA
811814
// via active PTRACE_O_TRACEFORK, PTRACE_O_TRACEVFORK, or
812815
// PTRACE_O_TRACECLONE options." - ptrace(2)
813816
child.ptraceOpts = t.ptraceOpts
814-
child.tg.signalHandlers.mu.Lock()
815817
// "PTRACE_SEIZE: ... Automatically attached children stop with
816818
// PTRACE_EVENT_STOP and WSTOPSIG(status) returns SIGTRAP instead
817819
// of having SIGSTOP signal delivered to them." - ptrace(2)

pkg/sentry/kernel/task.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,10 @@ type Task struct {
364364
// ptraceSeized is true if ptraceTracer attached to this task with
365365
// PTRACE_SEIZE.
366366
//
367-
// ptraceSeized is protected by the TaskSet mutex.
367+
// ptraceSeized is protected by both the TaskSet mutex and the signal
368+
// mutex: Mutating ptraceSeized requires locking the TaskSet mutex for
369+
// writing *and* locking the signal mutex. Reading ptraceSeized
370+
// requires locking the TaskSet mutex *or* locking the signal mutex.
368371
ptraceSeized bool
369372

370373
// ptraceOpts contains ptrace options explicitly set by the tracer. If

0 commit comments

Comments
 (0)