debug/elf: return error on reading from SHT_NOBITS sections

An SHT_NOBITS section contains no bytes and occupies no space in the
file. This change makes it return an error on reading from this section
so that it will force the caller to check for an SHT_NNOBITS section.

We have considered another option to return "nil, nil" for the Data
method. It's abandoned because it might lead a program to simply do
the wrong thing, thinking that the section is empty.

Please note that it breaks programs which expect a byte slice with the
length described by the sh_size field. There are two reasons to
introduce this breaking change: 1. SHT_NOBITS means no data and it's
unnecessary to allocate memory for it; 2. it could result in an OOM if
the file is corrupted and has a huge sh_size.

Fixes #54967.

Change-Id: I0c3ed4e097214fe88413d726a89122105ad45d4f
GitHub-Last-Rev: 994c12d9da8c7029ce1d26eb1eb4d333ef3b1f97
GitHub-Pull-Request: golang/go#54994
Reviewed-on: https://go-review.googlesource.com/c/go/+/429601
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>

diff --git a/src/debug/elf/elf_test.go b/src/debug/elf/elf_test.go
index b8c310dba55f9e3687d0243c567467c6b16e37ae..a61b491090d173a24235ca66becd05afed23cb86 100644 (file)
--- a/src/debug/elf/elf_test.go
+++ b/src/debug/elf/elf_test.go
@@ -47,25 +47,3 @@ func TestNames(t *testing.T) {
                }
        }
 }
-
-func TestNobitsSection(t *testing.T) {
-       const testdata = "testdata/gcc-amd64-linux-exec"
-       f, err := Open(testdata)
-       if err != nil {
-               t.Fatalf("could not read %s: %v", testdata, err)
-       }
-       defer f.Close()
-       bss := f.Section(".bss")
-       bssData, err := bss.Data()
-       if err != nil {
-               t.Fatalf("error reading .bss section: %v", err)
-       }
-       if g, w := uint64(len(bssData)), bss.Size; g != w {
-               t.Errorf(".bss section length mismatch: got %d, want %d", g, w)
-       }
-       for i := range bssData {
-               if bssData[i] != 0 {
-                       t.Fatalf("unexpected non-zero byte at offset %d: %#x", i, bssData[i])
-               }
-       }
-}

diff --git a/src/debug/elf/file.go b/src/debug/elf/file.go
index db07a2daff8800eef0188cdfd2fb4fba9948efa3..83a3cbc0b808b7cb8bc687fd4bb8ed0fc915682d 100644 (file)
--- a/src/debug/elf/file.go
+++ b/src/debug/elf/file.go
@@ -102,6 +102,8 @@ type Section struct {
 // Data reads and returns the contents of the ELF section.
 // Even if the section is stored compressed in the ELF file,
 // Data returns uncompressed data.
+//
+// For an SHT_NOBITS section, Data always returns a non-nil error.
 func (s *Section) Data() ([]byte, error) {
        return saferio.ReadData(s.Open(), s.Size)
 }
@@ -118,9 +120,12 @@ func (f *File) stringTable(link uint32) ([]byte, error) {
 // Open returns a new ReadSeeker reading the ELF section.
 // Even if the section is stored compressed in the ELF file,
 // the ReadSeeker reads uncompressed data.
+//
+// For an SHT_NOBITS section, all calls to the opened reader
+// will return a non-nil error.
 func (s *Section) Open() io.ReadSeeker {
        if s.Type == SHT_NOBITS {
-               return io.NewSectionReader(&zeroReader{}, 0, int64(s.Size))
+               return io.NewSectionReader(&nobitsSectionReader{}, 0, int64(s.Size))
        }
        if s.Flags&SHF_COMPRESSED == 0 {
                return io.NewSectionReader(s.sr, 0, 1<<63-1)
@@ -1602,11 +1607,8 @@ func (f *File) DynString(tag DynTag) ([]string, error) {
        return all, nil
 }
 
-type zeroReader struct{}
+type nobitsSectionReader struct{}
 
-func (*zeroReader) ReadAt(p []byte, off int64) (n int, err error) {
-       for i := range p {
-               p[i] = 0
-       }
-       return len(p), nil
+func (*nobitsSectionReader) ReadAt(p []byte, off int64) (n int, err error) {
+       return 0, errors.New("unexpected read from SHT_NOBITS section")
 }

diff --git a/src/debug/elf/file_test.go b/src/debug/elf/file_test.go
index fe72a1908f36c132a32c6a65272e94ead326f36d..282e1fccd91e3697dea3380cd1ef862da18104b8 100644 (file)
--- a/src/debug/elf/file_test.go
+++ b/src/debug/elf/file_test.go
@@ -945,6 +945,30 @@ func TestNoSectionOverlaps(t *testing.T) {
        }
 }
 
+func TestNobitsSection(t *testing.T) {
+       const testdata = "testdata/gcc-amd64-linux-exec"
+       f, err := Open(testdata)
+       if err != nil {
+               t.Fatalf("could not read %s: %v", testdata, err)
+       }
+       defer f.Close()
+
+       wantError := "unexpected read from SHT_NOBITS section"
+       bss := f.Section(".bss")
+
+       _, err = bss.Data()
+       if err == nil || err.Error() != wantError {
+               t.Fatalf("bss.Data() got error %q, want error %q", err, wantError)
+       }
+
+       r := bss.Open()
+       p := make([]byte, 1)
+       _, err = r.Read(p)
+       if err == nil || err.Error() != wantError {
+               t.Fatalf("r.Read(p) got error %q, want error %q", err, wantError)
+       }
+}
+
 // TestLargeNumberOfSections tests the case that a file has greater than or
 // equal to 65280 (0xff00) sections.
 func TestLargeNumberOfSections(t *testing.T) {