Merge pull request #4980 from thaJeztah/prevent_cio_npe

cio: prevent NPE when closing, and fix pipes potentially not being closed on Windows
This commit is contained in:
Phil Estes 2021-02-04 14:24:10 -05:00 committed by GitHub
commit aa5e55ad98
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 23 additions and 37 deletions

View File

@ -80,7 +80,7 @@ type FIFOSet struct {
// Close the FIFOSet // Close the FIFOSet
func (f *FIFOSet) Close() error { func (f *FIFOSet) Close() error {
if f.close != nil { if f != nil && f.close != nil {
return f.close() return f.close()
} }
return nil return nil

View File

@ -103,38 +103,36 @@ func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) {
}, nil }, nil
} }
func openFifos(ctx context.Context, fifos *FIFOSet) (pipes, error) { func openFifos(ctx context.Context, fifos *FIFOSet) (f pipes, retErr error) {
var err error
defer func() { defer func() {
if err != nil { if retErr != nil {
fifos.Close() fifos.Close()
} }
}() }()
var f pipes
if fifos.Stdin != "" { if fifos.Stdin != "" {
if f.Stdin, err = fifo.OpenFifo(ctx, fifos.Stdin, syscall.O_WRONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); err != nil { if f.Stdin, retErr = fifo.OpenFifo(ctx, fifos.Stdin, syscall.O_WRONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); retErr != nil {
return f, errors.Wrapf(err, "failed to open stdin fifo") return f, errors.Wrapf(retErr, "failed to open stdin fifo")
} }
defer func() { defer func() {
if err != nil && f.Stdin != nil { if retErr != nil && f.Stdin != nil {
f.Stdin.Close() f.Stdin.Close()
} }
}() }()
} }
if fifos.Stdout != "" { if fifos.Stdout != "" {
if f.Stdout, err = fifo.OpenFifo(ctx, fifos.Stdout, syscall.O_RDONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); err != nil { if f.Stdout, retErr = fifo.OpenFifo(ctx, fifos.Stdout, syscall.O_RDONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); retErr != nil {
return f, errors.Wrapf(err, "failed to open stdout fifo") return f, errors.Wrapf(retErr, "failed to open stdout fifo")
} }
defer func() { defer func() {
if err != nil && f.Stdout != nil { if retErr != nil && f.Stdout != nil {
f.Stdout.Close() f.Stdout.Close()
} }
}() }()
} }
if !fifos.Terminal && fifos.Stderr != "" { if !fifos.Terminal && fifos.Stderr != "" {
if f.Stderr, err = fifo.OpenFifo(ctx, fifos.Stderr, syscall.O_RDONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); err != nil { if f.Stderr, retErr = fifo.OpenFifo(ctx, fifos.Stderr, syscall.O_RDONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); retErr != nil {
return f, errors.Wrapf(err, "failed to open stderr fifo") return f, errors.Wrapf(retErr, "failed to open stderr fifo")
} }
} }
return f, nil return f, nil

View File

@ -20,7 +20,6 @@ import (
"context" "context"
"fmt" "fmt"
"io" "io"
"net"
winio "github.com/Microsoft/go-winio" winio "github.com/Microsoft/go-winio"
"github.com/containerd/containerd/log" "github.com/containerd/containerd/log"
@ -43,22 +42,21 @@ func NewFIFOSetInDir(_, id string, terminal bool) (*FIFOSet, error) {
}, nil), nil }, nil), nil
} }
func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) { func copyIO(fifos *FIFOSet, ioset *Streams) (_ *cio, retErr error) {
var ( cios := &cio{config: fifos.Config}
set []io.Closer
) defer func() {
if retErr != nil {
_ = cios.Close()
}
}()
if fifos.Stdin != "" { if fifos.Stdin != "" {
l, err := winio.ListenPipe(fifos.Stdin, nil) l, err := winio.ListenPipe(fifos.Stdin, nil)
if err != nil { if err != nil {
return nil, errors.Wrapf(err, "failed to create stdin pipe %s", fifos.Stdin) return nil, errors.Wrapf(err, "failed to create stdin pipe %s", fifos.Stdin)
} }
defer func(l net.Listener) { cios.closers = append(cios.closers, l)
if err != nil {
l.Close()
}
}(l)
set = append(set, l)
go func() { go func() {
c, err := l.Accept() c, err := l.Accept()
@ -81,12 +79,7 @@ func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) {
if err != nil { if err != nil {
return nil, errors.Wrapf(err, "failed to create stdout pipe %s", fifos.Stdout) return nil, errors.Wrapf(err, "failed to create stdout pipe %s", fifos.Stdout)
} }
defer func(l net.Listener) { cios.closers = append(cios.closers, l)
if err != nil {
l.Close()
}
}(l)
set = append(set, l)
go func() { go func() {
c, err := l.Accept() c, err := l.Accept()
@ -109,12 +102,7 @@ func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) {
if err != nil { if err != nil {
return nil, errors.Wrapf(err, "failed to create stderr pipe %s", fifos.Stderr) return nil, errors.Wrapf(err, "failed to create stderr pipe %s", fifos.Stderr)
} }
defer func(l net.Listener) { cios.closers = append(cios.closers, l)
if err != nil {
l.Close()
}
}(l)
set = append(set, l)
go func() { go func() {
c, err := l.Accept() c, err := l.Accept()
@ -132,7 +120,7 @@ func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) {
}() }()
} }
return &cio{config: fifos.Config, closers: set}, nil return cios, nil
} }
// NewDirectIO returns an IO implementation that exposes the IO streams as io.ReadCloser // NewDirectIO returns an IO implementation that exposes the IO streams as io.ReadCloser