[PATCH 3/6] gdb: validate the CU vector offset when loading the .gdb_index

[Andrew Burgess <aburgess@redhat.com>]

Within the .gdb_index, the symbol table contains indexes that point to
CU vectors within the constant pool.  Unfortunately, these indexes are
not validated against the size of the constant pool, so a malformed
.gdb_index can cause GDB to read outside the bounds of the .gdb_index.

This commit extends the dwarf.exp gdb_index proc to allow for the CU
offset to be overridden, this is then used to add tests that expose
this bug.

Finally, mapped_gdb_index::build_name_components is updated to sanity
check the CU offset before it is used.  If we find an invalid offset
then that symbol will be ignored.

To improve GDB's ability to catch errors I also added an assert to
offset_view::operator[] that the whole offset_type is within the view.
Without the build_name_components fix this assert will be triggered by
one of the new tests.
---
 gdb/dwarf2/read-gdb-index.c                   | 22 +++++++-
 .../gdb.dwarf2/gdb-index-corrupted.exp        | 54 +++++++++++++++++++
 gdb/testsuite/lib/dwarf.exp                   | 47 ++++++++++++----
 3 files changed, 111 insertions(+), 12 deletions(-)

diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
index e0895fb6aa8..343ad90847f 100644
--- a/gdb/dwarf2/read-gdb-index.c
+++ b/gdb/dwarf2/read-gdb-index.c
@@ -102,6 +102,11 @@ class offset_view
   /* Extract the INDEXth offset_type from the array.  */
   offset_type operator[] (size_t index) const
   {
+    /* Ensure the whole offset_type is within the underlying bytes.  If GDB
+       is compiled with libc++ assert then the access of M_BYTES will be
+       checked, but this only guarantees that the first byte is within the
+       array_view as it's a gdb_byte view.  */
+    gdb_assert (index < size ());
     const gdb_byte *bytes = &m_bytes[index * sizeof (offset_type)];
     return (offset_type) extract_unsigned_integer (bytes,
 						   sizeof (offset_type),
@@ -273,7 +278,22 @@ mapped_gdb_index::build_name_components (dwarf2_per_objfile *per_objfile)
       gdb_assert (components.size () > 0);
 
       std::vector<cooked_index_entry *> these_entries;
-      offset_view vec (constant_pool.slice (symbol_vec_index (idx)));
+
+      offset_type vec_index = symbol_vec_index (idx);
+      /* The CU vector starts with an offset_type length word.  Check that
+	 VEC_INDEX lands inside CONSTANT_POOL, and that there is at least an
+	 offset_type worth of bytes beyond this in CONSTANT_POOL to be
+	 read.  */
+      if (vec_index >= constant_pool.size ()
+	  || constant_pool.size () - vec_index < sizeof (offset_type))
+	{
+	  complaint (_(".gdb_index entry %s has bad CU vector offset %s "
+		       "[in module %s]"),
+		     pulongest (idx), pulongest (vec_index),
+		     objfile_name (per_objfile->objfile));
+	  continue;
+	}
+      offset_view vec (constant_pool.slice (vec_index));
       offset_type vec_len = vec[0];
 
       /* Bounds check VEC_LEN against the CONSTANT_POOL.  */
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
index 356b12f97ce..33c00874002 100644
--- a/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
+++ b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
@@ -173,6 +173,50 @@ proc run_test { testname } {
 		name _start {{cu_label function global}}
 		name foo {{cu_label function global}} 0xffffff00
 	    }
+	} elseif { $testname eq "cu-vector-offset-1" } {
+	    # Add a symbol whose CU vector offset in the symbol table is out
+	    # of range.  The slot has a valid name offset, so GDB won't
+	    # treat it as an empty slot and will try to read the CU vector.
+	    # Ensure GDB doesn't crash and instead just ignores this one
+	    # symbol.
+	    #
+	    # The 'end' offset is exactly the size of the constant pool,
+	    # that is, it points at the first byte after the pool.  This is
+	    # the off-by-one boundary case.
+	    gdb_index {} {
+		cu cu_label
+		name _start {{cu_label function global}}
+		name foo {{cu_label function global}} "" end
+	    }
+	} elseif { $testname eq "cu-vector-offset-2" } {
+	    # Just like 'cu-vector-offset-1', but here the CU vector offset
+	    # is huge, far past the end of the constant pool.  This is far
+	    # more likely to cause GDB to crash.
+	    gdb_index {} {
+		cu cu_label
+		name _start {{cu_label function global}}
+		name foo {{cu_label function global}} "" 0xffffff00
+	    }
+	} elseif { $testname eq "cu-vector-offset-3" } {
+	    # Like cu-vector-offset-1/-2, but aim the CU vector offset at
+	    # the last byte of the constant pool.  That byte offset is in
+	    # range, but there is no room there for the 4-byte CU vector
+	    # length word that GDB reads.  This ensures that GDB is
+	    # correctly validating the entire length of its reads.
+	    #
+	    # Work out the constant pool size from its contents.  There are
+	    # two symbols that need adding to the constant pool, '_start'
+	    # and 'foo'.  For each symbol we have a CU vector (a 4-byte
+	    # count and a 4-byte offset), then we have both symbol strings
+	    # plus their null terminating character.
+	    set pool_size \
+		[expr {2 * (4 + 4) + [string length "_start"] + 1 \
+			   + [string length "foo"] + 1}]
+	    gdb_index {} {
+		cu cu_label
+		name _start {{cu_label function global}}
+		name foo {{cu_label function global}} "" [expr {$pool_size - 1}]
+	    }
 	} elseif { $testname eq "bad-cu-length" } {
 	    # Create an otherwise valid index, but record a bogus length
 	    # for the CU in the index's CU list; one that doesn't match the
@@ -247,6 +291,9 @@ proc run_test { testname } {
 		   || $testname eq "long-cu-vector" \
 		   || $testname eq "name-offset-1" \
 		   || $testname eq "name-offset-2" \
+		   || $testname eq "cu-vector-offset-1" \
+		   || $testname eq "cu-vector-offset-2" \
+		   || $testname eq "cu-vector-offset-3" \
 		   || $testname eq "bad-cu-length" } {
 	set expect_gdb_index_ignored false
 	set expect_start true
@@ -262,6 +309,10 @@ proc run_test { testname } {
 	} elseif { $testname eq "name-offset-1" \
 		       || $testname eq "name-offset-2" } {
 	    set expect_warning_re "\r\nDuring symbol reading: \\.gdb_index entry $::decimal has bad name offset $::decimal \\\[in module \[^\r\n\]+\\\]\r\n"
+	} elseif { $testname eq "cu-vector-offset-1" \
+		       || $testname eq "cu-vector-offset-2" \
+		       || $testname eq "cu-vector-offset-3" } {
+	    set expect_warning_re "\r\nDuring symbol reading: \\.gdb_index entry $::decimal has bad CU vector offset $::decimal \\\[in module \[^\r\n\]+\\\]\r\n"
 	} else {
 	    set expect_warning_re ""
 	}
@@ -371,6 +422,9 @@ set tests { empty-component \
 		long-cu-vector \
 		name-offset-1 \
 		name-offset-2 \
+		cu-vector-offset-1 \
+		cu-vector-offset-2 \
+		cu-vector-offset-3 \
 		bad-cu-length \
 		valid-index }
 
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index 7a82f3915d3..b59e598a163 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -3835,7 +3835,7 @@ namespace eval Dwarf {
     #        .debug_info; providing a (bogus) override allows GDB's
     #        handling of an inconsistent CU length to be tested.
     #
-    #   name <name> <entries> [<name_offset>]
+    #   name <name> <entries> [<name_offset> [<cuvec_offset>]]
     #     -- add symbol <name> with its CU vector to the constant pool:
     #
     #        The <entries> is a list of {<cu_ref> <kind> <scope>} tuples
@@ -3862,7 +3862,17 @@ namespace eval Dwarf {
     #        encoded for testing GDB's error handling.  The value can be a
     #        raw number, or the keyword 'end' which resolves to the exact
     #        size of the constant pool (i.e. one byte past its end).  The
-    #        string <name> is still emitted into the constant pool.
+    #        string <name> is still emitted into the constant pool.  Pass
+    #        an empty string to keep the automatically computed offset.
+    #
+    #        Similarly, the optional <cuvec_offset> overrides the CU vector
+    #        offset written into this symbol's slot in the symbol table.
+    #        Normally this offset is computed to point at this symbol's CU
+    #        vector in the constant pool but an override allows an out of
+    #        range offset to be encoded.  The value can be a raw number or
+    #        the keyword 'end' which has the same meaning as for
+    #        <name_offset> above.  The CU vector itself is still emitted
+    #        into the constant pool.
     #
     #   main <language> <name>
     #     -- add symbol <name> of <language> to the index as the entry point
@@ -3923,10 +3933,9 @@ namespace eval Dwarf {
 	set _gdb_index_cus []
 
 	# A list of lists.  Each sub-list has the form: {name entries
-	# offset}, where <name> is a string, entries is a list of CU
-	# descriptors, and <offset> is usually the empty string, but can be
-	# a number which overrides the offset from the CU vector to the
-	# <name> string.  See _gdb_index_name for more details.
+	# name_offset cuvec_offset}.  See the 'name' command (proc
+	# _gdb_index_name) in the gdb_index documentation for details of
+	# what each field means.
 	variable _gdb_index_names
 	set _gdb_index_names []
 
@@ -3969,9 +3978,11 @@ namespace eval Dwarf {
 	# offset, or it can be the string 'end', which resolves to the exact
 	# size of the constant pool (i.e. one byte past its end).  This is
 	# useful for testing for off-by-one errors in GDB.
-	proc _gdb_index_name { name entries_list {name_offset ""} } {
+	proc _gdb_index_name { name entries_list {name_offset ""} \
+			       {cuvec_offset ""} } {
 	    variable _gdb_index_names
-	    lappend _gdb_index_names [list $name $entries_list $name_offset]
+	    lappend _gdb_index_names \
+		[list $name $entries_list $name_offset $cuvec_offset]
 	}
 
 	# Setup the name and language of the main function, as recorded in
@@ -4184,9 +4195,23 @@ namespace eval Dwarf {
 		} else {
 		    _op .4byte "$name_offset" "name offset"
 		}
-		_op .4byte \
-		    "$vec_label - $gdb_index_constant_pool" \
-		    "CU vector offset"
+
+		# The CU vector offset normally points at this symbol's CU
+		# vector in the constant pool, but it too can be overridden
+		# (e.g. to an out of range value) for testing.  See the
+		# 'name' command in the gdb_index documentation for details.
+		set cuvec_offset [lindex [lindex $_gdb_index_names $idx] 3]
+		if { $cuvec_offset eq "" } {
+		    _op .4byte \
+			"$vec_label - $gdb_index_constant_pool" \
+			"CU vector offset"
+		} elseif { $cuvec_offset eq "end" } {
+		    _op .4byte \
+			"$gdb_index_constant_pool_end - $gdb_index_constant_pool" \
+			"CU vector offset"
+		} else {
+		    _op .4byte "$cuvec_offset" "CU vector offset"
+		}
 	    } else {
 		_op .4byte 0 "empty slot"
 		_op .4byte 0 "empty slot"
-- 
2.25.4