From 7679c80be5e4a17aca57a4d7cf5b1b81d4cfad16 Mon Sep 17 00:00:00 2001 From: Alexander Morozov Date: Mon, 8 Aug 2016 10:14:11 -0700 Subject: [PATCH] libcontainer/configs: make hooks run safer It's possible that `cmd.Process` is still nil when we reach timeout. Start creates `Process` field synchronously, and there is no way to such race. Signed-off-by: Alexander Morozov --- libcontainer/configs/config.go | 39 +++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/libcontainer/configs/config.go b/libcontainer/configs/config.go index 3c38191b..a56d12bd 100644 --- a/libcontainer/configs/config.go +++ b/libcontainer/configs/config.go @@ -300,29 +300,38 @@ func (c Command) Run(s HookState) error { if err != nil { return err } + var stdout, stderr bytes.Buffer cmd := exec.Cmd{ - Path: c.Path, - Args: c.Args, - Env: c.Env, - Stdin: bytes.NewReader(b), + Path: c.Path, + Args: c.Args, + Env: c.Env, + Stdin: bytes.NewReader(b), + Stdout: &stdout, + Stderr: &stderr, + } + if err := cmd.Start(); err != nil { + return err } errC := make(chan error, 1) go func() { - out, err := cmd.CombinedOutput() + err := cmd.Wait() if err != nil { - err = fmt.Errorf("%s: %s", err, out) + err = fmt.Errorf("error running hook: %v, stdout: %s, stderr: %s", err, stdout.String(), stderr.String()) } errC <- err }() + var timerCh <-chan time.Time if c.Timeout != nil { - select { - case err := <-errC: - return err - case <-time.After(*c.Timeout): - cmd.Process.Kill() - cmd.Wait() - return fmt.Errorf("hook ran past specified timeout of %.1fs", c.Timeout.Seconds()) - } + timer := time.NewTimer(*c.Timeout) + defer timer.Stop() + timerCh = timer.C + } + select { + case err := <-errC: + return err + case <-timerCh: + cmd.Process.Kill() + cmd.Wait() + return fmt.Errorf("hook ran past specified timeout of %.1fs", c.Timeout.Seconds()) } - return <-errC }