Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix race in runc exec #1812

Merged
merged 1 commit into from
Jun 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ func execProcess(context *cli.Context) (int, error) {
detach: detach,
pidFile: context.String("pid-file"),
action: CT_ACT_RUN,
init: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should not need this.

}
return r.run(p)
}
Expand Down
29 changes: 9 additions & 20 deletions libcontainer/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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")
}
Expand All @@ -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,
}
Expand Down Expand Up @@ -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")
Expand All @@ -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)
}

Expand Down
2 changes: 2 additions & 0 deletions libcontainer/integration/checkpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ func testCheckpoint(t *testing.T, userns bool) {
Env: standardEnvironment,
Stdin: stdinR,
Stdout: &stdout,
Init: true,
}

err = container.Run(&pconfig)
Expand Down Expand Up @@ -205,6 +206,7 @@ func testCheckpoint(t *testing.T, userns bool) {
Cwd: "/",
Stdin: restoreStdinR,
Stdout: &stdout,
Init: true,
}

err = container.Restore(restoreProcessConfig, checkpointOpts)
Expand Down
19 changes: 19 additions & 0 deletions libcontainer/integration/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ func TestEnter(t *testing.T) {
Env: standardEnvironment,
Stdin: stdinR,
Stdout: &stdout,
Init: true,
}
err = container.Run(&pconfig)
stdinR.Close()
Expand Down Expand Up @@ -320,6 +321,7 @@ func TestProcessEnv(t *testing.T) {
},
Stdin: nil,
Stdout: &stdout,
Init: true,
}
err = container.Run(&pconfig)
ok(t, err)
Expand Down Expand Up @@ -366,6 +368,7 @@ func TestProcessEmptyCaps(t *testing.T) {
Env: standardEnvironment,
Stdin: nil,
Stdout: &stdout,
Init: true,
}
err = container.Run(&pconfig)
ok(t, err)
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -951,6 +960,7 @@ func TestSysctl(t *testing.T) {
Env: standardEnvironment,
Stdin: nil,
Stdout: &stdout,
Init: true,
}
err = container.Run(&pconfig)
ok(t, err)
Expand Down Expand Up @@ -1091,6 +1101,7 @@ func TestOomScoreAdj(t *testing.T) {
Env: standardEnvironment,
Stdin: nil,
Stdout: &stdout,
Init: true,
}
err = container.Run(&pconfig)
ok(t, err)
Expand Down Expand Up @@ -1196,6 +1207,7 @@ func TestHook(t *testing.T) {
Env: standardEnvironment,
Stdin: nil,
Stdout: &stdout,
Init: true,
}
err = container.Run(&pconfig)
ok(t, err)
Expand Down Expand Up @@ -1312,6 +1324,7 @@ func TestRootfsPropagationSlaveMount(t *testing.T) {
Args: []string{"cat"},
Env: standardEnvironment,
Stdin: stdinR,
Init: true,
}

err = container.Run(pconfig)
Expand Down Expand Up @@ -1429,6 +1442,7 @@ func TestRootfsPropagationSharedMount(t *testing.T) {
Args: []string{"cat"},
Env: standardEnvironment,
Stdin: stdinR,
Init: true,
}

err = container.Run(pconfig)
Expand Down Expand Up @@ -1537,6 +1551,7 @@ func TestInitJoinPID(t *testing.T) {
Args: []string{"cat"},
Env: standardEnvironment,
Stdin: stdinR1,
Init: true,
}
err = container1.Run(init1)
stdinR1.Close()
Expand All @@ -1563,6 +1578,7 @@ func TestInitJoinPID(t *testing.T) {
Args: []string{"cat"},
Env: standardEnvironment,
Stdin: stdinR2,
Init: true,
}
err = container2.Run(init2)
stdinR2.Close()
Expand Down Expand Up @@ -1642,6 +1658,7 @@ func TestInitJoinNetworkAndUser(t *testing.T) {
Args: []string{"cat"},
Env: standardEnvironment,
Stdin: stdinR1,
Init: true,
}
err = container1.Run(init1)
stdinR1.Close()
Expand Down Expand Up @@ -1676,6 +1693,7 @@ func TestInitJoinNetworkAndUser(t *testing.T) {
Args: []string{"cat"},
Env: standardEnvironment,
Stdin: stdinR2,
Init: true,
}
err = container2.Run(init2)
stdinR2.Close()
Expand Down Expand Up @@ -1743,6 +1761,7 @@ func TestTmpfsCopyUp(t *testing.T) {
Env: standardEnvironment,
Stdin: nil,
Stdout: &stdout,
Init: true,
}
err = container.Run(&pconfig)
ok(t, err)
Expand Down
11 changes: 11 additions & 0 deletions libcontainer/integration/execin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ func TestExecIn(t *testing.T) {
Args: []string{"cat"},
Env: standardEnvironment,
Stdin: stdinR,
Init: true,
}
err = container.Run(process)
stdinR.Close()
Expand Down Expand Up @@ -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()
Expand All @@ -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)
Expand Down Expand Up @@ -162,6 +165,7 @@ func TestExecInAdditionalGroups(t *testing.T) {
Args: []string{"cat"},
Env: standardEnvironment,
Stdin: stdinR,
Init: true,
}
err = container.Run(process)
stdinR.Close()
Expand Down Expand Up @@ -218,6 +222,7 @@ func TestExecInError(t *testing.T) {
Args: []string{"cat"},
Env: standardEnvironment,
Stdin: stdinR,
Init: true,
}
err = container.Run(process)
stdinR.Close()
Expand Down Expand Up @@ -270,6 +275,7 @@ func TestExecInTTY(t *testing.T) {
Args: []string{"cat"},
Env: standardEnvironment,
Stdin: stdinR,
Init: true,
}
err = container.Run(process)
stdinR.Close()
Expand Down Expand Up @@ -366,6 +372,7 @@ func TestExecInEnvironment(t *testing.T) {
Args: []string{"cat"},
Env: standardEnvironment,
Stdin: stdinR,
Init: true,
}
err = container.Run(process)
stdinR.Close()
Expand All @@ -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)
Expand Down Expand Up @@ -430,6 +438,7 @@ func TestExecinPassExtraFiles(t *testing.T) {
Args: []string{"cat"},
Env: standardEnvironment,
Stdin: stdinR,
Init: true,
}
err = container.Run(process)
stdinR.Close()
Expand Down Expand Up @@ -509,6 +518,7 @@ func TestExecInOomScoreAdj(t *testing.T) {
Args: []string{"cat"},
Env: standardEnvironment,
Stdin: stdinR,
Init: true,
}
err = container.Run(process)
stdinR.Close()
Expand Down Expand Up @@ -564,6 +574,7 @@ func TestExecInUserns(t *testing.T) {
Args: []string{"cat"},
Env: standardEnvironment,
Stdin: stdinR,
Init: true,
}
err = container.Run(process)
stdinR.Close()
Expand Down
3 changes: 3 additions & 0 deletions libcontainer/integration/seccomp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func TestSeccompDenyGetcwd(t *testing.T) {
Stdin: buffers.Stdin,
Stdout: buffers.Stdout,
Stderr: buffers.Stderr,
Init: true,
}

err = container.Run(pwd)
Expand Down Expand Up @@ -123,6 +124,7 @@ func TestSeccompPermitWriteConditional(t *testing.T) {
Stdin: buffers.Stdin,
Stdout: buffers.Stdout,
Stderr: buffers.Stderr,
Init: true,
}

err = container.Run(dmesg)
Expand Down Expand Up @@ -184,6 +186,7 @@ func TestSeccompDenyWriteConditional(t *testing.T) {
Stdin: buffers.Stdin,
Stdout: buffers.Stdout,
Stderr: buffers.Stderr,
Init: true,
}

err = container.Run(dmesg)
Expand Down
1 change: 1 addition & 0 deletions libcontainer/integration/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions libcontainer/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Loading