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: 994c12d9da
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>

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])
-		}
-	}
-}

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")
 }

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) {