Skip to content

Add a lockdep build config and syscall-test recipes - #14901

Open
shailend-g wants to merge 1 commit into
google:masterfrom
shailend-g:lockdep-build-config
Open

shailend-g wants to merge 1 commit into
google:masterfrom
shailend-g:lockdep-build-config

Conversation

@shailend-g

Copy link
Copy Markdown
Contributor

Promote the lockdep go build tag to a bazel config, so any target can be
built with the runtime lock-order checker via --config=lockdep, mirroring
--config=race. The lockdep gotag is added to go_binary and go_test targets
when //tools:lockdep is set.

Also document in the Makefile how to run syscall tests with lockdep and
with the race detector, add a SENTRY_SIDECAR_TARGET knob so the race build
can replace the pure gvisor_sentry sidecar (the sentry runs as a separate
binary, so runsc-race alone only covers the CLI and gofer), and add a
make syscall-test-boot-log target to find the newest runsc boot log.

Demo usage. Each recipe assumes a deliberate fault injected into the
sentry to precipitate the expected failure.

  1. Lockdep cycle (take t.mu then signalHandlers.mu in both orders under
    Getuid):

make syscall-tests TARGETS=//test/syscalls:uidgid_test_runsc_systrap_shared
BAZEL_OPTIONS=--config=lockdep OPTIONS=--nocache_test_results

The sentry panics; make syscall-test-boot-log TEST=uidgid names the log,
which shows the offending chain:

panic: WARNING: circular locking detected: kernel.taskMutex -> kernel.signalHandlersMutex
kernel.(*Task).LockdepDemoCycle pkg/sentry/kernel/threads.go
syscalls/linux.Getuid pkg/sentry/syscalls/linux/sys_identity.go
known lock chain: kernel.signalHandlersMutex -> kernel.taskMutex
kernel.(*TaskSet).newTask pkg/sentry/kernel/task_start.go

  1. Data race in the sentry kernel (race two goroutines during boot):

make syscall-tests TARGETS=//test/syscalls:uname_test_runsc_systrap_shared
RUNSC_TARGET='--config=race //runsc:runsc-race'
SENTRY_SIDECAR_TARGET='--config=race //runsc/cmd/sentry:gvisor_sentry-race'
OPTIONS='--nocache_test_results --test_env=GORACE=halt_on_error=1'

make syscall-test-boot-log TEST=uname names the log; it holds the race:

WARNING: DATA RACE
Read at 0x... by goroutine N:
kernel.RaceDemo.func1 pkg/sentry/kernel/threads.go
Previous write at 0x... by goroutine M:
kernel.RaceDemo.func1 pkg/sentry/kernel/threads.go

  1. Data race in runsc/boot (same race build, fault in the boot glue rather
    than the kernel package):

WARNING: DATA RACE
Read at 0x... by goroutine N:
runsc/boot.raceDemoBoot.func1 runsc/boot/loader.go
Previous write at 0x... by goroutine M:
runsc/boot.raceDemoBoot.func1 runsc/boot/loader.go

Promote the lockdep go build tag to a bazel config, so any target can be
built with the runtime lock-order checker via --config=lockdep, mirroring
--config=race. The lockdep gotag is added to go_binary and go_test targets
when //tools:lockdep is set.

Also document in the Makefile how to run syscall tests with lockdep and
with the race detector, add a SENTRY_SIDECAR_TARGET knob so the race build
can replace the pure gvisor_sentry sidecar (the sentry runs as a separate
binary, so runsc-race alone only covers the CLI and gofer), and add a
`make syscall-test-boot-log` target to find the newest runsc boot log.

Demo usage. Each recipe assumes a deliberate fault injected into the
sentry to precipitate the expected failure.

1. Lockdep cycle (take t.mu then signalHandlers.mu in both orders under
Getuid):

  make syscall-tests TARGETS=//test/syscalls:uidgid_test_runsc_systrap_shared \
    BAZEL_OPTIONS=--config=lockdep OPTIONS=--nocache_test_results

The sentry panics; `make syscall-test-boot-log TEST=uidgid` names the log,
which shows the offending chain:

  panic: WARNING: circular locking detected: kernel.taskMutex -> kernel.signalHandlersMutex
    kernel.(*Task).LockdepDemoCycle  pkg/sentry/kernel/threads.go
    syscalls/linux.Getuid            pkg/sentry/syscalls/linux/sys_identity.go
    known lock chain: kernel.signalHandlersMutex -> kernel.taskMutex
    kernel.(*TaskSet).newTask        pkg/sentry/kernel/task_start.go

2. Data race in the sentry kernel (race two goroutines during boot):

  make syscall-tests TARGETS=//test/syscalls:uname_test_runsc_systrap_shared \
    RUNSC_TARGET='--config=race //runsc:runsc-race' \
    SENTRY_SIDECAR_TARGET='--config=race //runsc/cmd/sentry:gvisor_sentry-race' \
    OPTIONS='--nocache_test_results --test_env=GORACE=halt_on_error=1'

`make syscall-test-boot-log TEST=uname` names the log; it holds the race:

  WARNING: DATA RACE
  Read at 0x... by goroutine N:
    kernel.RaceDemo.func1            pkg/sentry/kernel/threads.go
  Previous write at 0x... by goroutine M:
    kernel.RaceDemo.func1            pkg/sentry/kernel/threads.go

3. Data race in runsc/boot (same race build, fault in the boot glue rather
than the kernel package):

  WARNING: DATA RACE
  Read at 0x... by goroutine N:
    runsc/boot.raceDemoBoot.func1    runsc/boot/loader.go
  Previous write at 0x... by goroutine M:
    runsc/boot.raceDemoBoot.func1    runsc/boot/loader.go
Comment thread Makefile
RUNSC_TARGET ?= //runsc
RUNTIME_BIN ?= $(RUNTIME_DIR)/runsc
EXTRA_SIDECAR_TARGETS ?= # Extra binaries to install under gvisor-bin/.
SENTRY_SIDECAR_TARGET ?= # Sentry sidecar to install as gvisor-bin/gvisor_sentry.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just about to add per-platform sentries. We should not have this makefile assume that there is only one such binary.

Can you instead have the //:release target swap the runsc target it uses when --config=race?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants