netbird/gvisor
0
0
Fork 0
mirror of https://github.com/netbirdio/gvisor.git synced 2026-05-22 17:12:49 -07:00
Code Issues Packages Projects Releases Wiki Activity

runsc: Fix the data race

Browse Source 
==================
WARNING: DATA RACE
Write at 0x00c000115be0 by main goroutine:
  gvisor.dev/gvisor/runsc/sandbox.(*Sandbox).waitForStopped()
      runsc/sandbox/sandbox.go:1275 +0x24b
  gvisor.dev/gvisor/runsc/sandbox.(*Sandbox).Wait()
      runsc/sandbox/sandbox.go:823 +0xa2c
  gvisor.dev/gvisor/runsc/container.(*Container).Wait()
      runsc/container/container.go:524 +0x131
  gvisor.dev/gvisor/runsc/cmd.startContainerAndWait()
      runsc/cmd/do.go:398 +0xa18
  gvisor.dev/gvisor/runsc/cmd.(*Do).Execute()
      runsc/cmd/do.go:156 +0xc16
  github.com/google/subcommands.(*Commander).Execute()
      external/com_github_google_subcommands/subcommands.go:200 +0x66b
  github.com/google/subcommands.Execute()
      external/com_github_google_subcommands/subcommands.go:481 +0x2096
  gvisor.dev/gvisor/runsc/cli.Main()
      runsc/cli/main.go:243 +0x1fb4
  main.main()
      runsc/main.go:23 +0x56

Previous read at 0x00c000115be0 by goroutine 9:
  gvisor.dev/gvisor/runsc/sandbox.(*Sandbox).connError()
      runsc/sandbox/sandbox.go:387 +0x1e9
  gvisor.dev/gvisor/runsc/sandbox.(*Sandbox).sandboxConnect()
      runsc/sandbox/sandbox.go:381 +0x291
  gvisor.dev/gvisor/runsc/sandbox.(*Sandbox).SignalProcess()
      runsc/sandbox/sandbox.go:932 +0x135
  gvisor.dev/gvisor/runsc/container.(*Container).ForwardSignals.func1()
      runsc/container/container.go:591 +0x267
  gvisor.dev/gvisor/pkg/sentry/sighandling.handleSignals()
      pkg/sentry/sighandling/sighandling.go:63 +0x4f8

Goroutine 9 (running) created at:
  gvisor.dev/gvisor/pkg/sentry/sighandling.StartSignalForwarding()
      pkg/sentry/sighandling/sighandling.go:96 +0x284
  gvisor.dev/gvisor/runsc/container.(*Container).ForwardSignals()
      runsc/container/container.go:589 +0x21d
  gvisor.dev/gvisor/runsc/cmd.startContainerAndWait()
      runsc/cmd/do.go:395 +0x9d0
  gvisor.dev/gvisor/runsc/cmd.(*Do).Execute()
      runsc/cmd/do.go:156 +0xc16
  github.com/google/subcommands.(*Commander).Execute()
      external/com_github_google_subcommands/subcommands.go:200 +0x66b
  github.com/google/subcommands.Execute()
      external/com_github_google_subcommands/subcommands.go:481 +0x2096
  gvisor.dev/gvisor/runsc/cli.Main()
      runsc/cli/main.go:243 +0x1fb4
  main.main()
      runsc/main.go:23 +0x56
==================
Found 1 data race(s)

Signed-off-by: Andrei Vagin <avagin@gmail.com>
This commit is contained in:
Andrei Vagin
2021-12-23 13:03:58 -08:00
parent 715f9b6539
commit 6e7404427c
7 changed files with 73 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
runsc/cmd/capability_test.go
+1 -1
View File
@@ -112,7 +112,7 @@ func TestCapabilities(t *testing.T) {
}
// Check that sandbox and gofer have the proper capabilities.
if err := checkProcessCaps(c.Sandbox.Pid, spec.Process.Capabilities); err != nil {
if err := checkProcessCaps(c.Sandbox.Getpid(), spec.Process.Capabilities); err != nil {
t.Error(err)
}
if err := checkProcessCaps(c.GoferPid, goferCaps); err != nil {
runsc/cmd/debug.go
+5 -4
View File
@@ -139,13 +139,14 @@ func (d *Debug) Execute(_ context.Context, f *flag.FlagSet, args ...interface{})
if !c.IsSandboxRunning() {
return Errorf("container sandbox is not running")
}
log.Infof("Found sandbox %q, PID: %d", c.Sandbox.ID, c.Sandbox.Pid)
log.Infof("Found sandbox %q, PID: %d", c.Sandbox.ID, c.Sandbox.Getpid())
// Perform synchronous actions.
if d.signal > 0 {
log.Infof("Sending signal %d to process: %d", d.signal, c.Sandbox.Pid)
if err := unix.Kill(c.Sandbox.Pid, unix.Signal(d.signal)); err != nil {
return Errorf("failed to send signal %d to processs %d", d.signal, c.Sandbox.Pid)
pid := c.Sandbox.Getpid()
log.Infof("Sending signal %d to process: %d", d.signal, pid)
if err := unix.Kill(pid, unix.Signal(d.signal)); err != nil {
return Errorf("failed to send signal %d to processs %d", d.signal, pid)
}
}
if d.stacks {
runsc/container/container.go
+3 -3
View File
@@ -514,13 +514,13 @@ func (c *Container) Event() (*boot.EventOut, error) {
return event, nil
}
// SandboxPid returns the Pid of the sandbox the container is running in, or -1 if the
// SandboxPid returns the Getpid of the sandbox the container is running in, or -1 if the
// container is not running.
func (c *Container) SandboxPid() int {
if err := c.requireStatus("get PID", Created, Running, Paused); err != nil {
return -1
}
return c.Sandbox.Pid
return c.Sandbox.Getpid()
}
// Wait waits for the container to exit, and returns its WaitStatus.
@@ -1145,7 +1145,7 @@ func adjustSandboxOOMScoreAdj(s *sandbox.Sandbox, spec *specs.Spec, rootDir stri
}
// Set the lowest of all containers oom_score_adj to the sandbox.
return setOOMScoreAdj(s.Pid, lowScore)
return setOOMScoreAdj(s.Getpid(), lowScore)
}
// setOOMScoreAdj sets oom_score_adj to the given value for the given PID.
runsc/container/container_test.go
+1 -1
View File
@@ -1938,7 +1938,7 @@ func doGoferExitTest(t *testing.T, vfs2 bool) {
}
// Kill sandbox and expect gofer to exit on its own.
sandboxProc, err := os.FindProcess(c.Sandbox.Pid)
sandboxProc, err := os.FindProcess(c.Sandbox.Getpid())
if err != nil {
t.Fatalf("error finding sandbox process: %v", err)
}
runsc/container/multi_container_test.go
+1 -1
View File
@@ -694,7 +694,7 @@ func TestMultiContainerSignal(t *testing.T) {
t.Errorf("error waiting for gofer to exit: %v", err)
}
err = blockUntilWaitable(containers[0].Sandbox.Pid)
err = blockUntilWaitable(containers[0].Sandbox.Getpid())
if err != nil && err != unix.ECHILD {
t.Errorf("error waiting for sandbox to exit: %v", err)
}
runsc/sandbox/sandbox.go
+60 -19
View File
@@ -17,6 +17,7 @@ package sandbox
import (
"context"
"encoding/json"
"fmt"
"io"
"math"
@@ -24,6 +25,7 @@ import (
"os/exec"
"strconv"
"strings"
"sync/atomic"
"syscall"
"time"
@@ -50,6 +52,36 @@ import (
"gvisor.dev/gvisor/runsc/specutils"
)
// pid is an atomic type that implements JSON marshal/unmarshal interfaces.
type pid struct {
// +checkatomics
val int64
}
func (p *pid) store(pid int) {
atomic.StoreInt64(&p.val, int64(pid))
}
func (p *pid) load() int {
return int(atomic.LoadInt64(&p.val))
}
// UnmarshalJSON implements json.Unmarshaler.UnmarshalJSON.
func (p *pid) UnmarshalJSON(b []byte) error {
var pid int
if err := json.Unmarshal(b, &pid); err != nil {
return err
}
p.store(pid)
return nil
}
// MarshalJSON implements json.Marshaler.MarshalJSON
func (p *pid) MarshalJSON() ([]byte, error) {
return json.Marshal(p.load())
}
// Sandbox wraps a sandbox process.
//
// It is used to start/stop sandbox process (and associated processes like
@@ -63,9 +95,9 @@ type Sandbox struct {
// ID as the first container run in the sandbox.
ID string `json:"id"`
// Pid is the pid of the running sandbox (immutable). May be 0 if the sandbox
// Pid is the pid of the running sandbox. May be 0 if the sandbox
// is not running.
Pid int `json:"pid"`
Pid pid `json:"pid"`
// UID is the user ID in the parent namespace that the sandbox is running as.
UID int `json:"uid"`
@@ -96,6 +128,11 @@ type Sandbox struct {
status unix.WaitStatus
}
// Getpid returns the process ID of the sandbox process.
func (s *Sandbox) Getpid() int {
return s.Pid.load()
}
// Args is used to configure a new sandbox.
type Args struct {
// ID is the sandbox unique identifier.
@@ -185,7 +222,7 @@ func New(conf *config.Config, args *Args) (*Sandbox, error) {
// CreateSubcontainer creates a container inside the sandbox.
func (s *Sandbox) CreateSubcontainer(conf *config.Config, cid string, tty *os.File) error {
log.Debugf("Create sub-container %q in sandbox %q, PID: %d", cid, s.ID, s.Pid)
log.Debugf("Create sub-container %q in sandbox %q, PID: %d", cid, s.ID, s.Pid.load())
var files []*os.File
if tty != nil {
@@ -213,7 +250,8 @@ func (s *Sandbox) CreateSubcontainer(conf *config.Config, cid string, tty *os.Fi
// StartRoot starts running the root container process inside the sandbox.
func (s *Sandbox) StartRoot(spec *specs.Spec, conf *config.Config) error {
log.Debugf("Start root sandbox %q, PID: %d", s.ID, s.Pid)
pid := s.Pid.load()
log.Debugf("Start root sandbox %q, PID: %d", s.ID, pid)
conn, err := s.sandboxConnect()
if err != nil {
return err
@@ -221,7 +259,7 @@ func (s *Sandbox) StartRoot(spec *specs.Spec, conf *config.Config) error {
defer conn.Close()
// Configure the network.
if err := setupNetwork(conn, s.Pid, conf); err != nil {
if err := setupNetwork(conn, pid, conf); err != nil {
return fmt.Errorf("setting up network: %v", err)
}
@@ -236,7 +274,7 @@ func (s *Sandbox) StartRoot(spec *specs.Spec, conf *config.Config) error {
// StartSubcontainer starts running a sub-container inside the sandbox.
func (s *Sandbox) StartSubcontainer(spec *specs.Spec, conf *config.Config, cid string, stdios, goferFiles []*os.File) error {
log.Debugf("Start sub-container %q in sandbox %q, PID: %d", cid, s.ID, s.Pid)
log.Debugf("Start sub-container %q in sandbox %q, PID: %d", cid, s.ID, s.Pid.load())
if err := s.configureStdios(conf, stdios); err != nil {
return err
@@ -299,7 +337,7 @@ func (s *Sandbox) Restore(cid string, spec *specs.Spec, conf *config.Config, fil
defer conn.Close()
// Configure the network.
if err := setupNetwork(conn, s.Pid, conf); err != nil {
if err := setupNetwork(conn, s.Pid.load(), conf); err != nil {
return fmt.Errorf("setting up network: %v", err)
}
@@ -330,7 +368,7 @@ func (s *Sandbox) Processes(cid string) ([]*control.Process, error) {
// NewCGroup returns the sandbox's Cgroup, or an error if it does not have one.
func (s *Sandbox) NewCGroup() (cgroup.Cgroup, error) {
return cgroup.NewFromPid(s.Pid)
return cgroup.NewFromPid(s.Pid.load())
}
// Execute runs the specified command in the container. It returns the PID of
@@ -385,7 +423,7 @@ func (s *Sandbox) sandboxConnect() (*urpc.Client, error) {
}
func (s *Sandbox) connError(err error) error {
return fmt.Errorf("connecting to control server at PID %d: %v", s.Pid, err)
return fmt.Errorf("connecting to control server at PID %d: %v", s.Pid.load(), err)
}
// createSandboxProcess starts the sandbox as a subprocess by running the "boot"
@@ -853,8 +891,8 @@ func (s *Sandbox) createSandboxProcess(conf *config.Config, args *Args, startSyn
}
s.child = true
s.Pid = cmd.Process.Pid
log.Infof("Sandbox started, PID: %d", s.Pid)
s.Pid.store(cmd.Process.Pid)
log.Infof("Sandbox started, PID: %d", cmd.Process.Pid)
return nil
}
@@ -943,10 +981,11 @@ func (s *Sandbox) IsRootContainer(cid string) bool {
// is idempotent.
func (s *Sandbox) destroy() error {
log.Debugf("Destroy sandbox %q", s.ID)
if s.Pid != 0 {
pid := s.Pid.load()
if pid != 0 {
log.Debugf("Killing sandbox %q", s.ID)
if err := unix.Kill(s.Pid, unix.SIGKILL); err != nil && err != unix.ESRCH {
return fmt.Errorf("killing sandbox %q PID %q: %v", s.ID, s.Pid, err)
if err := unix.Kill(pid, unix.SIGKILL); err != nil && err != unix.ESRCH {
return fmt.Errorf("killing sandbox %q PID %q: %v", s.ID, pid, err)
}
if err := s.waitForStopped(); err != nil {
return fmt.Errorf("waiting sandbox %q stop: %v", s.ID, err)
@@ -1168,9 +1207,10 @@ func (s *Sandbox) Stream(cid string, filters []string, out *os.File) error {
// IsRunning returns true if the sandbox or gofer process is running.
func (s *Sandbox) IsRunning() bool {
if s.Pid != 0 {
pid := s.Pid.load()
if pid != 0 {
// Send a signal 0 to the sandbox process.
if err := unix.Kill(s.Pid, 0); err == nil {
if err := unix.Kill(pid, 0); err == nil {
// Succeeded, process is running.
return true
}
@@ -1324,15 +1364,16 @@ func (s *Sandbox) waitForStopped() error {
if s.child {
s.statusMu.Lock()
defer s.statusMu.Unlock()
if s.Pid == 0 {
pid := s.Pid.load()
if pid == 0 {
return nil
}
// The sandbox process is a child of the current process,
// so we can wait it and collect its zombie.
if _, err := unix.Wait4(int(s.Pid), &s.status, 0, nil); err != nil {
if _, err := unix.Wait4(int(pid), &s.status, 0, nil); err != nil {
return fmt.Errorf("error waiting the sandbox process: %v", err)
}
s.Pid = 0
s.Pid.store(0)
return nil
}
test/root/oom_score_adj_test.go
+2 -2
View File
@@ -102,7 +102,7 @@ func TestOOMScoreAdjSingle(t *testing.T) {
//
// The sandbox should be the same for all containers so just use
// the first one.
sandboxPid := c.Sandbox.Pid
sandboxPid := c.Sandbox.Getpid()
sandboxScore, err := specutils.GetOOMScoreAdj(sandboxPid)
if err != nil {
t.Fatalf("error reading sandbox oom_score_adj: %v", err)
@@ -254,7 +254,7 @@ func TestOOMScoreAdjMulti(t *testing.T) {
//
// The sandbox should be the same for all containers so just use
// the first one.
sandboxPid := containers[0].Sandbox.Pid
sandboxPid := containers[0].Sandbox.Getpid()
if testCase.Expected != nil {
score, err := specutils.GetOOMScoreAdj(sandboxPid)
if err != nil {