From: Bryan C. Mills <bcmills@google.com>
Date: Tue, 12 Nov 2019 15:18:06 +0000 (-0500)
Subject: cmd/dist: save and restore original permissions in makeGOROOTUnwritable
X-Git-Tag: go1.14beta1~232
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=fc7ee0ba2c9f284ea2d4a37a0c133524e94d88cd;p=gostls13.git

cmd/dist: save and restore original permissions in makeGOROOTUnwritable

Also log a message and skip the Chmods if running as root.

Updates #30316

Change-Id: Ifb68d06ce845275a72d64c808407e8609df270bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/206757
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
---

diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go
index e0fa51f146..84ad5fd59d 100644
--- a/src/cmd/dist/test.go
+++ b/src/cmd/dist/test.go
@@ -192,8 +192,13 @@ func (t *tester) run() {
 	}
 
 	// On a few builders, make GOROOT unwritable to catch tests writing to it.
+	restoreGOROOT := func() {}
 	if strings.HasPrefix(os.Getenv("GO_BUILDER_NAME"), "linux-") {
-		t.makeGOROOTUnwritable()
+		if os.Getuid() == 0 {
+			log.Printf("Not making GOROOT unwritable: running as root, so permissions would have no effect.")
+		} else {
+			restoreGOROOT = t.makeGOROOTUnwritable()
+		}
 	}
 
 	for _, dt := range t.tests {
@@ -208,12 +213,15 @@ func (t *tester) run() {
 			if t.keepGoing {
 				log.Printf("Failed: %v", err)
 			} else {
+				restoreGOROOT()
 				log.Fatalf("Failed: %v", err)
 			}
 		}
 	}
 	t.runPending(nil)
+	restoreGOROOT()
 	timelog("end", "dist test")
+
 	if t.failed {
 		fmt.Println("\nFAILED")
 		os.Exit(1)
@@ -1423,32 +1431,45 @@ func (t *tester) packageHasBenchmarks(pkg string) bool {
 
 // makeGOROOTUnwritable makes all $GOROOT files & directories non-writable to
 // check that no tests accidentally write to $GOROOT.
-func (t *tester) makeGOROOTUnwritable() {
-	if os.Getenv("GO_BUILDER_NAME") == "" {
-		panic("not a builder")
-	}
-	if os.Getenv("GOROOT") == "" {
+func (t *tester) makeGOROOTUnwritable() (undo func()) {
+	dir := os.Getenv("GOROOT")
+	if dir == "" {
 		panic("GOROOT not set")
 	}
-	err := filepath.Walk(os.Getenv("GOROOT"), func(name string, fi os.FileInfo, err error) error {
-		if err != nil {
-			return err
-		}
-		if !fi.Mode().IsRegular() && !fi.IsDir() {
-			return nil
+
+	type pathMode struct {
+		path string
+		mode os.FileMode
+	}
+	var dirs []pathMode // in lexical order
+
+	undo = func() {
+		for i := range dirs {
+			os.Chmod(dirs[i].path, dirs[i].mode) // best effort
 		}
-		mode := fi.Mode()
-		newMode := mode & ^os.FileMode(0222)
-		if newMode != mode {
-			if err := os.Chmod(name, newMode); err != nil {
-				return err
+	}
+
+	filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
+		if err == nil {
+			mode := info.Mode()
+			if mode&0222 != 0 && (mode.IsDir() || mode.IsRegular()) {
+				dirs = append(dirs, pathMode{path, mode})
 			}
 		}
 		return nil
 	})
-	if err != nil {
-		log.Fatalf("making builder's files read-only: %v", err)
+
+	// Run over list backward to chmod children before parents.
+	for i := len(dirs) - 1; i >= 0; i-- {
+		err := os.Chmod(dirs[i].path, dirs[i].mode&^0222)
+		if err != nil {
+			dirs = dirs[i:] // Only undo what we did so far.
+			undo()
+			log.Fatalf("failed to make GOROOT read-only: %v", err)
+		}
 	}
+
+	return undo
 }
 
 // shouldUsePrecompiledStdTest reports whether "dist test" should use