Skip to content

Commit 6b25c1e

Browse files
crosbymichaelcrosbymichael
authored
Merge pull request #2970 from Random-Liu/fix-exec-race-condition
Fix exec race condition
2 parents 18a8a06 + 952d582 commit 6b25c1e

5 files changed

Lines changed: 26 additions & 31 deletions

File tree

‎runtime/v1/linux/proc/deleted_state.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,3 @@ func (s *deletedState) SetExited(status int) {
6969
func (s *deletedState) Exec(ctx context.Context, path string, r *ExecConfig) (proc.Process, error) {
7070
return nil, errors.Errorf("cannot exec in a deleted state")
7171
}
72-
73-
func (s *deletedState) Pid() int {
74-
return -1
75-
}

‎runtime/v1/linux/proc/exec.go‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ type execProcess struct {
4949
io runc.IO
5050
status int
5151
exited time.Time
52-
pid int
52+
pid *safePid
5353
closers []io.Closer
5454
stdin io.Closer
5555
stdio proc.Stdio
@@ -69,11 +69,7 @@ func (e *execProcess) ID() string {
6969
}
7070

7171
func (e *execProcess) Pid() int {
72-
return e.execState.Pid()
73-
}
74-
75-
func (e *execProcess) pidv() int {
76-
return e.pid
72+
return e.pid.get()
7773
}
7874

7975
func (e *execProcess) ExitStatus() int {
@@ -145,7 +141,7 @@ func (e *execProcess) Kill(ctx context.Context, sig uint32, _ bool) error {
145141
}
146142

147143
func (e *execProcess) kill(ctx context.Context, sig uint32, _ bool) error {
148-
pid := e.pid
144+
pid := e.pid.get()
149145
if pid != 0 {
150146
if err := unix.Kill(pid, syscall.Signal(sig)); err != nil {
151147
return errors.Wrapf(checkKillError(err), "exec kill error")
@@ -170,6 +166,12 @@ func (e *execProcess) Start(ctx context.Context) error {
170166
}
171167

172168
func (e *execProcess) start(ctx context.Context) (err error) {
169+
// The reaper may receive exit signal right after
170+
// the container is started, before the e.pid is updated.
171+
// In that case, we want to block the signal handler to
172+
// access e.pid until it is updated.
173+
e.pid.Lock()
174+
defer e.pid.Unlock()
173175
var (
174176
socket *runc.Socket
175177
pidfile = filepath.Join(e.path, fmt.Sprintf("%s.pid", e.id))
@@ -229,7 +231,7 @@ func (e *execProcess) start(ctx context.Context) (err error) {
229231
if err != nil {
230232
return errors.Wrap(err, "failed to retrieve OCI runtime exec pid")
231233
}
232-
e.pid = pid
234+
e.pid.pid = pid
233235
return nil
234236
}
235237

@@ -247,11 +249,11 @@ func (e *execProcess) Status(ctx context.Context) (string, error) {
247249
e.mu.Lock()
248250
defer e.mu.Unlock()
249251
// if we don't have a pid then the exec process has just been created
250-
if e.pid == 0 {
252+
if e.pid.get() == 0 {
251253
return "created", nil
252254
}
253255
// if we have a pid and it can be signaled, the process is running
254-
if err := unix.Kill(e.pid, 0); err == nil {
256+
if err := unix.Kill(e.pid.get(), 0); err == nil {
255257
return "running", nil
256258
}
257259
// else if we have a pid but it can nolonger be signaled, it has stopped

‎runtime/v1/linux/proc/exec_state.go‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ type execState interface {
3131
Delete(context.Context) error
3232
Kill(context.Context, uint32, bool) error
3333
SetExited(int)
34-
Pid() int
3534
}
3635

3736
type execCreatedState struct {
@@ -83,12 +82,6 @@ func (s *execCreatedState) SetExited(status int) {
8382
}
8483
}
8584

86-
func (s *execCreatedState) Pid() int {
87-
s.p.mu.Lock()
88-
defer s.p.mu.Unlock()
89-
return s.p.pidv()
90-
}
91-
9285
type execRunningState struct {
9386
p *execProcess
9487
}
@@ -127,12 +120,6 @@ func (s *execRunningState) SetExited(status int) {
127120
}
128121
}
129122

130-
func (s *execRunningState) Pid() int {
131-
s.p.mu.Lock()
132-
defer s.p.mu.Unlock()
133-
return s.p.pidv()
134-
}
135-
136123
type execStoppedState struct {
137124
p *execProcess
138125
}
@@ -170,7 +157,3 @@ func (s *execStoppedState) Kill(ctx context.Context, sig uint32, all bool) error
170157
func (s *execStoppedState) SetExited(status int) {
171158
// no op
172159
}
173-
174-
func (s *execStoppedState) Pid() int {
175-
return s.p.pidv()
176-
}

‎runtime/v1/linux/proc/init.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,7 @@ func (p *Init) exec(ctx context.Context, path string, r *ExecConfig) (proc.Proce
407407
Terminal: r.Terminal,
408408
},
409409
waitBlock: make(chan struct{}),
410+
pid: &safePid{},
410411
}
411412
e.execState = &execCreatedState{p: e}
412413
return e, nil

‎runtime/v1/linux/proc/utils.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"io"
2424
"os"
2525
"strings"
26+
"sync"
2627
"time"
2728

2829
"github.com/containerd/containerd/errdefs"
@@ -31,6 +32,18 @@ import (
3132
"golang.org/x/sys/unix"
3233
)
3334

35+
// safePid is a thread safe wrapper for pid.
36+
type safePid struct {
37+
sync.Mutex
38+
pid int
39+
}
40+
41+
func (s *safePid) get() int {
42+
s.Lock()
43+
defer s.Unlock()
44+
return s.pid
45+
}
46+
3447
// TODO(mlaventure): move to runc package?
3548
func getLastRuntimeError(r *runc.Runc) (string, error) {
3649
if r.Log == "" {

0 commit comments

Comments
 (0)