From 3f0532595679c388362203bbce1d4b6c4d2e336b Mon Sep 17 00:00:00 2001 From: Nicolas Lacasse Date: Mon, 15 Oct 2018 11:06:20 -0700 Subject: Never send boot process stdio to application stdio. We treat handle the boot process stdio separately from the application stdio (which gets passed via flags), but we were still sending both to same place. As a result, some logs that are written directly to os.Stderr by the boot process were ending up in the application logs. This CL starts sendind boot process stdio to the null device (since we don't have any better options). The boot process is already configured to send all logs (and panics) to the log file, so we won't miss anything important. PiperOrigin-RevId: 217173020 Change-Id: I5ab980da037f34620e7861a3736ba09c18d73794 --- runsc/sandbox/sandbox.go | 46 ++++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 18 deletions(-) (limited to 'runsc/sandbox') diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 6c1b39be7..be68e864f 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -351,6 +351,15 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund nextFD++ } + // The current process' stdio must be passed to the application via the + // --stdio-fds flag. The stdio of the sandbox process itself must not + // be connected to the same FDs, otherwise we risk leaking sandbox + // errors to the application, so we set the sandbox stdio to nil, + // causing them to read/write from the null device. + cmd.Stdin = nil + cmd.Stdout = nil + cmd.Stderr = nil + // If the console control socket file is provided, then create a new // pty master/slave pair and set the TTY on the sandbox process. if consoleSocket != "" { @@ -364,21 +373,13 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund } defer tty.Close() - // Ideally we would set the sandbox stdin to this process' - // stdin, but for some reason Docker does not like that (it - // never calls `runsc start`). Instead we set stdio to the - // console TTY, but note that this is distinct from the - // container stdio, which is passed via the flags below. - cmd.Stdin = tty - cmd.Stdout = tty - cmd.Stderr = tty - // Set the TTY as a controlling TTY on the sandbox process. // Note that the Ctty field must be the FD of the TTY in the - // *new* process, not this process. Since we set the TTY to + // *new* process, not this process. Since we are about to + // assign the TTY to nextFD, we can use that value here. // stdin, we can use FD 0 here. cmd.SysProcAttr.Setctty = true - cmd.SysProcAttr.Ctty = 0 + cmd.SysProcAttr.Ctty = nextFD // Pass the tty as all stdio fds to sandbox. for i := 0; i < 3; i++ { @@ -386,14 +387,15 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund cmd.Args = append(cmd.Args, "--stdio-fds="+strconv.Itoa(nextFD)) nextFD++ } - } else { - // Connect the sandbox process to this process's stdios. Note - // that this is distinct from the container's stdio, which is - // passed by the flags below. - cmd.Stdin = os.Stdin - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr + if conf.Debug { + // If debugging, send the boot process stdio to the + // TTY, so that it is easier to find. + cmd.Stdin = tty + cmd.Stdout = tty + cmd.Stderr = tty + } + } else { // If not using a console, pass our current stdio as the // container stdio via flags. for _, f := range []*os.File{os.Stdin, os.Stdout, os.Stderr} { @@ -401,6 +403,14 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund cmd.Args = append(cmd.Args, "--stdio-fds="+strconv.Itoa(nextFD)) nextFD++ } + + if conf.Debug { + // If debugging, send the boot process stdio to the + // this process' stdio, so that is is easier to find. + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + } } // Detach from this session, otherwise cmd will get SIGHUP and SIGCONT -- cgit v1.2.3