fileutil: fix race in ProcessTempDir

One goroutine decides that it needs to clean up an instance,
but before it tries to delete pid file it is preempted.
Then another goroutine cleans up this instances and creates
a new instances in the same dir.
Then first goroutine removes already new pid file and removes
the used dir.

Fix this by using flock on a lock file.
Add a test.

2 files changed, 93 insertions, 2 deletions

diff --git a/fileutil/fileutil.go b/fileutil/fileutil.go
index 34e2562fa..a019a14ea 100644
--- a/fileutil/fileutil.go
+++ b/fileutil/fileutil.go
@@ -59,14 +59,25 @@ func WriteTempFile(data []byte) (string, error) {
 // ProcessTempDir creates a new temp dir in where and returns its path and an unique index.
 // It also cleans up old, unused temp dirs after dead processes.
 func ProcessTempDir(where string) (string, int, error) {
-	for i := 0; i < 1e4; i++ {
+	lk := filepath.Join(where, "instance-lock")
+	lkf, err := syscall.Open(lk, syscall.O_RDWR|syscall.O_CREAT, 0600)
+	if err != nil {
+		return "", 0, err
+	}
+	defer syscall.Close(lkf)
+	if err := syscall.Flock(lkf, syscall.LOCK_EX); err != nil {
+		return "", 0, err
+	}
+	defer syscall.Flock(lkf, syscall.LOCK_UN)
+
+	for i := 0; i < 1e3; i++ {
 		path := filepath.Join(where, fmt.Sprintf("instance-%v", i))
 		pidfile := filepath.Join(path, ".pid")
 		err := os.Mkdir(path, 0700)
 		if os.IsExist(err) {
 			// Try to clean up.
 			data, err := ioutil.ReadFile(pidfile)
-			if err == nil {
+			if err == nil && len(data) > 0 {
 				pid, err := strconv.Atoi(string(data))
 				if err == nil && pid > 1 {
 					if err := syscall.Kill(pid, 0); err == syscall.ESRCH {
diff --git a/fileutil/fileutil_test.go b/fileutil/fileutil_test.go
new file mode 100644
index 000000000..1084cc98d
--- /dev/null
+++ b/fileutil/fileutil_test.go
@@ -0,0 +1,80 @@
+// Copyright 2015 syzkaller project authors. All rights reserved.
+// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
+
+package fileutil
+
+import (
+	"bytes"
+	"io/ioutil"
+	"os"
+	"strconv"
+	"sync"
+	"testing"
+	"path/filepath"
+)
+
+func TestProcessTempDir(t *testing.T) {
+	for try := 0; try < 10; try++ {
+		func() {
+			tmp, err := ioutil.TempDir("", "syz")
+			if err != nil {
+				t.Fatalf("failed to create a temp dir: %v", err)
+			}
+			defer os.RemoveAll(tmp)
+			const P = 16
+			// Pre-create half of the instances with stale pid.
+			var dirs []string
+			for i := 0; i < P/2; i++ {
+				dir, idx, err := ProcessTempDir(tmp)
+				if err != nil {
+					t.Fatalf("failed to create process temp dir")
+				}
+				if idx != i {
+					t.Fatalf("unexpected index: want %v, got %v", i, idx)
+				}
+				dirs = append(dirs, dir)
+			}
+			for _, dir := range dirs {
+				if err := ioutil.WriteFile(filepath.Join(dir, ".pid"), []byte(strconv.Itoa(999999999)), 0600); err != nil {
+					t.Fatalf("failed to write pid file: %v", err)
+				}
+			}
+			// Now request a bunch of instances concurrently.
+			done := make(chan bool)
+			indices := make(map[int]bool)
+			var mu sync.Mutex
+			for p := 0; p < P; p++ {
+				go func() {
+					defer func() {
+						done <- true
+					}()
+					dir, idx, err := ProcessTempDir(tmp)
+					if err != nil {
+						t.Fatalf("failed to create process temp dir")
+					}
+					mu.Lock()
+					present := indices[idx]
+					indices[idx] = true
+					mu.Unlock()
+					if present {
+						t.Fatalf("duplicate index %v", idx)
+					}
+					data := []byte(strconv.Itoa(idx))
+					if err := ioutil.WriteFile(filepath.Join(dir, "data"), data, 0600); err != nil {
+						t.Fatalf("failed to write data file: %v", err)
+					}
+					data1, err := ioutil.ReadFile(filepath.Join(dir, "data"))
+					if err != nil {
+						t.Fatalf("failed to read data file: %v", err)
+					}
+					if bytes.Compare(data, data1) != 0 {
+						t.Fatalf("corrupted data file")
+					}
+				}()
+			}
+			for p := 0; p < P; p++ {
+				<-done
+			}
+		}()
+	}
+}