From e91876f4406a89eb0b0c07e93a6ae4e4f32d48b6 Mon Sep 17 00:00:00 2001
From: Roland Shoemaker <roland@golang.org>
Date: Wed, 15 Mar 2023 20:53:10 -0700
Subject: [PATCH] internal/fuzz: release lock when reading file fails

When corpusEntryData failed in workerClient.fuzz and
workerClient.minimize, the shared memory mutex wasn't properly given up,
which would cause a deadlock when worker.cleanup was called.

This was tickled by #59062, wherein the fuzz cache directory would be
removed during operation of the fuzzer, causing corpusEntryData to fail
because the entry files no longer existed.

Updates #51484

Change-Id: Iea284041c20d1581c662bddbbc7e12191771a364
Reviewed-on: https://go-review.googlesource.com/c/go/+/476815
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
---
 .../script/test_fuzz_err_deadlock.txt         | 50 +++++++++++++++++++
 src/internal/fuzz/worker.go                   |  3 +-
 2 files changed, 52 insertions(+), 1 deletion(-)
 create mode 100644 src/cmd/go/testdata/script/test_fuzz_err_deadlock.txt

diff --git a/src/cmd/go/testdata/script/test_fuzz_err_deadlock.txt b/src/cmd/go/testdata/script/test_fuzz_err_deadlock.txt
new file mode 100644
index 0000000000..4feb41a30f
--- /dev/null
+++ b/src/cmd/go/testdata/script/test_fuzz_err_deadlock.txt
@@ -0,0 +1,50 @@
+[short] skip
+[!fuzz-instrumented] skip
+
+env GOCACHE=$WORK/cache
+! go test -fuzz=FuzzDead -v
+# This is a somewhat inexact check, but since we don't prefix the error with anything
+# and as the error suffix is platform dependent, this is the best we can do. In the
+# deadlock failure case, the test will just deadlock and timeout anyway, so it should
+# be clear that that failure mode is different.
+stdout 'open'
+
+-- go.mod --
+module test
+
+-- cov_test.go --
+package dead
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+func FuzzDead(f *testing.F) {
+	go func() {
+		c := filepath.Join(os.Getenv("GOCACHE"), "fuzz", "test", "FuzzDead")
+		t := time.NewTicker(time.Second)
+		for range t.C {
+			files, _ := os.ReadDir(c)
+			if len(files) > 0 {
+				os.RemoveAll(c)
+			}
+		}
+	}()
+
+	f.Fuzz(func(t *testing.T, b []byte) {
+		if len(b) == 8 &&
+			b[0] == 'h' &&
+			b[1] == 'e' &&
+			b[2] == 'l' &&
+			b[3] == 'l' &&
+			b[4] == 'o' &&
+			b[5] == ' ' &&
+			b[6] == ':' &&
+			b[7] == ')' {
+			return
+		}
+	})
+}
diff --git a/src/internal/fuzz/worker.go b/src/internal/fuzz/worker.go
index d40245a3f2..c952670995 100644
--- a/src/internal/fuzz/worker.go
+++ b/src/internal/fuzz/worker.go
@@ -993,13 +993,13 @@ func (wc *workerClient) minimize(ctx context.Context, entryIn CorpusEntry, args
 	if !ok {
 		return CorpusEntry{}, minimizeResponse{}, errSharedMemClosed
 	}
+	defer func() { wc.memMu <- mem }()
 	mem.header().count = 0
 	inp, err := corpusEntryData(entryIn)
 	if err != nil {
 		return CorpusEntry{}, minimizeResponse{}, err
 	}
 	mem.setValue(inp)
-	defer func() { wc.memMu <- mem }()
 	entryOut = entryIn
 	entryOut.Values, err = unmarshalCorpusFile(inp)
 	if err != nil {
@@ -1082,6 +1082,7 @@ func (wc *workerClient) fuzz(ctx context.Context, entryIn CorpusEntry, args fuzz
 	mem.header().count = 0
 	inp, err := corpusEntryData(entryIn)
 	if err != nil {
+		wc.memMu <- mem
 		return CorpusEntry{}, fuzzResponse{}, true, err
 	}
 	mem.setValue(inp)
-- 
2.50.0