os: guarantee min buffer size for ReadFile reads on /proc-like files

For instance, this fixes os.ReadFile on plan9's /net/iproute file.

But it's not necessarily plan9-specific; Linux /proc and /sys filesystems
can exhibit the same problems.

Fixes #72080

Change-Id: I60b035913f583a91c6d84df95a6ea7b7ec2b3c92
Reviewed-on: https://go-review.googlesource.com/c/go/+/654315
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

4 files changed, 135 insertions, 18 deletions

diff --git a/src/os/export_test.go b/src/os/export_test.go
index dc7caae267..03df0ffccd 100644
--- a/src/os/export_test.go
+++ b/src/os/export_test.go
@@ -15,3 +15,5 @@ var ErrPatternHasSeparator = errPatternHasSeparator
 func init() {
 	checkWrapErr = true
 }
+
+var ExportReadFileContents = readFileContents
diff --git a/src/os/file.go b/src/os/file.go
index 32ff6be7be..1aeb0d2864 100644
--- a/src/os/file.go
+++ b/src/os/file.go
@@ -51,6 +51,7 @@ import (
 	"io"
 	"io/fs"
 	"runtime"
+	"slices"
 	"syscall"
 	"time"
 	"unsafe"
@@ -846,30 +847,45 @@ func ReadFile(name string) ([]byte, error) {
 		return nil, err
 	}
 	defer f.Close()
-	return readFileContents(f)
+
+	return readFileContents(statOrZero(f), f.Read)
+}
+
+func statOrZero(f *File) int64 {
+	if fi, err := f.Stat(); err == nil {
+		return fi.Size()
+	}
+	return 0
 }
 
-func readFileContents(f *File) ([]byte, error) {
+// readFileContents reads the contents of a file using the provided read function
+// (*os.File.Read, except in tests) one or more times, until an error is seen.
+//
+// The provided size is the stat size of the file, which might be 0 for a
+// /proc-like file that doesn't report a size.
+func readFileContents(statSize int64, read func([]byte) (int, error)) ([]byte, error) {
+	zeroSize := statSize == 0
+
+	// Figure out how big to make the initial slice. For files with known size
+	// that fit in memory, use that size + 1. Otherwise, use a small buffer and
+	// we'll grow.
 	var size int
-	if info, err := f.Stat(); err == nil {
-		size64 := info.Size()
-		if int64(int(size64)) == size64 {
-			size = int(size64)
-		}
+	if int64(int(statSize)) == statSize {
+		size = int(statSize)
 	}
 	size++ // one byte for final read at EOF
 
-	// If a file claims a small size, read at least 512 bytes.
-	// In particular, files in Linux's /proc claim size 0 but
-	// then do not work right if read in small pieces,
-	// so an initial read of 1 byte would not work correctly.
-	if size < 512 {
-		size = 512
+	const minBuf = 512
+	// If a file claims a small size, read at least 512 bytes. In particular,
+	// files in Linux's /proc claim size 0 but then do not work right if read in
+	// small pieces, so an initial read of 1 byte would not work correctly.
+	if size < minBuf {
+		size = minBuf
 	}
 
 	data := make([]byte, 0, size)
 	for {
-		n, err := f.Read(data[len(data):cap(data)])
+		n, err := read(data[len(data):cap(data)])
 		data = data[:len(data)+n]
 		if err != nil {
 			if err == io.EOF {
@@ -878,9 +894,12 @@ func readFileContents(f *File) ([]byte, error) {
 			return data, err
 		}
 
-		if len(data) >= cap(data) {
-			d := append(data[:cap(data)], 0)
-			data = d[:len(data)]
+		// If we're either out of capacity or if the file was a /proc-like zero
+		// sized file, grow the buffer. Per Issue 72080, we always want to issue
+		// Read calls on zero-length files with a non-tiny buffer size.
+		capRemain := cap(data) - len(data)
+		if capRemain == 0 || (zeroSize && capRemain < minBuf) {
+			data = slices.Grow(data, minBuf)
 		}
 	}
 }
diff --git a/src/os/os_test.go b/src/os/os_test.go
index 03fe6b1134..81c9fddf5f 100644
--- a/src/os/os_test.go
+++ b/src/os/os_test.go
@@ -3855,3 +3855,99 @@ func TestOpenFileDevNull(t *testing.T) {
 	}
 	f.Close()
 }
+
+func TestReadFileContents(t *testing.T) {
+	type readStep struct {
+		bufSize int   // non-zero to check length of buf to Read
+		retN    int   // result of Read call
+		retErr  error // error result of Read call
+	}
+	errFoo := errors.New("foo")
+	tests := []struct {
+		name     string
+		statSize int64 // size of file to read, per stat (may be 0 for /proc files)
+		wantSize int   // wanted length of []byte from readFileContents
+		wantErr  error // wanted error from readFileContents
+		reads    []readStep
+	}{
+		{
+			name:     "big-file",
+			statSize: 2000,
+			wantSize: 2000,
+			reads: []readStep{
+				{bufSize: 2001, retN: 21, retErr: nil},
+				{bufSize: 1980, retN: 1979, retErr: io.EOF},
+			},
+		},
+		{
+			name:     "small-file",
+			statSize: 100,
+			wantSize: 100,
+			reads: []readStep{
+				{bufSize: 512, retN: 100, retErr: io.EOF},
+			},
+		},
+		{
+			name:     "returning-error",
+			statSize: 1000,
+			wantSize: 50,
+			wantErr:  errFoo,
+			reads: []readStep{
+				{bufSize: 1001, retN: 25, retErr: nil},
+				{retN: 25, retErr: errFoo},
+			},
+		},
+		{
+			name:     "proc-file",
+			statSize: 0,
+			wantSize: 1023,
+			reads: []readStep{
+				{bufSize: 512, retN: 512, retErr: nil},
+				{retN: 511, retErr: io.EOF},
+			},
+		},
+		{
+			name:     "plan9-iproute-file", // Issue 72080
+			statSize: 0,
+			wantSize: 1032,
+			reads: []readStep{
+				{bufSize: 512, retN: 511, retErr: nil},
+				{retN: 511, retErr: nil},
+				{retN: 10, retErr: io.EOF},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			remain := tt.reads
+			i := -1
+			got, err := ExportReadFileContents(tt.statSize, func(buf []byte) (int, error) {
+				i++
+				t.Logf("read[%d] with buf size %d", i, len(buf))
+				if len(remain) == 0 {
+					t.Fatalf("unexpected read of length %d after %d expected reads", len(buf), len(tt.reads))
+				}
+				if tt.statSize == 0 && len(buf) < 512 {
+					// Issue 72080: readFileContents should not do /proc reads with buffers
+					// smaller than 512.
+					t.Fatalf("read[%d] with buf size %d; want at least 512 for 0-sized file", i, len(buf))
+				}
+				step := remain[0]
+				remain = remain[1:]
+				if step.bufSize != 0 && len(buf) != step.bufSize {
+					t.Fatalf("read[%d] has buffer size %d; want %d", i, len(buf), step.bufSize)
+				}
+				return step.retN, step.retErr
+			})
+			if len(remain) > 0 {
+				t.Fatalf("expected %d reads, got %d", len(tt.reads), i+1)
+			}
+			if fmt.Sprint(err) != fmt.Sprint(tt.wantErr) {
+				t.Errorf("got error %v; want %v", err, tt.wantErr)
+			}
+			if len(got) != tt.wantSize {
+				t.Errorf("got size %d; want %d", len(got), tt.wantSize)
+			}
+		})
+	}
+}
diff --git a/src/os/root.go b/src/os/root.go
index a7e667b3c8..0d2c79640d 100644
--- a/src/os/root.go
+++ b/src/os/root.go
@@ -312,7 +312,7 @@ func (rfs *rootFS) ReadFile(name string) ([]byte, error) {
 		return nil, err
 	}
 	defer f.Close()
-	return readFileContents(f)
+	return readFileContents(statOrZero(f), f.Read)
 }
 
 func (rfs *rootFS) Stat(name string) (FileInfo, error) {