From 219fa3d0a5a1d3a86479f6d54aef2b94c1fdf9ea Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Fri, 29 Jan 2021 12:31:46 +0100
Subject: [PATCH] cio.copyIO: fix pipes potentially not being closed (Windows)

The defer functions were checking the local variable, and would therefore
not be executed, as the function returned if an error occurred.

Perhaps best illustrated when renaming the local variables;

    if fifos.Stdin != "" {
        l, err1 := winio.ListenPipe(fifos.Stdin, nil)
        if err1 != nil {
            return nil, errors.Wrapf(err1, "failed to create stdin pipe %s", fifos.Stdin)
        }
        defer func(l net.Listener) {
            if err1 != nil {
                l.Close()
            }
        }(l)
        // ...
    }

    if fifos.Stdout != "" {
        l, err2 := winio.ListenPipe(fifos.Stdout, nil)
        if err2 != nil {
            return nil, errors.Wrapf(err2, "failed to create stdout pipe %s", fifos.Stdout)
        }
        defer func(l net.Listener) {
            if err2 != nil {
                l.Close()
            }
        }(l)
        // ....
    }

This patch changes the function to use a named return variable, and to use
a single `defer()` that closes all pipes.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 cio/io_windows.go | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/cio/io_windows.go b/cio/io_windows.go
index ca6cd54a4..5940b4854 100644
--- a/cio/io_windows.go
+++ b/cio/io_windows.go
@@ -20,7 +20,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"net"
 
 	winio "github.com/Microsoft/go-winio"
 	"github.com/containerd/containerd/log"
@@ -43,21 +42,28 @@ func NewFIFOSetInDir(_, id string, terminal bool) (*FIFOSet, error) {
 	}, nil), nil
 }
 
-func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) {
+func copyIO(fifos *FIFOSet, ioset *Streams) (_ *cio, retErr error) {
 	var (
 		set []io.Closer
 	)
 
+	defer func() {
+		if retErr == nil {
+			return
+		}
+		for _, closer := range set {
+			if closer == nil {
+				continue
+			}
+			_ = closer.Close()
+		}
+	}()
+
 	if fifos.Stdin != "" {
 		l, err := winio.ListenPipe(fifos.Stdin, nil)
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to create stdin pipe %s", fifos.Stdin)
 		}
-		defer func(l net.Listener) {
-			if err != nil {
-				l.Close()
-			}
-		}(l)
 		set = append(set, l)
 
 		go func() {
@@ -81,11 +87,6 @@ func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) {
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to create stdout pipe %s", fifos.Stdout)
 		}
-		defer func(l net.Listener) {
-			if err != nil {
-				l.Close()
-			}
-		}(l)
 		set = append(set, l)
 
 		go func() {
@@ -109,11 +110,6 @@ func copyIO(fifos *FIFOSet, ioset *Streams) (*cio, error) {
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to create stderr pipe %s", fifos.Stderr)
 		}
-		defer func(l net.Listener) {
-			if err != nil {
-				l.Close()
-			}
-		}(l)
 		set = append(set, l)
 
 		go func() {