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 <mrunalp@gmail.com>
This commit is contained in:
Mrunal Patel 2018-06-01 12:56:13 -07:00
parent ecd55a4135
commit bd3c4f844a
9 changed files with 54 additions and 22 deletions

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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)

View File

@ -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)

View File

@ -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()

View File

@ -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)

View File

@ -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)

View File

@ -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
}

View File

@ -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)
}