]> Cypherpunks repositories - gostls13.git/commitdiff
os: guarantee min buffer size for ReadFile reads on /proc-like files
authorBrad Fitzpatrick <bradfitz@golang.org>
Mon, 3 Mar 2025 17:21:26 +0000 (09:21 -0800)
committerBrad Fitzpatrick <bradfitz@golang.org>
Tue, 4 Mar 2025 16:58:31 +0000 (08:58 -0800)
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>

src/os/export_test.go
src/os/file.go
src/os/os_test.go
src/os/root.go

index dc7caae267d7041134c6a700885eb117c567de7c..03df0ffccd3cea3145d9bf817989a207ae635473 100644 (file)
@@ -15,3 +15,5 @@ var ErrPatternHasSeparator = errPatternHasSeparator
 func init() {
        checkWrapErr = true
 }
+
+var ExportReadFileContents = readFileContents
index 32ff6be7be253823259f334d0ae632aea931fdc8..1aeb0d2864c105f22ef12627ff3bdd9d47f64edf 100644 (file)
@@ -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)
                }
        }
 }
index 03fe6b11345476142ec520eb31d5009918cdf414..81c9fddf5f5e606664a0ae0dc75bdc9c32c1490b 100644 (file)
@@ -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)
+                       }
+               })
+       }
+}
index a7e667b3c823af7353a60866d89e99e9d3653582..0d2c79640dda0a9eacd379d2a1555056d9e53849 100644 (file)
@@ -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) {