TheArchive/tailscale
1
0
Fork 0
mirror of https://github.com/tailscale/tailscale.git synced 2025-08-14 23:17:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

ssh/tailssh: fix double race condition with non-pty command (#8405)

Browse Source 
There are two race conditions in output handling.

The first race condition is due to a misuse of exec.Cmd.StdoutPipe.
The documentation explicitly forbids concurrent use of StdoutPipe
with exec.Cmd.Wait (see golang/go#60908) because Wait will
close both sides of the pipe once the process ends without
any guarantees that all data has been read from the pipe.
To fix this, we allocate the os.Pipes ourselves and
manage cleanup ourselves when the process has ended.

The second race condition is because sshSession.run waits
upon exec.Cmd to finish and then immediately proceeds to call ss.Exit,
which will close all output streams going to the SSH client.
This may interrupt any asynchronous io.Copy still copying data.
To fix this, we close the write-side of the os.Pipes after
the process has finished (and before calling ss.Exit) and
synchronously wait for the io.Copy routines to finish.

Fixes #7601

Signed-off-by: Joe Tsai <joetsai@digital-static.net>
Co-authored-by: Maisem Ali <maisem@tailscale.com>
This commit is contained in:
Joe Tsai
2023-06-21 19:57:45 -07:00
committed by GitHub
parent d4de60c3ae
commit 61886e031e
3 changed files with 69 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
ssh/tailssh/incubator.go
View File

@@ -476,10 +476,10 @@ func (ss *sshSession) launchProcess() error {
}
go resizeWindow(ptyDup /* arbitrary fd */, winCh)
ss.tty = tty
ss.stdin = pty
ss.stdout = os.NewFile(uintptr(ptyDup), pty.Name())
ss.stderr = nil // not available for pty
ss.wrStdin = pty
ss.rdStdout = os.NewFile(uintptr(ptyDup), pty.Name())
ss.rdStderr = nil // not available for pty
ss.childPipes = []io.Closer{tty}
return nil
}
@@ -658,40 +658,29 @@ func (ss *sshSession) startWithPTY() (ptyFile, tty *os.File, err error) {
// startWithStdPipes starts cmd with os.Pipe for Stdin, Stdout and Stderr.
func (ss *sshSession) startWithStdPipes() (err error) {
var stdin io.WriteCloser
var stdout, stderr io.ReadCloser
var rdStdin, wrStdout, wrStderr io.ReadWriteCloser
defer func() {
if err != nil {
for _, c := range []io.Closer{stdin, stdout, stderr} {
if c != nil {
c.Close()
}
}
closeAll(rdStdin, ss.wrStdin, ss.rdStdout, wrStdout, ss.rdStderr, wrStderr)
}
}()
cmd := ss.cmd
if cmd == nil {
if ss.cmd == nil {
return errors.New("nil cmd")
}
stdin, err = cmd.StdinPipe()
if err != nil {
if rdStdin, ss.wrStdin, err = os.Pipe(); err != nil {
return err
}
stdout, err = cmd.StdoutPipe()
if err != nil {
if ss.rdStdout, wrStdout, err = os.Pipe(); err != nil {
return err
}
stderr, err = cmd.StderrPipe()
if err != nil {
if ss.rdStderr, wrStderr, err = os.Pipe(); err != nil {
return err
}
if err := cmd.Start(); err != nil {
return err
}
ss.stdin = stdin
ss.stdout = stdout
ss.stderr = stderr
return nil
ss.cmd.Stdin = rdStdin
ss.cmd.Stdout = wrStdout
ss.cmd.Stderr = wrStderr
ss.childPipes = []io.Closer{rdStdin, wrStdout, wrStderr}
return ss.cmd.Start()
}
func envForUser(u *userMeta) []string {