Skip to content

Commit bd3c4f8

Browse files
mrunalpmrunalp
committed
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 <[email protected]>
1 parent ecd55a4 commit bd3c4f8

File tree

9 files changed

+54
-22
lines changed

9 files changed

+54
-22
lines changed

‎exec.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ func execProcess(context *cli.Context) (int, error) {
140140
detach: detach,
141141
pidFile: context.String("pid-file"),
142142
action: CT_ACT_RUN,
143+
init: false,
143144
}
144145
return r.run(p)
145146
}

‎libcontainer/container_linux.go‎

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -224,17 +224,13 @@ func (c *linuxContainer) Set(config configs.Config) error {
224224
func (c *linuxContainer) Start(process *Process) error {
225225
c.m.Lock()
226226
defer c.m.Unlock()
227-
status, err := c.currentStatus()
228-
if err != nil {
229-
return err
230-
}
231-
if status == Stopped {
227+
if process.Init {
232228
if err := c.createExecFifo(); err != nil {
233229
return err
234230
}
235231
}
236-
if err := c.start(process, status == Stopped); err != nil {
237-
if status == Stopped {
232+
if err := c.start(process); err != nil {
233+
if process.Init {
238234
c.deleteExecFifo()
239235
}
240236
return err
@@ -243,17 +239,10 @@ func (c *linuxContainer) Start(process *Process) error {
243239
}
244240

245241
func (c *linuxContainer) Run(process *Process) error {
246-
c.m.Lock()
247-
status, err := c.currentStatus()
248-
if err != nil {
249-
c.m.Unlock()
250-
return err
251-
}
252-
c.m.Unlock()
253242
if err := c.Start(process); err != nil {
254243
return err
255244
}
256-
if status == Stopped {
245+
if process.Init {
257246
return c.exec()
258247
}
259248
return nil
@@ -334,8 +323,8 @@ type openResult struct {
334323
err error
335324
}
336325

337-
func (c *linuxContainer) start(process *Process, isInit bool) error {
338-
parent, err := c.newParentProcess(process, isInit)
326+
func (c *linuxContainer) start(process *Process) error {
327+
parent, err := c.newParentProcess(process)
339328
if err != nil {
340329
return newSystemErrorWithCause(err, "creating new parent process")
341330
}
@@ -348,7 +337,7 @@ func (c *linuxContainer) start(process *Process, isInit bool) error {
348337
}
349338
// generate a timestamp indicating when the container was started
350339
c.created = time.Now().UTC()
351-
if isInit {
340+
if process.Init {
352341
c.state = &createdState{
353342
c: c,
354343
}
@@ -438,7 +427,7 @@ func (c *linuxContainer) includeExecFifo(cmd *exec.Cmd) error {
438427
return nil
439428
}
440429

441-
func (c *linuxContainer) newParentProcess(p *Process, doInit bool) (parentProcess, error) {
430+
func (c *linuxContainer) newParentProcess(p *Process) (parentProcess, error) {
442431
parentPipe, childPipe, err := utils.NewSockPair("init")
443432
if err != nil {
444433
return nil, newSystemErrorWithCause(err, "creating new init pipe")
@@ -447,7 +436,7 @@ func (c *linuxContainer) newParentProcess(p *Process, doInit bool) (parentProces
447436
if err != nil {
448437
return nil, newSystemErrorWithCause(err, "creating new command template")
449438
}
450-
if !doInit {
439+
if !p.Init {
451440
return c.newSetnsProcess(p, cmd, parentPipe, childPipe)
452441
}
453442

‎libcontainer/integration/checkpoint_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ func testCheckpoint(t *testing.T, userns bool) {
110110
Env: standardEnvironment,
111111
Stdin: stdinR,
112112
Stdout: &stdout,
113+
Init: true,
113114
}
114115

115116
err = container.Run(&pconfig)
@@ -205,6 +206,7 @@ func testCheckpoint(t *testing.T, userns bool) {
205206
Cwd: "/",
206207
Stdin: restoreStdinR,
207208
Stdout: &stdout,
209+
Init: true,
208210
}
209211

210212
err = container.Restore(restoreProcessConfig, checkpointOpts)

‎libcontainer/integration/exec_test.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ func TestEnter(t *testing.T) {
231231
Env: standardEnvironment,
232232
Stdin: stdinR,
233233
Stdout: &stdout,
234+
Init: true,
234235
}
235236
err = container.Run(&pconfig)
236237
stdinR.Close()
@@ -320,6 +321,7 @@ func TestProcessEnv(t *testing.T) {
320321
},
321322
Stdin: nil,
322323
Stdout: &stdout,
324+
Init: true,
323325
}
324326
err = container.Run(&pconfig)
325327
ok(t, err)
@@ -366,6 +368,7 @@ func TestProcessEmptyCaps(t *testing.T) {
366368
Env: standardEnvironment,
367369
Stdin: nil,
368370
Stdout: &stdout,
371+
Init: true,
369372
}
370373
err = container.Run(&pconfig)
371374
ok(t, err)
@@ -417,6 +420,7 @@ func TestProcessCaps(t *testing.T) {
417420
Stdin: nil,
418421
Stdout: &stdout,
419422
Capabilities: &configs.Capabilities{},
423+
Init: true,
420424
}
421425
pconfig.Capabilities.Bounding = append(config.Capabilities.Bounding, "CAP_NET_ADMIN")
422426
pconfig.Capabilities.Permitted = append(config.Capabilities.Permitted, "CAP_NET_ADMIN")
@@ -491,6 +495,7 @@ func TestAdditionalGroups(t *testing.T) {
491495
Stdin: nil,
492496
Stdout: &stdout,
493497
AdditionalGroups: []string{"plugdev", "audio"},
498+
Init: true,
494499
}
495500
err = container.Run(&pconfig)
496501
ok(t, err)
@@ -551,6 +556,7 @@ func testFreeze(t *testing.T, systemd bool) {
551556
Args: []string{"cat"},
552557
Env: standardEnvironment,
553558
Stdin: stdinR,
559+
Init: true,
554560
}
555561
err = container.Run(pconfig)
556562
stdinR.Close()
@@ -762,6 +768,7 @@ func TestContainerState(t *testing.T) {
762768
Args: []string{"cat"},
763769
Env: standardEnvironment,
764770
Stdin: stdinR,
771+
Init: true,
765772
}
766773
err = container.Run(p)
767774
if err != nil {
@@ -821,6 +828,7 @@ func TestPassExtraFiles(t *testing.T) {
821828
ExtraFiles: []*os.File{pipein1, pipein2},
822829
Stdin: nil,
823830
Stdout: &stdout,
831+
Init: true,
824832
}
825833
err = container.Run(&process)
826834
if err != nil {
@@ -902,6 +910,7 @@ func TestMountCmds(t *testing.T) {
902910
Cwd: "/",
903911
Args: []string{"sh", "-c", "env"},
904912
Env: standardEnvironment,
913+
Init: true,
905914
}
906915
err = container.Run(&pconfig)
907916
if err != nil {
@@ -951,6 +960,7 @@ func TestSysctl(t *testing.T) {
951960
Env: standardEnvironment,
952961
Stdin: nil,
953962
Stdout: &stdout,
963+
Init: true,
954964
}
955965
err = container.Run(&pconfig)
956966
ok(t, err)
@@ -1091,6 +1101,7 @@ func TestOomScoreAdj(t *testing.T) {
10911101
Env: standardEnvironment,
10921102
Stdin: nil,
10931103
Stdout: &stdout,
1104+
Init: true,
10941105
}
10951106
err = container.Run(&pconfig)
10961107
ok(t, err)
@@ -1196,6 +1207,7 @@ func TestHook(t *testing.T) {
11961207
Env: standardEnvironment,
11971208
Stdin: nil,
11981209
Stdout: &stdout,
1210+
Init: true,
11991211
}
12001212
err = container.Run(&pconfig)
12011213
ok(t, err)
@@ -1312,6 +1324,7 @@ func TestRootfsPropagationSlaveMount(t *testing.T) {
13121324
Args: []string{"cat"},
13131325
Env: standardEnvironment,
13141326
Stdin: stdinR,
1327+
Init: true,
13151328
}
13161329

13171330
err = container.Run(pconfig)
@@ -1429,6 +1442,7 @@ func TestRootfsPropagationSharedMount(t *testing.T) {
14291442
Args: []string{"cat"},
14301443
Env: standardEnvironment,
14311444
Stdin: stdinR,
1445+
Init: true,
14321446
}
14331447

14341448
err = container.Run(pconfig)
@@ -1537,6 +1551,7 @@ func TestInitJoinPID(t *testing.T) {
15371551
Args: []string{"cat"},
15381552
Env: standardEnvironment,
15391553
Stdin: stdinR1,
1554+
Init: true,
15401555
}
15411556
err = container1.Run(init1)
15421557
stdinR1.Close()
@@ -1563,6 +1578,7 @@ func TestInitJoinPID(t *testing.T) {
15631578
Args: []string{"cat"},
15641579
Env: standardEnvironment,
15651580
Stdin: stdinR2,
1581+
Init: true,
15661582
}
15671583
err = container2.Run(init2)
15681584
stdinR2.Close()
@@ -1642,6 +1658,7 @@ func TestInitJoinNetworkAndUser(t *testing.T) {
16421658
Args: []string{"cat"},
16431659
Env: standardEnvironment,
16441660
Stdin: stdinR1,
1661+
Init: true,
16451662
}
16461663
err = container1.Run(init1)
16471664
stdinR1.Close()
@@ -1676,6 +1693,7 @@ func TestInitJoinNetworkAndUser(t *testing.T) {
16761693
Args: []string{"cat"},
16771694
Env: standardEnvironment,
16781695
Stdin: stdinR2,
1696+
Init: true,
16791697
}
16801698
err = container2.Run(init2)
16811699
stdinR2.Close()
@@ -1743,6 +1761,7 @@ func TestTmpfsCopyUp(t *testing.T) {
17431761
Env: standardEnvironment,
17441762
Stdin: nil,
17451763
Stdout: &stdout,
1764+
Init: true,
17461765
}
17471766
err = container.Run(&pconfig)
17481767
ok(t, err)

‎libcontainer/integration/execin_test.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ func TestExecIn(t *testing.T) {
3838
Args: []string{"cat"},
3939
Env: standardEnvironment,
4040
Stdin: stdinR,
41+
Init: true,
4142
}
4243
err = container.Run(process)
4344
stdinR.Close()
@@ -108,6 +109,7 @@ func testExecInRlimit(t *testing.T, userns bool) {
108109
Args: []string{"cat"},
109110
Env: standardEnvironment,
110111
Stdin: stdinR,
112+
Init: true,
111113
}
112114
err = container.Run(process)
113115
stdinR.Close()
@@ -126,6 +128,7 @@ func testExecInRlimit(t *testing.T, userns bool) {
126128
// increase process rlimit higher than container rlimit to test per-process limit
127129
{Type: unix.RLIMIT_NOFILE, Hard: 1026, Soft: 1026},
128130
},
131+
Init: true,
129132
}
130133
err = container.Run(ps)
131134
ok(t, err)
@@ -162,6 +165,7 @@ func TestExecInAdditionalGroups(t *testing.T) {
162165
Args: []string{"cat"},
163166
Env: standardEnvironment,
164167
Stdin: stdinR,
168+
Init: true,
165169
}
166170
err = container.Run(process)
167171
stdinR.Close()
@@ -218,6 +222,7 @@ func TestExecInError(t *testing.T) {
218222
Args: []string{"cat"},
219223
Env: standardEnvironment,
220224
Stdin: stdinR,
225+
Init: true,
221226
}
222227
err = container.Run(process)
223228
stdinR.Close()
@@ -270,6 +275,7 @@ func TestExecInTTY(t *testing.T) {
270275
Args: []string{"cat"},
271276
Env: standardEnvironment,
272277
Stdin: stdinR,
278+
Init: true,
273279
}
274280
err = container.Run(process)
275281
stdinR.Close()
@@ -366,6 +372,7 @@ func TestExecInEnvironment(t *testing.T) {
366372
Args: []string{"cat"},
367373
Env: standardEnvironment,
368374
Stdin: stdinR,
375+
Init: true,
369376
}
370377
err = container.Run(process)
371378
stdinR.Close()
@@ -385,6 +392,7 @@ func TestExecInEnvironment(t *testing.T) {
385392
Stdin: buffers.Stdin,
386393
Stdout: buffers.Stdout,
387394
Stderr: buffers.Stderr,
395+
Init: true,
388396
}
389397
err = container.Run(process2)
390398
ok(t, err)
@@ -430,6 +438,7 @@ func TestExecinPassExtraFiles(t *testing.T) {
430438
Args: []string{"cat"},
431439
Env: standardEnvironment,
432440
Stdin: stdinR,
441+
Init: true,
433442
}
434443
err = container.Run(process)
435444
stdinR.Close()
@@ -509,6 +518,7 @@ func TestExecInOomScoreAdj(t *testing.T) {
509518
Args: []string{"cat"},
510519
Env: standardEnvironment,
511520
Stdin: stdinR,
521+
Init: true,
512522
}
513523
err = container.Run(process)
514524
stdinR.Close()
@@ -564,6 +574,7 @@ func TestExecInUserns(t *testing.T) {
564574
Args: []string{"cat"},
565575
Env: standardEnvironment,
566576
Stdin: stdinR,
577+
Init: true,
567578
}
568579
err = container.Run(process)
569580
stdinR.Close()

‎libcontainer/integration/seccomp_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ func TestSeccompDenyGetcwd(t *testing.T) {
4848
Stdin: buffers.Stdin,
4949
Stdout: buffers.Stdout,
5050
Stderr: buffers.Stderr,
51+
Init: true,
5152
}
5253

5354
err = container.Run(pwd)
@@ -123,6 +124,7 @@ func TestSeccompPermitWriteConditional(t *testing.T) {
123124
Stdin: buffers.Stdin,
124125
Stdout: buffers.Stdout,
125126
Stderr: buffers.Stderr,
127+
Init: true,
126128
}
127129

128130
err = container.Run(dmesg)
@@ -184,6 +186,7 @@ func TestSeccompDenyWriteConditional(t *testing.T) {
184186
Stdin: buffers.Stdin,
185187
Stdout: buffers.Stdout,
186188
Stderr: buffers.Stderr,
189+
Init: true,
187190
}
188191

189192
err = container.Run(dmesg)

‎libcontainer/integration/utils_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ func runContainer(config *configs.Config, console string, args ...string) (buffe
152152
Stdin: buffers.Stdin,
153153
Stdout: buffers.Stdout,
154154
Stderr: buffers.Stderr,
155+
Init: true,
155156
}
156157

157158
err = container.Run(process)

‎libcontainer/process.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ type Process struct {
7272
// ConsoleSocket provides the masterfd console.
7373
ConsoleSocket *os.File
7474

75+
// Init specifies whether the process is the first process in the container.
76+
Init bool
77+
7578
ops processOperations
7679
}
7780

0 commit comments

Comments
 (0)