From: Brad Fitzpatrick Date: Mon, 3 Mar 2025 17:21:26 +0000 (-0800) Subject: os: guarantee min buffer size for ReadFile reads on /proc-like files X-Git-Tag: go1.25rc1~841 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=ba3c57fc7ceb6c1158e81ccd8071cdeb7a6d6793;p=gostls13.git 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 Reviewed-by: Damien Neil LUCI-TryBot-Result: Go LUCI --- 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) {