Skip to content

Commit 973cf76

Browse files
shailend-ggvisor-bot
authored andcommitted
Fix incorrect lock order in execve(2) around TaskSet.mu
FSContext.mu is made a lockdep mutex. Fixes the following cycle detected by syzcaller: goroutine 1 (execve): fs.mu.Lock() -> ts.mu.Rlock() goroutine 2 (CreateProcess): ts.mu.Lock() -> t.mu.Lock() goroutine 3 (unshare): t.mu.Lock() -> fs.mu.Lock() PiperOrigin-RevId: 805914251
1 parent 40dfee4 commit 973cf76

File tree

3 files changed

+14
-5
lines changed

3 files changed

+14
-5
lines changed

pkg/sentry/kernel/BUILD

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,13 @@ declare_mutex(
9494
prefix = "fdTable",
9595
)
9696

97+
declare_mutex(
98+
name = "fs_context_mutex",
99+
out = "fs_context_mutex.go",
100+
package = "kernel",
101+
prefix = "fsContext",
102+
)
103+
97104
declare_mutex(
98105
name = "running_tasks_mutex",
99106
out = "running_tasks_mutex.go",
@@ -245,6 +252,7 @@ go_library(
245252
"fd_table_refs.go",
246253
"fd_table_unsafe.go",
247254
"fs_context.go",
255+
"fs_context_mutex.go",
248256
"fs_context_refs.go",
249257
"ipc_namespace.go",
250258
"kcov.go",

pkg/sentry/kernel/fs_context.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919

2020
"gvisor.dev/gvisor/pkg/context"
2121
"gvisor.dev/gvisor/pkg/sentry/vfs"
22-
"gvisor.dev/gvisor/pkg/sync"
2322
)
2423

2524
// FSContext contains filesystem context.
@@ -31,7 +30,7 @@ type FSContext struct {
3130
FSContextRefs
3231

3332
// mu protects below.
34-
mu sync.Mutex `state:"nosave"`
33+
mu fsContextMutex `state:"nosave"`
3534

3635
// root is the filesystem root.
3736
root vfs.VirtualDentry
@@ -203,16 +202,17 @@ func (f *FSContext) SwapUmask(mask uint) uint {
203202
//
204203
// See Linux's fs_struct->in_exec.
205204
func (f *FSContext) checkAndPreventSharingOutsideTG(tg *ThreadGroup) bool {
205+
tg.pidns.owner.mu.RLock()
206+
defer tg.pidns.owner.mu.RUnlock()
206207
f.mu.Lock()
207208
defer f.mu.Unlock()
208209

209210
tgCount := int64(0)
210-
tg.ForEachTask(func(t *Task) bool {
211+
for t := tg.tasks.Front(); t != nil; t = t.Next() {
211212
if t.FSContext() == f {
212213
tgCount++
213214
}
214-
return true
215-
})
215+
}
216216

217217
shared := f.ReadRefs() > tgCount
218218
if !shared {

pkg/sentry/kernel/kernel.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
// TaskSet.mu
2727
// SignalHandlers.mu
2828
// Task.mu
29+
// FSContext.mu
2930
// runningTasksMu
3031
//
3132
// Locking SignalHandlers.mu in multiple SignalHandlers requires locking

0 commit comments

Comments
 (0)