summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
-rw-r--r--runsc/boot/controller.go3
-rw-r--r--runsc/boot/loader.go17
-rw-r--r--runsc/cmd/kill.go2
-rw-r--r--runsc/container/container_test.go61
-rw-r--r--runsc/container/multi_container_test.go169
5 files changed, 227 insertions, 25 deletions
diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go
index cb5d8ea31..5e849cb37 100644
--- a/runsc/boot/controller.go
+++ b/runsc/boot/controller.go
@@ -547,7 +547,8 @@ type SignalArgs struct {
// Signo is the signal to send to the process.
Signo int32
- // PID is the process ID in the given container that will be signaled.
+ // PID is the process ID in the given container that will be signaled,
+ // relative to the root PID namespace, not the container's.
// If 0, the root container will be signalled.
PID int32
diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go
index a02eb2ec5..5afce232d 100644
--- a/runsc/boot/loader.go
+++ b/runsc/boot/loader.go
@@ -1171,7 +1171,8 @@ func (f *sandboxNetstackCreator) CreateStack() (inet.Stack, error) {
// signal sends a signal to one or more processes in a container. If PID is 0,
// then the container init process is used. Depending on the SignalDeliveryMode
// option, the signal may be sent directly to the indicated process, to all
-// processes in the container, or to the foreground process group.
+// processes in the container, or to the foreground process group. pid is
+// relative to the root PID namespace, not the container's.
func (l *Loader) signal(cid string, pid, signo int32, mode SignalDeliveryMode) error {
if pid < 0 {
return fmt.Errorf("PID (%d) must be positive", pid)
@@ -1208,6 +1209,8 @@ func (l *Loader) signal(cid string, pid, signo int32, mode SignalDeliveryMode) e
}
}
+// signalProcess sends signal to process in the given container. tgid is
+// relative to the root PID namespace, not the container's.
func (l *Loader) signalProcess(cid string, tgid kernel.ThreadID, signo int32) error {
execTG, err := l.threadGroupFromID(execID{cid: cid, pid: tgid})
if err == nil {
@@ -1216,18 +1219,14 @@ func (l *Loader) signalProcess(cid string, tgid kernel.ThreadID, signo int32) er
}
// The caller may be signaling a process not started directly via exec.
- // In this case, find the process in the container's PID namespace and
- // signal it.
- initTG, err := l.threadGroupFromID(execID{cid: cid})
- if err != nil {
- return fmt.Errorf("no thread group found: %v", err)
- }
- tg := initTG.PIDNamespace().ThreadGroupWithID(tgid)
+ // In this case, find the process and check that the process belongs to the
+ // container in question.
+ tg := l.k.RootPIDNamespace().ThreadGroupWithID(tgid)
if tg == nil {
return fmt.Errorf("no such process with PID %d", tgid)
}
if tg.Leader().ContainerID() != cid {
- return fmt.Errorf("process %d is part of a different container: %q", tgid, tg.Leader().ContainerID())
+ return fmt.Errorf("process %d belongs to a different container: %q", tgid, tg.Leader().ContainerID())
}
return l.k.SendExternalSignalThreadGroup(tg, &arch.SignalInfo{Signo: signo})
}
diff --git a/runsc/cmd/kill.go b/runsc/cmd/kill.go
index aecf0b7ab..e0df39266 100644
--- a/runsc/cmd/kill.go
+++ b/runsc/cmd/kill.go
@@ -52,7 +52,7 @@ func (*Kill) Usage() string {
// SetFlags implements subcommands.Command.SetFlags.
func (k *Kill) SetFlags(f *flag.FlagSet) {
f.BoolVar(&k.all, "all", false, "send the specified signal to all processes inside the container")
- f.IntVar(&k.pid, "pid", 0, "send the specified signal to a specific process")
+ f.IntVar(&k.pid, "pid", 0, "send the specified signal to a specific process. pid is relative to the root PID namespace")
}
// Execute implements subcommands.Command.Execute.
diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go
index 129478505..862d9444d 100644
--- a/runsc/container/container_test.go
+++ b/runsc/container/container_test.go
@@ -112,6 +112,39 @@ func blockUntilWaitable(pid int) error {
return err
}
+// execPS executes `ps` inside the container and return the processes.
+func execPS(c *Container) ([]*control.Process, error) {
+ out, err := executeCombinedOutput(c, "/bin/ps", "-e")
+ if err != nil {
+ return nil, err
+ }
+ lines := strings.Split(string(out), "\n")
+ if len(lines) < 1 {
+ return nil, fmt.Errorf("missing header: %q", lines)
+ }
+ procs := make([]*control.Process, 0, len(lines)-1)
+ for _, line := range lines[1:] {
+ if len(line) == 0 {
+ continue
+ }
+ fields := strings.Fields(line)
+ if len(fields) != 4 {
+ return nil, fmt.Errorf("malformed line: %s", line)
+ }
+ pid, err := strconv.Atoi(fields[0])
+ if err != nil {
+ return nil, err
+ }
+ cmd := fields[3]
+ // Fill only the fields we need thus far.
+ procs = append(procs, &control.Process{
+ PID: kernel.ThreadID(pid),
+ Cmd: cmd,
+ })
+ }
+ return procs, nil
+}
+
// procListsEqual is used to check whether 2 Process lists are equal. Fields
// set to -1 in wants are ignored. Timestamp and threads fields are always
// ignored.
@@ -1568,9 +1601,9 @@ func TestReadonlyRoot(t *testing.T) {
}
// Read mounts to check that root is readonly.
- out, ws, err := executeCombinedOutput(c, "/bin/sh", "-c", "mount | grep ' / '")
- if err != nil || ws != 0 {
- t.Fatalf("exec failed, ws: %v, err: %v", ws, err)
+ out, err := executeCombinedOutput(c, "/bin/sh", "-c", "mount | grep ' / '")
+ if err != nil {
+ t.Fatalf("exec failed: %v", err)
}
t.Logf("root mount: %q", out)
if !strings.Contains(string(out), "(ro)") {
@@ -1578,7 +1611,7 @@ func TestReadonlyRoot(t *testing.T) {
}
// Check that file cannot be created.
- ws, err = execute(c, "/bin/touch", "/foo")
+ ws, err := execute(c, "/bin/touch", "/foo")
if err != nil {
t.Fatalf("touch file in ro mount: %v", err)
}
@@ -1627,9 +1660,9 @@ func TestReadonlyMount(t *testing.T) {
// Read mounts to check that volume is readonly.
cmd := fmt.Sprintf("mount | grep ' %s '", dir)
- out, ws, err := executeCombinedOutput(c, "/bin/sh", "-c", cmd)
- if err != nil || ws != 0 {
- t.Fatalf("exec failed, ws: %v, err: %v", ws, err)
+ out, err := executeCombinedOutput(c, "/bin/sh", "-c", cmd)
+ if err != nil {
+ t.Fatalf("exec failed, err: %v", err)
}
t.Logf("mount: %q", out)
if !strings.Contains(string(out), "(ro)") {
@@ -1637,7 +1670,7 @@ func TestReadonlyMount(t *testing.T) {
}
// Check that file cannot be created.
- ws, err = execute(c, "/bin/touch", path.Join(dir, "file"))
+ ws, err := execute(c, "/bin/touch", path.Join(dir, "file"))
if err != nil {
t.Fatalf("touch file in ro mount: %v", err)
}
@@ -2424,10 +2457,10 @@ func execute(cont *Container, name string, arg ...string) (syscall.WaitStatus, e
return cont.executeSync(args)
}
-func executeCombinedOutput(cont *Container, name string, arg ...string) ([]byte, syscall.WaitStatus, error) {
+func executeCombinedOutput(cont *Container, name string, arg ...string) ([]byte, error) {
r, w, err := os.Pipe()
if err != nil {
- return nil, 0, err
+ return nil, err
}
defer r.Close()
@@ -2439,10 +2472,14 @@ func executeCombinedOutput(cont *Container, name string, arg ...string) ([]byte,
ws, err := cont.executeSync(args)
w.Close()
if err != nil {
- return nil, 0, err
+ return nil, err
+ }
+ if ws != 0 {
+ return nil, fmt.Errorf("exec failed, status: %v", ws)
}
+
out, err := ioutil.ReadAll(r)
- return out, ws, err
+ return out, err
}
// executeSync synchronously executes a new process.
diff --git a/runsc/container/multi_container_test.go b/runsc/container/multi_container_test.go
index 173332cc2..17aef2121 100644
--- a/runsc/container/multi_container_test.go
+++ b/runsc/container/multi_container_test.go
@@ -166,8 +166,8 @@ func TestMultiContainerSanity(t *testing.T) {
}
}
-// TestMultiPIDNS checks that it is possible to run 2 dead-simple
-// containers in the same sandbox with different pidns.
+// TestMultiPIDNS checks that it is possible to run 2 dead-simple containers in
+// the same sandbox with different pidns.
func TestMultiPIDNS(t *testing.T) {
for name, conf := range configs(t, all...) {
t.Run(name, func(t *testing.T) {
@@ -208,6 +208,33 @@ func TestMultiPIDNS(t *testing.T) {
if err := waitForProcessList(containers[1], expectedPL); err != nil {
t.Errorf("failed to wait for sleep to start: %v", err)
}
+
+ // Root container runs in the root PID namespace and can see all
+ // processes.
+ expectedPL = []*control.Process{
+ newProcessBuilder().PID(1).Cmd("sleep").Process(),
+ newProcessBuilder().PID(2).Cmd("sleep").Process(),
+ newProcessBuilder().Cmd("ps").Process(),
+ }
+ got, err := execPS(containers[0])
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !procListsEqual(got, expectedPL) {
+ t.Errorf("container got process list: %s, want: %s", procListToString(got), procListToString(expectedPL))
+ }
+
+ expectedPL = []*control.Process{
+ newProcessBuilder().PID(1).Cmd("sleep").Process(),
+ newProcessBuilder().Cmd("ps").Process(),
+ }
+ got, err = execPS(containers[1])
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !procListsEqual(got, expectedPL) {
+ t.Errorf("container got process list: %s, want: %s", procListToString(got), procListToString(expectedPL))
+ }
})
}
}
@@ -274,6 +301,144 @@ func TestMultiPIDNSPath(t *testing.T) {
if err := waitForProcessList(containers[1], expectedPL); err != nil {
t.Errorf("failed to wait for sleep to start: %v", err)
}
+
+ // Root container runs in the root PID namespace and can see all
+ // processes.
+ expectedPL = []*control.Process{
+ newProcessBuilder().PID(1).Cmd("sleep").Process(),
+ newProcessBuilder().PID(2).Cmd("sleep").Process(),
+ newProcessBuilder().PID(3).Cmd("sleep").Process(),
+ newProcessBuilder().Cmd("ps").Process(),
+ }
+ got, err := execPS(containers[0])
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !procListsEqual(got, expectedPL) {
+ t.Errorf("container got process list: %s, want: %s", procListToString(got), procListToString(expectedPL))
+ }
+
+ // Container 1 runs in the same PID namespace as the root container.
+ expectedPL = []*control.Process{
+ newProcessBuilder().PID(1).Cmd("sleep").Process(),
+ newProcessBuilder().PID(2).Cmd("sleep").Process(),
+ newProcessBuilder().PID(3).Cmd("sleep").Process(),
+ newProcessBuilder().Cmd("ps").Process(),
+ }
+ got, err = execPS(containers[1])
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !procListsEqual(got, expectedPL) {
+ t.Errorf("container got process list: %s, want: %s", procListToString(got), procListToString(expectedPL))
+ }
+
+ // Container 2 runs on its own namespace.
+ expectedPL = []*control.Process{
+ newProcessBuilder().PID(1).Cmd("sleep").Process(),
+ newProcessBuilder().Cmd("ps").Process(),
+ }
+ got, err = execPS(containers[2])
+ if err != nil {
+ t.Fatal(err)
+ }
+ if !procListsEqual(got, expectedPL) {
+ t.Errorf("container got process list: %s, want: %s", procListToString(got), procListToString(expectedPL))
+ }
+ })
+ }
+}
+
+// TestMultiPIDNSKill kills processes using PID when containers are using
+// different PID namespaces to ensure PID is taken from the root namespace.
+func TestMultiPIDNSKill(t *testing.T) {
+ app, err := testutil.FindFile("test/cmd/test_app/test_app")
+ if err != nil {
+ t.Fatal("error finding test_app:", err)
+ }
+
+ for name, conf := range configs(t, all...) {
+ t.Run(name, func(t *testing.T) {
+ rootDir, cleanup, err := testutil.SetupRootDir()
+ if err != nil {
+ t.Fatalf("error creating root dir: %v", err)
+ }
+ defer cleanup()
+ conf.RootDir = rootDir
+
+ // Setup the containers.
+ cmd := []string{app, "task-tree", "--depth=1", "--width=2", "--pause=true"}
+ const processes = 3
+ testSpecs, ids := createSpecs(cmd, cmd)
+
+ // TODO: Uncomment after https://github.com/google/gvisor/pull/5519.
+ //testSpecs[1].Linux = &specs.Linux{
+ // Namespaces: []specs.LinuxNamespace{
+ // {
+ // Type: "pid",
+ // },
+ // },
+ //}
+
+ containers, cleanup, err := startContainers(conf, testSpecs, ids)
+ if err != nil {
+ t.Fatalf("error starting containers: %v", err)
+ }
+ defer cleanup()
+
+ // Wait until all processes are created.
+ for _, c := range containers {
+ if err := waitForProcessCount(c, processes); err != nil {
+ t.Fatalf("error waitting for processes: %v", err)
+ }
+ }
+
+ for i, c := range containers {
+ // First, kill a process that belongs to the container.
+ procs, err := c.Processes()
+ if err != nil {
+ t.Fatalf("container.Processes(): %v", err)
+ }
+ t.Logf("Container %q procs: %s", c.ID, procListToString(procs))
+ pidToKill := procs[processes-1].PID
+ t.Logf("PID to kill: %d", pidToKill)
+ if err := c.SignalProcess(syscall.SIGKILL, int32(pidToKill)); err != nil {
+ t.Errorf("container.SignalProcess: %v", err)
+ }
+ // Wait for the process to get killed.
+ if err := waitForProcessCount(c, processes-1); err != nil {
+ t.Fatalf("error waitting for processes: %v", err)
+ }
+ procs, err = c.Processes()
+ if err != nil {
+ t.Fatalf("container.Processes(): %v", err)
+ }
+ t.Logf("Container %q procs after kill: %s", c.ID, procListToString(procs))
+ for _, proc := range procs {
+ if proc.PID == pidToKill {
+ t.Errorf("process %d not killed: %+v", pidToKill, proc)
+ }
+ }
+
+ // Next, attempt to kill a process from another container and check that
+ // it fails.
+ other := containers[(i+1)%len(containers)]
+ procs, err = other.Processes()
+ if err != nil {
+ t.Fatalf("container.Processes(): %v", err)
+ }
+ t.Logf("Other container %q procs: %s", other.ID, procListToString(procs))
+
+ pidToKill = procs[len(procs)-1].PID
+ t.Logf("PID that should not be killed: %d", pidToKill)
+ err = c.SignalProcess(syscall.SIGKILL, int32(pidToKill))
+ if err == nil {
+ t.Fatalf("killing another container's process should fail")
+ }
+ if !strings.Contains(err.Error(), "belongs to a different container") {
+ t.Errorf("wrong error message from killing another container's: %v", err)
+ }
+ }
})
}
}