Skip to content

Commit aa5e55a

Browse files
estespestesp
authored
Merge pull request #4980 from thaJeztah/prevent_cio_npe
cio: prevent NPE when closing, and fix pipes potentially not being closed on Windows
2 parents e178af2 + 7a468a3 commit aa5e55a

3 files changed

Lines changed: 23 additions & 37 deletions

File tree

‎cio/io.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ type FIFOSet struct {
8080

8181
// Close the FIFOSet
8282
func (f *FIFOSet) Close() error {
83-
if f.close != nil {
83+
if f != nil && f.close != nil {
8484
return f.close()
8585
}
8686
return nil

‎cio/io_unix.go‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -103,38 +103,36 @@ func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) {
103103
}, nil
104104
}
105105

106-
func openFifos(ctx context.Context, fifos *FIFOSet) (pipes, error) {
107-
var err error
106+
func openFifos(ctx context.Context, fifos *FIFOSet) (f pipes, retErr error) {
108107
defer func() {
109-
if err != nil {
108+
if retErr != nil {
110109
fifos.Close()
111110
}
112111
}()
113112

114-
var f pipes
115113
if fifos.Stdin != "" {
116-
if f.Stdin, err = fifo.OpenFifo(ctx, fifos.Stdin, syscall.O_WRONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); err != nil {
117-
return f, errors.Wrapf(err, "failed to open stdin fifo")
114+
if f.Stdin, retErr = fifo.OpenFifo(ctx, fifos.Stdin, syscall.O_WRONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); retErr != nil {
115+
return f, errors.Wrapf(retErr, "failed to open stdin fifo")
118116
}
119117
defer func() {
120-
if err != nil && f.Stdin != nil {
118+
if retErr != nil && f.Stdin != nil {
121119
f.Stdin.Close()
122120
}
123121
}()
124122
}
125123
if fifos.Stdout != "" {
126-
if f.Stdout, err = fifo.OpenFifo(ctx, fifos.Stdout, syscall.O_RDONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); err != nil {
127-
return f, errors.Wrapf(err, "failed to open stdout fifo")
124+
if f.Stdout, retErr = fifo.OpenFifo(ctx, fifos.Stdout, syscall.O_RDONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); retErr != nil {
125+
return f, errors.Wrapf(retErr, "failed to open stdout fifo")
128126
}
129127
defer func() {
130-
if err != nil && f.Stdout != nil {
128+
if retErr != nil && f.Stdout != nil {
131129
f.Stdout.Close()
132130
}
133131
}()
134132
}
135133
if !fifos.Terminal && fifos.Stderr != "" {
136-
if f.Stderr, err = fifo.OpenFifo(ctx, fifos.Stderr, syscall.O_RDONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); err != nil {
137-
return f, errors.Wrapf(err, "failed to open stderr fifo")
134+
if f.Stderr, retErr = fifo.OpenFifo(ctx, fifos.Stderr, syscall.O_RDONLY|syscall.O_CREAT|syscall.O_NONBLOCK, 0700); retErr != nil {
135+
return f, errors.Wrapf(retErr, "failed to open stderr fifo")
138136
}
139137
}
140138
return f, nil

‎cio/io_windows.go‎

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"context"
2121
"fmt"
2222
"io"
23-
"net"
2423

2524
winio "github.com/Microsoft/go-winio"
2625
"github.com/containerd/containerd/log"
@@ -43,22 +42,21 @@ func NewFIFOSetInDir(_, id string, terminal bool) (*FIFOSet, error) {
4342
}, nil), nil
4443
}
4544

46-
func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) {
47-
var (
48-
set []io.Closer
49-
)
45+
func copyIO(fifos *FIFOSet, ioset *Streams) (_ *cio, retErr error) {
46+
cios := &cio{config: fifos.Config}
47+
48+
defer func() {
49+
if retErr != nil {
50+
_ = cios.Close()
51+
}
52+
}()
5053

5154
if fifos.Stdin != "" {
5255
l, err := winio.ListenPipe(fifos.Stdin, nil)
5356
if err != nil {
5457
return nil, errors.Wrapf(err, "failed to create stdin pipe %s", fifos.Stdin)
5558
}
56-
defer func(l net.Listener) {
57-
if err != nil {
58-
l.Close()
59-
}
60-
}(l)
61-
set = append(set, l)
59+
cios.closers = append(cios.closers, l)
6260

6361
go func() {
6462
c, err := l.Accept()
@@ -81,12 +79,7 @@ func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) {
8179
if err != nil {
8280
return nil, errors.Wrapf(err, "failed to create stdout pipe %s", fifos.Stdout)
8381
}
84-
defer func(l net.Listener) {
85-
if err != nil {
86-
l.Close()
87-
}
88-
}(l)
89-
set = append(set, l)
82+
cios.closers = append(cios.closers, l)
9083

9184
go func() {
9285
c, err := l.Accept()
@@ -109,12 +102,7 @@ func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) {
109102
if err != nil {
110103
return nil, errors.Wrapf(err, "failed to create stderr pipe %s", fifos.Stderr)
111104
}
112-
defer func(l net.Listener) {
113-
if err != nil {
114-
l.Close()
115-
}
116-
}(l)
117-
set = append(set, l)
105+
cios.closers = append(cios.closers, l)
118106

119107
go func() {
120108
c, err := l.Accept()
@@ -132,7 +120,7 @@ func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) {
132120
}()
133121
}
134122

135-
return &cio{config: fifos.Config, closers: set}, nil
123+
return cios, nil
136124
}
137125

138126
// NewDirectIO returns an IO implementation that exposes the IO streams as io.ReadCloser

0 commit comments

Comments
 (0)