Data Race with cmd output buffers

[sipsma]

There appears to be a race condition involving the buffers used by the cmdOutput function. I have a fix for this that I'll submit a PR for, just creating this issue to reference there and provide more context.

The cmd output buffer used gets returned to the sync.Pool at the end of cmdOutput, but the value of .Bytes() is also returned. Bytes provides a slice that is "valid for use only until the next buffer modification", so because the buffer was returned to the pool, it can get used by a different caller of cmdOutput and result in simultaneous reads+writes of the same slice.

This was found in the firecracker-containerd repo when running one of our processes built with the -race flag, though we've only seen the race reported rather than any actual issues as a result of it yet. The process is a wrapper around containerd's v2 runc shim, which is where it picks up the transitive dependency on go-runc.

Here's example output of the race detector from that process:

    WARNING: DATA RACE
    Write at 0x00c000138d28 by goroutine 99:
      syscall.Read()
          /usr/local/go/src/internal/race/race.go:49 +0x9a
      internal/poll.(*FD).Read()
          /usr/local/go/src/internal/poll/fd_unix.go:165 +0x1c7
      os.(*File).Read()
          /usr/local/go/src/os/file_unix.go:259 +0xa6
      bytes.(*Buffer).ReadFrom()
          /usr/local/go/src/bytes/buffer.go:204 +0x158
      io.copyBuffer()
          /usr/local/go/src/io/io.go:388 +0x3fa
      os/exec.(*Cmd).writerDescriptor.func1()
          /usr/local/go/src/io/io.go:364 +0x7a
      os/exec.(*Cmd).Start.func1()
          /usr/local/go/src/os/exec/exec.go:435 +0x34
    Previous read at 0x00c000138d28 by goroutine 61:
      encoding/json.checkValid()
          /usr/local/go/src/encoding/json/scanner.go:27 +0x11b
      encoding/json.Unmarshal()
          /usr/local/go/src/encoding/json/decode.go:100 +0xbf
      github.com/containerd/go-runc.(*Runc).State()
          /go/pkg/mod/github.com/containerd/go-runc@v0.0.0-20190226155025-7d11b49dc076/runc.go:92 +0x2c9
      github.com/containerd/containerd/pkg/process.(*Init).Status()
          /go/pkg/mod/github.com/containerd/containerd@v1.3.1/pkg/process/init.go:243 +0x129
      github.com/containerd/containerd/pkg/process.(*execProcess).Status()
          /go/pkg/mod/github.com/containerd/containerd@v1.3.1/pkg/process/exec.go:252 +0x9e
      github.com/containerd/containerd/runtime/v2/runc/v2.(*service).State()
          /go/pkg/mod/github.com/containerd/containerd@v1.3.1/runtime/v2/runc/v2/service.go:419 +0x12d
      main.(*TaskService).State()
          /src/agent/service.go:258 +0x39b
      github.com/containerd/containerd/runtime/v2/task.RegisterTaskService.func1()
          /go/pkg/mod/github.com/containerd/containerd@v1.3.1/runtime/v2/task/shim.pb.go:3193 +0x142
      github.com/containerd/ttrpc.defaultServerInterceptor()
          /go/pkg/mod/github.com/containerd/ttrpc@v0.0.0-20190613183316-1fb3814edf44/interceptor.go:45 +0x58
      github.com/containerd/ttrpc.(*serviceSet).dispatch()
          /go/pkg/mod/github.com/containerd/ttrpc@v0.0.0-20190613183316-1fb3814edf44/services.go:93 +0x2a7
      github.com/containerd/ttrpc.(*serviceSet).call()
          /go/pkg/mod/github.com/containerd/ttrpc@v0.0.0-20190613183316-1fb3814edf44/services.go:62 +0xc5
      github.com/containerd/ttrpc.(*serverConn).run.func2()
          /go/pkg/mod/github.com/containerd/ttrpc@v0.0.0-20190613183316-1fb3814edf44/server.go:423 +0x1ef
    Goroutine 99 (running) created at:
      os/exec.(*Cmd).Start()
          /usr/local/go/src/os/exec/exec.go:434 +0xa8d
      github.com/containerd/containerd/sys/reaper.(*Monitor).Start()
          /go/pkg/mod/github.com/containerd/containerd@v1.3.1/sys/reaper/reaper_unix.go:94 +0x54
      github.com/containerd/go-runc.cmdOutput()
          /go/pkg/mod/github.com/containerd/go-runc@v0.0.0-20190226155025-7d11b49dc076/runc.go:696 +0x148
      github.com/containerd/go-runc.(*Runc).State()
          /go/pkg/mod/github.com/containerd/go-runc@v0.0.0-20190226155025-7d11b49dc076/runc.go:87 +0xda
      github.com/containerd/containerd/pkg/process.(*Init).Status()
          /go/pkg/mod/github.com/containerd/containerd@v1.3.1/pkg/process/init.go:243 +0x129
      github.com/containerd/containerd/pkg/process.(*execProcess).Status()
          /go/pkg/mod/github.com/containerd/containerd@v1.3.1/pkg/process/exec.go:252 +0x9e
      github.com/containerd/containerd/runtime/v2/runc/v2.(*service).State()
          /go/pkg/mod/github.com/containerd/containerd@v1.3.1/runtime/v2/runc/v2/service.go:419 +0x12d
      main.(*TaskService).State()
          /src/agent/service.go:258 +0x39b
      github.com/containerd/containerd/runtime/v2/task.RegisterTaskService.func1()
          /go/pkg/mod/github.com/containerd/containerd@v1.3.1/runtime/v2/task/shim.pb.go:3193 +0x142
      github.com/containerd/ttrpc.defaultServerInterceptor()
          /go/pkg/mod/github.com/containerd/ttrpc@v0.0.0-20190613183316-1fb3814edf44/interceptor.go:45 +0x58
      github.com/containerd/ttrpc.(*serviceSet).dispatch()
          /go/pkg/mod/github.com/containerd/ttrpc@v0.0.0-20190613183316-1fb3814edf44/services.go:93 +0x2a7
      github.com/containerd/ttrpc.(*serviceSet).call()
          /go/pkg/mod/github.com/containerd/ttrpc@v0.0.0-20190613183316-1fb3814edf44/services.go:62 +0xc5
      github.com/containerd/ttrpc.(*serverConn).run.func2()
          /go/pkg/mod/github.com/containerd/ttrpc@v0.0.0-20190613183316-1fb3814edf44/server.go:423 +0x1ef
    Goroutine 61 (finished) created at:
      github.com/containerd/ttrpc.(*serverConn).run()

          /go/pkg/mod/github.com/containerd/ttrpc@v0.0.0-20190613183316-1fb3814edf44/server.go:419 +0xa7e