From c467ed0b628baec6b7cfdb166d2bee724745cc54 Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josh@tailscale.com>
Date: Fri, 29 Oct 2021 13:26:45 -0700
Subject: [PATCH] wgengine/wgcfg: always close io.Pipe

In DeviceConfig, we did not close r after calling FromUAPI.
If FromUAPI returned early due to an error, then it might
not have read all the data that IpcGetOperation wanted to write.
As a result, IpcGetOperation could hang, as in #3220.

We were also closing the wrong end of the pipe after IpcSetOperation
in ReconfigDevice.

To ensure that we get all available information to diagnose
such a situation, include all errors anytime something goes wrong.

This should fix the immediate crashing problem in #3220.
We'll then need to figure out why IpcGetOperation was failing.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
---
 wgengine/wgcfg/device.go | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/wgengine/wgcfg/device.go b/wgengine/wgcfg/device.go
index 526f0e92c..fa1968d64 100644
--- a/wgengine/wgcfg/device.go
+++ b/wgengine/wgcfg/device.go
@@ -10,6 +10,7 @@ import (
 
 	"golang.zx2c4.com/wireguard/device"
 	"tailscale.com/types/logger"
+	"tailscale.com/util/multierr"
 )
 
 func DeviceConfig(d *device.Device) (*Config, error) {
@@ -19,12 +20,10 @@ func DeviceConfig(d *device.Device) (*Config, error) {
 		errc <- d.IpcGetOperation(w)
 		w.Close()
 	}()
-	cfg, err := FromUAPI(r)
-	// Prefer errors from IpcGetOperation.
-	if setErr := <-errc; setErr != nil {
-		return nil, setErr
-	}
-	// Check FromUAPI error.
+	cfg, fromErr := FromUAPI(r)
+	r.Close()
+	getErr := <-errc
+	err := multierr.New(getErr, fromErr)
 	if err != nil {
 		return nil, err
 	}
@@ -51,14 +50,11 @@ func ReconfigDevice(d *device.Device, cfg *Config, logf logger.Logf) (err error)
 	errc := make(chan error, 1)
 	go func() {
 		errc <- d.IpcSetOperation(r)
-		w.Close()
+		r.Close()
 	}()
 
-	err = cfg.ToUAPI(w, prev)
+	toErr := cfg.ToUAPI(w, prev)
 	w.Close()
-	// Prefer errors from IpcSetOperation.
-	if setErr := <-errc; setErr != nil {
-		return setErr
-	}
-	return err // err (if any) from cfg.ToUAPI
+	setErr := <-errc
+	return multierr.New(setErr, toErr)
 }