From 680f8d97932566ad9bde24b44c11f591ec16c7d3 Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Tue, 27 Sep 2022 00:37:27 +0200 Subject: [PATCH] all: fix more resource leaks found by staticmajor Updates #5706 Signed-off-by: Emmanuel T Odeke --- cmd/tailscaled/debug.go | 10 +++++++++- cmd/tailscaled/tailscaled.go | 2 ++ cmd/tsconnect/common.go | 5 +++++ control/controlclient/auto.go | 8 +++++++- ssh/tailssh/tailssh.go | 8 +++++++- tsnet/example/tshello/tshello.go | 3 +++ wgengine/bench/bench.go | 12 ++++++++++++ 7 files changed, 45 insertions(+), 3 deletions(-) diff --git a/cmd/tailscaled/debug.go b/cmd/tailscaled/debug.go index 247c0de53..1672af20f 100644 --- a/cmd/tailscaled/debug.go +++ b/cmd/tailscaled/debug.go @@ -88,6 +88,8 @@ func runMonitor(ctx context.Context, loop bool) error { if err != nil { return err } + defer mon.Close() + mon.RegisterChangeCallback(func(changed bool, st *interfaces.State) { if !changed { log.Printf("Link monitor fired; no change") @@ -162,7 +164,7 @@ func getURL(ctx context.Context, urlStr string) error { return res.Write(os.Stdout) } -func checkDerp(ctx context.Context, derpRegion string) error { +func checkDerp(ctx context.Context, derpRegion string) (err error) { req, err := http.NewRequestWithContext(ctx, "GET", ipn.DefaultControlURL+"/derpmap/default", nil) if err != nil { return fmt.Errorf("create derp map request: %w", err) @@ -201,6 +203,12 @@ func checkDerp(ctx context.Context, derpRegion string) error { c1 := derphttp.NewRegionClient(priv1, log.Printf, getRegion) c2 := derphttp.NewRegionClient(priv2, log.Printf, getRegion) + defer func() { + if err != nil { + c1.Close() + c2.Close() + } + }() c2.NotePreferred(true) // just to open it diff --git a/cmd/tailscaled/tailscaled.go b/cmd/tailscaled/tailscaled.go index 5878b3590..49268515d 100644 --- a/cmd/tailscaled/tailscaled.go +++ b/cmd/tailscaled/tailscaled.go @@ -564,6 +564,8 @@ func tryEngine(logf logger.Logf, linkMon *monitor.Mon, dialer *tsdial.Dialer, na } d, err := dns.NewOSConfigurator(logf, devName) if err != nil { + dev.Close() + r.Close() return nil, false, fmt.Errorf("dns.NewOSConfigurator: %w", err) } conf.DNS = d diff --git a/cmd/tsconnect/common.go b/cmd/tsconnect/common.go index 06097ee6b..20ee449f0 100644 --- a/cmd/tsconnect/common.go +++ b/cmd/tsconnect/common.go @@ -187,7 +187,12 @@ func buildWasm(dev bool) ([]byte, error) { return nil, fmt.Errorf("Cannot create main.wasm output file: %w", err) } outputPath := outputFile.Name() + defer os.Remove(outputPath) + // Running defer (*os.File).Close() in defer order before os.Remove + // because on some systems like Windows, it is possible for os.Remove + // to fail for unclosed files. + defer outputFile.Close() args := []string{"build", "-tags", "tailscale_go,osusergo,netgo,nethttpomithttp2,omitidna,omitpemdecrypt"} if !dev { diff --git a/control/controlclient/auto.go b/control/controlclient/auto.go index c4348b8dc..7ea520b94 100644 --- a/control/controlclient/auto.go +++ b/control/controlclient/auto.go @@ -88,11 +88,17 @@ func New(opts Options) (*Auto, error) { } // NewNoStart creates a new Auto, but without calling Start on it. -func NewNoStart(opts Options) (*Auto, error) { +func NewNoStart(opts Options) (_ *Auto, err error) { direct, err := NewDirect(opts) if err != nil { return nil, err } + defer func() { + if err != nil { + direct.Close() + } + }() + if opts.Status == nil { return nil, errors.New("missing required Options.Status") } diff --git a/ssh/tailssh/tailssh.go b/ssh/tailssh/tailssh.go index 870f7f4a9..22ba54334 100644 --- a/ssh/tailssh/tailssh.go +++ b/ssh/tailssh/tailssh.go @@ -1258,7 +1258,7 @@ func randBytes(n int) []byte { // // It writes an asciinema file to // $TAILSCALE_VAR_ROOT/ssh-sessions/ssh-session--*.cast. -func (ss *sshSession) startNewRecording() (*recording, error) { +func (ss *sshSession) startNewRecording() (_ *recording, err error) { var w ssh.Window if ptyReq, _, isPtyReq := ss.Pty(); isPtyReq { w = ptyReq.Window @@ -1282,6 +1282,12 @@ func (ss *sshSession) startNewRecording() (*recording, error) { if err := os.MkdirAll(dir, 0700); err != nil { return nil, err } + defer func() { + if err != nil { + rec.Close() + } + }() + f, err := os.CreateTemp(dir, fmt.Sprintf("ssh-session-%v-*.cast", now.UnixNano())) if err != nil { return nil, err diff --git a/tsnet/example/tshello/tshello.go b/tsnet/example/tshello/tshello.go index 57b8fa2a4..8148967f1 100644 --- a/tsnet/example/tshello/tshello.go +++ b/tsnet/example/tshello/tshello.go @@ -25,10 +25,13 @@ var ( func main() { flag.Parse() s := new(tsnet.Server) + defer s.Close() ln, err := s.Listen("tcp", *addr) if err != nil { log.Fatal(err) } + defer ln.Close() + if *addr == ":443" { ln = tls.NewListener(ln, &tls.Config{ GetCertificate: tailscale.GetCertificate, diff --git a/wgengine/bench/bench.go b/wgengine/bench/bench.go index 63ec09fcd..7a204ced6 100644 --- a/wgengine/bench/bench.go +++ b/wgengine/bench/bench.go @@ -16,6 +16,7 @@ import ( "net/netip" "os" "strconv" + "sync" "time" "tailscale.com/types/logger" @@ -323,6 +324,13 @@ func setupBatchTCPTest(logf logger.Logf, traf *TrafficGen) { log.Fatalf("listen: %v", err) } + var slCloseOnce sync.Once + slClose := func() { + slCloseOnce.Do(func() { + sl.Close() + }) + } + s1, err := net.Dial("tcp", sl.Addr().String()) if err != nil { log.Fatalf("dial: %v", err) @@ -340,6 +348,8 @@ func setupBatchTCPTest(logf logger.Logf, traf *TrafficGen) { go func() { // transmitter + defer slClose() + defer s1.Close() bs1 := bufio.NewWriterSize(s1, 1024*1024) @@ -375,6 +385,8 @@ func setupBatchTCPTest(logf logger.Logf, traf *TrafficGen) { go func() { // receiver + defer slClose() + defer s2.Close() bs2 := bufio.NewReaderSize(s2, 1024*1024)