From bd3c4f844abed063a0d0a8575eb596159f33732c Mon Sep 17 00:00:00 2001 From: Mrunal Patel Date: Fri, 1 Jun 2018 12:56:13 -0700 Subject: [PATCH] Fix race in runc exec There is a race in runc exec when the init process stops just before the check for the container status. It is then wrongly assumed that we are trying to start an init process instead of an exec process. This commit add an Init field to libcontainer Process to distinguish between init and exec processes to prevent this race. Signed-off-by: Mrunal Patel --- exec.go | 1 + libcontainer/container_linux.go | 29 +++++++-------------- libcontainer/integration/checkpoint_test.go | 2 ++ libcontainer/integration/exec_test.go | 19 ++++++++++++++ libcontainer/integration/execin_test.go | 11 ++++++++ libcontainer/integration/seccomp_test.go | 3 +++ libcontainer/integration/utils_test.go | 1 + libcontainer/process.go | 3 +++ utils_linux.go | 7 +++-- 9 files changed, 54 insertions(+), 22 deletions(-) diff --git a/exec.go b/exec.go index 5696ce6e293..ae8e60e4263 100644 --- a/exec.go +++ b/exec.go @@ -140,6 +140,7 @@ func execProcess(context *cli.Context) (int, error) { detach: detach, pidFile: context.String("pid-file"), action: CT_ACT_RUN, + init: false, } return r.run(p) } diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 3160f96990a..6bd1dd1545e 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -224,17 +224,13 @@ func (c *linuxContainer) Set(config configs.Config) error { func (c *linuxContainer) Start(process *Process) error { c.m.Lock() defer c.m.Unlock() - status, err := c.currentStatus() - if err != nil { - return err - } - if status == Stopped { + if process.Init { if err := c.createExecFifo(); err != nil { return err } } - if err := c.start(process, status == Stopped); err != nil { - if status == Stopped { + if err := c.start(process); err != nil { + if process.Init { c.deleteExecFifo() } return err @@ -243,17 +239,10 @@ func (c *linuxContainer) Start(process *Process) error { } func (c *linuxContainer) Run(process *Process) error { - c.m.Lock() - status, err := c.currentStatus() - if err != nil { - c.m.Unlock() - return err - } - c.m.Unlock() if err := c.Start(process); err != nil { return err } - if status == Stopped { + if process.Init { return c.exec() } return nil @@ -334,8 +323,8 @@ type openResult struct { err error } -func (c *linuxContainer) start(process *Process, isInit bool) error { - parent, err := c.newParentProcess(process, isInit) +func (c *linuxContainer) start(process *Process) error { + parent, err := c.newParentProcess(process) if err != nil { return newSystemErrorWithCause(err, "creating new parent process") } @@ -348,7 +337,7 @@ func (c *linuxContainer) start(process *Process, isInit bool) error { } // generate a timestamp indicating when the container was started c.created = time.Now().UTC() - if isInit { + if process.Init { c.state = &createdState{ c: c, } @@ -438,7 +427,7 @@ func (c *linuxContainer) includeExecFifo(cmd *exec.Cmd) error { return nil } -func (c *linuxContainer) newParentProcess(p *Process, doInit bool) (parentProcess, error) { +func (c *linuxContainer) newParentProcess(p *Process) (parentProcess, error) { parentPipe, childPipe, err := utils.NewSockPair("init") if err != nil { return nil, newSystemErrorWithCause(err, "creating new init pipe") @@ -447,7 +436,7 @@ func (c *linuxContainer) newParentProcess(p *Process, doInit bool) (parentProces if err != nil { return nil, newSystemErrorWithCause(err, "creating new command template") } - if !doInit { + if !p.Init { return c.newSetnsProcess(p, cmd, parentPipe, childPipe) } diff --git a/libcontainer/integration/checkpoint_test.go b/libcontainer/integration/checkpoint_test.go index 9645999dd5d..63fa14f0f74 100644 --- a/libcontainer/integration/checkpoint_test.go +++ b/libcontainer/integration/checkpoint_test.go @@ -110,6 +110,7 @@ func testCheckpoint(t *testing.T, userns bool) { Env: standardEnvironment, Stdin: stdinR, Stdout: &stdout, + Init: true, } err = container.Run(&pconfig) @@ -205,6 +206,7 @@ func testCheckpoint(t *testing.T, userns bool) { Cwd: "/", Stdin: restoreStdinR, Stdout: &stdout, + Init: true, } err = container.Restore(restoreProcessConfig, checkpointOpts) diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 0ef425be9fc..932024d2fa2 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -231,6 +231,7 @@ func TestEnter(t *testing.T) { Env: standardEnvironment, Stdin: stdinR, Stdout: &stdout, + Init: true, } err = container.Run(&pconfig) stdinR.Close() @@ -320,6 +321,7 @@ func TestProcessEnv(t *testing.T) { }, Stdin: nil, Stdout: &stdout, + Init: true, } err = container.Run(&pconfig) ok(t, err) @@ -366,6 +368,7 @@ func TestProcessEmptyCaps(t *testing.T) { Env: standardEnvironment, Stdin: nil, Stdout: &stdout, + Init: true, } err = container.Run(&pconfig) ok(t, err) @@ -417,6 +420,7 @@ func TestProcessCaps(t *testing.T) { Stdin: nil, Stdout: &stdout, Capabilities: &configs.Capabilities{}, + Init: true, } pconfig.Capabilities.Bounding = append(config.Capabilities.Bounding, "CAP_NET_ADMIN") pconfig.Capabilities.Permitted = append(config.Capabilities.Permitted, "CAP_NET_ADMIN") @@ -491,6 +495,7 @@ func TestAdditionalGroups(t *testing.T) { Stdin: nil, Stdout: &stdout, AdditionalGroups: []string{"plugdev", "audio"}, + Init: true, } err = container.Run(&pconfig) ok(t, err) @@ -551,6 +556,7 @@ func testFreeze(t *testing.T, systemd bool) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(pconfig) stdinR.Close() @@ -762,6 +768,7 @@ func TestContainerState(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(p) if err != nil { @@ -821,6 +828,7 @@ func TestPassExtraFiles(t *testing.T) { ExtraFiles: []*os.File{pipein1, pipein2}, Stdin: nil, Stdout: &stdout, + Init: true, } err = container.Run(&process) if err != nil { @@ -902,6 +910,7 @@ func TestMountCmds(t *testing.T) { Cwd: "/", Args: []string{"sh", "-c", "env"}, Env: standardEnvironment, + Init: true, } err = container.Run(&pconfig) if err != nil { @@ -951,6 +960,7 @@ func TestSysctl(t *testing.T) { Env: standardEnvironment, Stdin: nil, Stdout: &stdout, + Init: true, } err = container.Run(&pconfig) ok(t, err) @@ -1091,6 +1101,7 @@ func TestOomScoreAdj(t *testing.T) { Env: standardEnvironment, Stdin: nil, Stdout: &stdout, + Init: true, } err = container.Run(&pconfig) ok(t, err) @@ -1196,6 +1207,7 @@ func TestHook(t *testing.T) { Env: standardEnvironment, Stdin: nil, Stdout: &stdout, + Init: true, } err = container.Run(&pconfig) ok(t, err) @@ -1312,6 +1324,7 @@ func TestRootfsPropagationSlaveMount(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(pconfig) @@ -1429,6 +1442,7 @@ func TestRootfsPropagationSharedMount(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(pconfig) @@ -1537,6 +1551,7 @@ func TestInitJoinPID(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR1, + Init: true, } err = container1.Run(init1) stdinR1.Close() @@ -1563,6 +1578,7 @@ func TestInitJoinPID(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR2, + Init: true, } err = container2.Run(init2) stdinR2.Close() @@ -1642,6 +1658,7 @@ func TestInitJoinNetworkAndUser(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR1, + Init: true, } err = container1.Run(init1) stdinR1.Close() @@ -1676,6 +1693,7 @@ func TestInitJoinNetworkAndUser(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR2, + Init: true, } err = container2.Run(init2) stdinR2.Close() @@ -1743,6 +1761,7 @@ func TestTmpfsCopyUp(t *testing.T) { Env: standardEnvironment, Stdin: nil, Stdout: &stdout, + Init: true, } err = container.Run(&pconfig) ok(t, err) diff --git a/libcontainer/integration/execin_test.go b/libcontainer/integration/execin_test.go index 56fa3c75c76..14f8a596406 100644 --- a/libcontainer/integration/execin_test.go +++ b/libcontainer/integration/execin_test.go @@ -38,6 +38,7 @@ func TestExecIn(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() @@ -108,6 +109,7 @@ func testExecInRlimit(t *testing.T, userns bool) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() @@ -126,6 +128,7 @@ func testExecInRlimit(t *testing.T, userns bool) { // increase process rlimit higher than container rlimit to test per-process limit {Type: unix.RLIMIT_NOFILE, Hard: 1026, Soft: 1026}, }, + Init: true, } err = container.Run(ps) ok(t, err) @@ -162,6 +165,7 @@ func TestExecInAdditionalGroups(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() @@ -218,6 +222,7 @@ func TestExecInError(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() @@ -270,6 +275,7 @@ func TestExecInTTY(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() @@ -366,6 +372,7 @@ func TestExecInEnvironment(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() @@ -385,6 +392,7 @@ func TestExecInEnvironment(t *testing.T) { Stdin: buffers.Stdin, Stdout: buffers.Stdout, Stderr: buffers.Stderr, + Init: true, } err = container.Run(process2) ok(t, err) @@ -430,6 +438,7 @@ func TestExecinPassExtraFiles(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() @@ -509,6 +518,7 @@ func TestExecInOomScoreAdj(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() @@ -564,6 +574,7 @@ func TestExecInUserns(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() diff --git a/libcontainer/integration/seccomp_test.go b/libcontainer/integration/seccomp_test.go index 3d80032e459..77f1a8d4655 100644 --- a/libcontainer/integration/seccomp_test.go +++ b/libcontainer/integration/seccomp_test.go @@ -48,6 +48,7 @@ func TestSeccompDenyGetcwd(t *testing.T) { Stdin: buffers.Stdin, Stdout: buffers.Stdout, Stderr: buffers.Stderr, + Init: true, } err = container.Run(pwd) @@ -123,6 +124,7 @@ func TestSeccompPermitWriteConditional(t *testing.T) { Stdin: buffers.Stdin, Stdout: buffers.Stdout, Stderr: buffers.Stderr, + Init: true, } err = container.Run(dmesg) @@ -184,6 +186,7 @@ func TestSeccompDenyWriteConditional(t *testing.T) { Stdin: buffers.Stdin, Stdout: buffers.Stdout, Stderr: buffers.Stderr, + Init: true, } err = container.Run(dmesg) diff --git a/libcontainer/integration/utils_test.go b/libcontainer/integration/utils_test.go index 25500032c93..541d42eb86b 100644 --- a/libcontainer/integration/utils_test.go +++ b/libcontainer/integration/utils_test.go @@ -152,6 +152,7 @@ func runContainer(config *configs.Config, console string, args ...string) (buffe Stdin: buffers.Stdin, Stdout: buffers.Stdout, Stderr: buffers.Stderr, + Init: true, } err = container.Run(process) diff --git a/libcontainer/process.go b/libcontainer/process.go index 86bf7387f8c..9a7c6014121 100644 --- a/libcontainer/process.go +++ b/libcontainer/process.go @@ -72,6 +72,9 @@ type Process struct { // ConsoleSocket provides the masterfd console. ConsoleSocket *os.File + // Init specifies whether the process is the first process in the container. + Init bool + ops processOperations } diff --git a/utils_linux.go b/utils_linux.go index 3944eb4b540..c6a3489737c 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -105,7 +105,7 @@ func getDefaultImagePath(context *cli.Context) string { // newProcess returns a new libcontainer Process with the arguments from the // spec and stdio from the current process. -func newProcess(p specs.Process) (*libcontainer.Process, error) { +func newProcess(p specs.Process, init bool) (*libcontainer.Process, error) { lp := &libcontainer.Process{ Args: p.Args, Env: p.Env, @@ -115,6 +115,7 @@ func newProcess(p specs.Process) (*libcontainer.Process, error) { Label: p.SelinuxLabel, NoNewPrivileges: &p.NoNewPrivileges, AppArmorProfile: p.ApparmorProfile, + Init: init, } if p.ConsoleSize != nil { @@ -269,6 +270,7 @@ func createContainer(context *cli.Context, id string, spec *specs.Spec) (libcont } type runner struct { + init bool enableSubreaper bool shouldDestroy bool detach bool @@ -287,7 +289,7 @@ func (r *runner) run(config *specs.Process) (int, error) { r.destroy() return -1, err } - process, err := newProcess(*config) + process, err := newProcess(*config, r.init) if err != nil { r.destroy() return -1, err @@ -450,6 +452,7 @@ func startContainer(context *cli.Context, spec *specs.Spec, action CtAct, criuOp preserveFDs: context.Int("preserve-fds"), action: action, criuOpts: criuOpts, + init: true, } return r.run(spec.Process) }