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>
pull/3140/head
Josh Bleecher Snyder 3 years ago committed by Josh Bleecher Snyder
parent 3fd5f4380f
commit c467ed0b62

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

Loading…
Cancel
Save