From 7bef8ad8bbe3fed26c02b070b6ba09d484ec514b Mon Sep 17 00:00:00 2001 From: wangfengtu Date: Fri, 1 Jun 2018 12:56:13 -0700 Subject: [PATCH 78/94] runc: Fix race in runc exec reason: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. cherry-picked from upstream https://github.com/opencontainers/runc/pull/1812 conflicts: exec.go libcontainer/container_linux.go utils_linux.go Change-Id: I945a5f663914e652cc117aa33885d687f70a51e4 Signed-off-by: Mrunal Patel Signed-off-by: wangfengtu --- 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 22f2689..9ed90ea 100644 --- a/exec.go +++ b/exec.go @@ -135,6 +135,7 @@ func execProcess(context *cli.Context) (int, error) { consoleSocket: context.String("console-socket"), detach: detach, pidFile: context.String("pid-file"), + init: false, } return r.run(p) } diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 8e0ad12..8100aca 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -197,17 +197,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 @@ -216,17 +212,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 @@ -315,8 +304,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") } @@ -329,7 +318,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, } @@ -409,7 +398,7 @@ func (c *linuxContainer) deleteExecFifo() { os.Remove(fifoName) } -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") @@ -418,7 +407,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 bc5b0a3..b4d55e0 100644 --- a/libcontainer/integration/checkpoint_test.go +++ b/libcontainer/integration/checkpoint_test.go @@ -87,6 +87,7 @@ func TestCheckpoint(t *testing.T) { Env: standardEnvironment, Stdin: stdinR, Stdout: &stdout, + Init: true, } err = container.Run(&pconfig) @@ -182,6 +183,7 @@ func TestCheckpoint(t *testing.T) { 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 f3dd72a..583b04a 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -230,6 +230,7 @@ func TestEnter(t *testing.T) { Env: standardEnvironment, Stdin: stdinR, Stdout: &stdout, + Init: true, } err = container.Run(&pconfig) stdinR.Close() @@ -319,6 +320,7 @@ func TestProcessEnv(t *testing.T) { }, Stdin: nil, Stdout: &stdout, + Init: true, } err = container.Run(&pconfig) ok(t, err) @@ -365,6 +367,7 @@ func TestProcessEmptyCaps(t *testing.T) { Env: standardEnvironment, Stdin: nil, Stdout: &stdout, + Init: true, } err = container.Run(&pconfig) ok(t, err) @@ -416,6 +419,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") @@ -490,6 +494,7 @@ func TestAdditionalGroups(t *testing.T) { Stdin: nil, Stdout: &stdout, AdditionalGroups: []string{"plugdev", "audio"}, + Init: true, } err = container.Run(&pconfig) ok(t, err) @@ -550,6 +555,7 @@ func testFreeze(t *testing.T, systemd bool) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(pconfig) stdinR.Close() @@ -761,6 +767,7 @@ func TestContainerState(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(p) if err != nil { @@ -814,6 +821,7 @@ func TestPassExtraFiles(t *testing.T) { ExtraFiles: []*os.File{pipein1, pipein2}, Stdin: nil, Stdout: &stdout, + Init: true, } err = container.Run(&process) if err != nil { @@ -895,6 +903,7 @@ func TestMountCmds(t *testing.T) { Cwd: "/", Args: []string{"sh", "-c", "env"}, Env: standardEnvironment, + Init: true, } err = container.Run(&pconfig) if err != nil { @@ -944,6 +953,7 @@ func TestSysctl(t *testing.T) { Env: standardEnvironment, Stdin: nil, Stdout: &stdout, + Init: true, } err = container.Run(&pconfig) ok(t, err) @@ -1084,6 +1094,7 @@ func TestOomScoreAdj(t *testing.T) { Env: standardEnvironment, Stdin: nil, Stdout: &stdout, + Init: true, } err = container.Run(&pconfig) ok(t, err) @@ -1189,6 +1200,7 @@ func TestHook(t *testing.T) { Env: standardEnvironment, Stdin: nil, Stdout: &stdout, + Init: true, } err = container.Run(&pconfig) ok(t, err) @@ -1305,6 +1317,7 @@ func TestRootfsPropagationSlaveMount(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(pconfig) @@ -1422,6 +1435,7 @@ func TestRootfsPropagationSharedMount(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(pconfig) @@ -1530,6 +1544,7 @@ func TestInitJoinPID(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR1, + Init: true, } err = container1.Run(init1) stdinR1.Close() @@ -1556,6 +1571,7 @@ func TestInitJoinPID(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR2, + Init: true, } err = container2.Run(init2) stdinR2.Close() @@ -1635,6 +1651,7 @@ func TestInitJoinNetworkAndUser(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR1, + Init: true, } err = container1.Run(init1) stdinR1.Close() @@ -1669,6 +1686,7 @@ func TestInitJoinNetworkAndUser(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR2, + Init: true, } err = container2.Run(init2) stdinR2.Close() @@ -1736,6 +1754,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 f06075e..988b667 100644 --- a/libcontainer/integration/execin_test.go +++ b/libcontainer/integration/execin_test.go @@ -36,6 +36,7 @@ func TestExecIn(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() @@ -103,6 +104,7 @@ func testExecInRlimit(t *testing.T, userns bool) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() @@ -121,6 +123,7 @@ func testExecInRlimit(t *testing.T, userns bool) { // increase process rlimit higher than container rlimit to test per-process limit {Type: syscall.RLIMIT_NOFILE, Hard: 1026, Soft: 1026}, }, + Init: true, } err = container.Run(ps) ok(t, err) @@ -157,6 +160,7 @@ func TestExecInAdditionalGroups(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() @@ -213,6 +217,7 @@ func TestExecInError(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() @@ -265,6 +270,7 @@ func TestExecInTTY(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() @@ -349,6 +355,7 @@ func TestExecInEnvironment(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() @@ -368,6 +375,7 @@ func TestExecInEnvironment(t *testing.T) { Stdin: buffers.Stdin, Stdout: buffers.Stdout, Stderr: buffers.Stderr, + Init: true, } err = container.Run(process2) ok(t, err) @@ -413,6 +421,7 @@ func TestExecinPassExtraFiles(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() @@ -486,6 +495,7 @@ func TestExecInOomScoreAdj(t *testing.T) { Args: []string{"cat"}, Env: standardEnvironment, Stdin: stdinR, + Init: true, } err = container.Run(process) stdinR.Close() @@ -541,6 +551,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 055f887..8e2c7cd 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 74d9413..dc6a4d8 100644 --- a/libcontainer/integration/utils_test.go +++ b/libcontainer/integration/utils_test.go @@ -148,6 +148,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 f1ad081..150510d 100644 --- a/libcontainer/process.go +++ b/libcontainer/process.go @@ -68,6 +68,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 c6a8c02..df98cf9 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -72,7 +72,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, @@ -82,6 +82,7 @@ func newProcess(p specs.Process) (*libcontainer.Process, error) { Label: p.SelinuxLabel, NoNewPrivileges: &p.NoNewPrivileges, AppArmorProfile: p.ApparmorProfile, + Init: init, } if p.Capabilities != nil { lp.Capabilities = &configs.Capabilities{} @@ -212,6 +213,7 @@ func createContainer(context *cli.Context, id string, spec *specs.Spec) (libcont } type runner struct { + init bool enableSubreaper bool shouldDestroy bool detach bool @@ -229,7 +231,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 @@ -373,6 +375,7 @@ func startContainer(context *cli.Context, spec *specs.Spec, create bool) (int, e pidFile: context.String("pid-file"), preserveFDs: context.Int("preserve-fds"), create: create, + init: true, } return r.run(&spec.Process) } -- 2.7.4.3