Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCH 5/6] gdb: handle corrupted .gdb_index header offsets
Date: Sun, 18 Oct 2026 21:50:34 +0100	[thread overview]
Message-ID: <b222c954bf141b5c8056e79a087abd60090423f5.1792356284.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1792356284.git.aburgess@redhat.com>

The .gdb_index section has a header.  The header is a list of offsets
into the .gdb_index section.  Currently GDB has basically no error
checking of these offsets, the offsets can point outside the
.gdb_index section and GDB will usually try to do something bad as a
result.

Most often a bad offset will result in GDB trying to allocate a huge
amount of memory.  Indeed, I don't recommend running these new tests
on an unpatched GDB; I managed to bring down my machine due to
excessive memory allocation.

I've added a mechanism to the gdb_index proc (in lib/dwarf.exp) to
allow the various .gdb_index header offsets to be overridden.  Then I
made use of this in gdb.dwarf2/gdb-index-corrupted.exp to override
each offset in turn.

I also added a new test gdb.dwarf2/gdb-index-tiny-header.exp which
tests with a .gdb_index sized between 0 and 28 bytes (inclusive).
This covers indexes that are too small to even hold a valid version
number field, and indexes upto the size of the full header block.  All
of these indexes are too small to hold useful data so GDB should
reject them all (without crashing).

The changes in gdb/dwarf2/read-gdb-index.c are fairly straightforward;
check the .gdb_index is big enough for the version field, then use the
version field to check that we have all of the offsets, and that the
offsets point within the .gdb_index section.

Also, as the data areas that the .gdb_index header offsets point to
are arranged in offset order, I also check that offsets are always
equal or increasing in value.
---
 gdb/dwarf2/read-gdb-index.c                   |  59 +++++++++
 .../gdb.dwarf2/gdb-index-corrupted.exp        |  59 +++++++++
 .../gdb.dwarf2/gdb-index-tiny-header.S        |  49 +++++++
 .../gdb.dwarf2/gdb-index-tiny-header.exp      | 121 ++++++++++++++++++
 gdb/testsuite/lib/dwarf.exp                   |  79 ++++++++++--
 5 files changed, 355 insertions(+), 12 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/gdb-index-tiny-header.S
 create mode 100644 gdb/testsuite/gdb.dwarf2/gdb-index-tiny-header.exp

diff --git a/gdb/dwarf2/read-gdb-index.c b/gdb/dwarf2/read-gdb-index.c
index 83e628d3afb..5959b0ba6bc 100644
--- a/gdb/dwarf2/read-gdb-index.c
+++ b/gdb/dwarf2/read-gdb-index.c
@@ -481,6 +481,22 @@ read_gdb_index_from_buffer (const char *filename,
   const gdb_byte *addr = &buffer[0];
   offset_view metadata (buffer);
 
+  /* Emit a warning that the .gdb_index section is too small.  */
+  auto invalid_index_size_warning = [&] ()
+  {
+    warning (_(".gdb_index section in %ps is too small (%s bytes),"
+	       " ignoring .gdb_index"),
+	     styled_string (file_name_style.style (), filename),
+	     pulongest (buffer.size ()));
+    return false;
+  };
+
+  /* Ensure that there is at least a version number available for reading.
+     We cannot do a minimum buffer size check until we know what version of
+     index we have, so first just check we can read that safely.  */
+  if (buffer.size () < sizeof (offset_type))
+    return invalid_index_size_warning ();
+
   /* Version check.  */
   offset_type version = metadata[0];
   /* GDB now requires the symbol attributes, which were added in
@@ -513,6 +529,49 @@ read_gdb_index_from_buffer (const char *filename,
 
   map->version = version;
 
+  /* The .gdb_index header is a version number followed by a series of
+     offsets into the .gdb_index section.  Each header offset points to a
+     data block, and the offset order, and the data block order are the
+     same.  This means that each offset must be greater than, or equal to
+     the previous.  The first offset should point to the first byte after
+     the .gdb_index header.  None of the header offsets should point outside
+     of the .gdb_index section.
+
+     If any of these requirements are not met then GDB can end either
+     crashing by accessing invalid memory, or trying to allocate a huge
+     amount of memory to hold an incorrectly sized data block.
+
+     So, before we make use of the header, let's sanity check it.
+
+     First, how many offsets do we expect in the header?  Version 9 added
+     the shortcut table offset.  */
+  const size_t offset_count = version >= 9 ? 6 : 5;
+
+  /* Ensure the buffer is large enough to contain the whole header (the
+     version word plus OFFSET_COUNT offset words) before reading any
+     offsets.  */
+  if (metadata.size () < offset_count + 1)
+    return invalid_index_size_warning ();
+
+  /* Each offset must not be smaller than the end of the header, no larger
+     than the whole .gdb_index section, and no smaller than the preceding
+     offset.  Initially set PREV to the header size to enforce the lower
+     bound on the first offset.  */
+  size_t prev = (offset_count + 1) * sizeof (offset_type);
+  for (size_t idx = 1; idx <= offset_count; ++idx)
+    {
+      size_t off = metadata[idx];
+      if (off < prev || off > buffer.size ())
+	{
+	  warning (_("skipping .gdb_index section in %ps, an invalid offset (%s)"
+		     " was found in the header"),
+		   styled_string (file_name_style.style (), filename),
+		   hex_string (off));
+	  return false;
+	}
+      prev = off;
+    }
+
   int i = 1;
   *cu_list = addr + metadata[i];
   *cu_list_elements = (metadata[i + 1] - metadata[i]) / 8;
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
index 58ac13ce2ee..865e5e50f97 100644
--- a/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
+++ b/gdb/testsuite/gdb.dwarf2/gdb-index-corrupted.exp
@@ -276,6 +276,48 @@ proc run_test { testname } {
 		addr $_start_start "$_start_end - 2" cu_label
 		addr "$_start_end - 4" $_start_end cu_label
 	    }
+	} elseif { $testname eq "header-cu-list-invalid" } {
+	    # Create an index with an out-of-bounds CU list offset.
+	    # GDB should detect this and reject the index.
+	    gdb_index { header_override {cu_list 0xffffffff} } {
+		cu cu_label
+		name _start {{cu_label function global}}
+	    }
+	} elseif { $testname eq "header-tu-list-invalid" } {
+	    # Create an index with an out-of-bounds TU list offset.
+	    # GDB should detect this and reject the index.
+	    gdb_index { header_override {tu_list 0xffffffff} } {
+		cu cu_label
+		name _start {{cu_label function global}}
+	    }
+	} elseif { $testname eq "header-address-table-invalid" } {
+	    # Create an index with an out-of-bounds address table offset.
+	    # GDB should detect this and reject the index.
+	    gdb_index { header_override {address_table 0xffffffff} } {
+		cu cu_label
+		name _start {{cu_label function global}}
+	    }
+	} elseif { $testname eq "header-symbol-table-invalid" } {
+	    # Create an index with an out-of-bounds symbol table offset.
+	    # GDB should detect this and reject the index.
+	    gdb_index { header_override {symbol_table 0xffffffff} } {
+		cu cu_label
+		name _start {{cu_label function global}}
+	    }
+	} elseif { $testname eq "header-shortcut-invalid" } {
+	    # Create an index with an out-of-bounds shortcut table offset.
+	    # GDB should detect this and reject the index.
+	    gdb_index { header_override {shortcut 0xffffffff} } {
+		cu cu_label
+		name _start {{cu_label function global}}
+	    }
+	} elseif { $testname eq "header-constant-pool-invalid" } {
+	    # Create an index with an out-of-bounds constant pool offset.
+	    # GDB should detect this and reject the index.
+	    gdb_index { header_override {constant_pool 0xffffffff} } {
+		cu cu_label
+		name _start {{cu_label function global}}
+	    }
 	} elseif { $testname eq "valid-index" } {
 	    # Generate a well-formed index that includes a valid main
 	    # function (valid language, with the name pointing at the
@@ -367,6 +409,17 @@ proc run_test { testname } {
 	    set expect_warning_re ""
 	}
 	set expect_addr_map {}
+    } elseif { $testname eq "header-cu-list-invalid" \
+		   || $testname eq "header-tu-list-invalid" \
+		   || $testname eq "header-address-table-invalid" \
+		   || $testname eq "header-symbol-table-invalid" \
+		   || $testname eq "header-shortcut-invalid" \
+		   || $testname eq "header-constant-pool-invalid" } {
+	# Each of these tests puts an out-of-range offset in one field of
+	# the index header.  GDB should detect the invalid offset, emit a
+	# warning, and reject the index.
+	set expect_gdb_index_ignored true
+	set expect_warning_re "\r\nwarning: skipping \\.gdb_index section in \[^\r\n\]*, an invalid offset \\($::hex\\) was found in the header\r\n"
     } elseif { $testname eq "addr-valid" } {
 	set expect_start true
 	set expect_method false
@@ -538,6 +591,12 @@ set tests { empty-component \
 		addr-swapped \
 		addr-overlap \
 		addr-valid \
+		header-cu-list-invalid \
+		header-tu-list-invalid \
+		header-address-table-invalid \
+		header-symbol-table-invalid \
+		header-shortcut-invalid \
+		header-constant-pool-invalid \
 		valid-index }
 
 foreach_with_prefix test $tests {
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-index-tiny-header.S b/gdb/testsuite/gdb.dwarf2/gdb-index-tiny-header.S
new file mode 100644
index 00000000000..3df0fa666f9
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/gdb-index-tiny-header.S
@@ -0,0 +1,49 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2026 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Create a .gdb_index section that is too small to hold a valid header.
+   The size of the section, in bytes, is given by the INDEX_SIZE macro
+   which the exp script sets when this file is assembled.
+
+   A valid version 9 header is 28 bytes: a 4-byte version word followed by
+   six 4-byte offsets.  Any section smaller than this is invalid and GDB
+   should reject it rather than reading beyond the end of the section.
+
+   The .gdb_index format is always little-endian, so we build the section
+   byte by byte rather than with directives like .4byte, whose encoding
+   would depend on the host's endianness.  The first byte is 0x09, the low
+   byte of a little-endian version 9 word; any remaining bytes are zero.
+   For sizes of 4 or more this yields a valid version word (0x09 0x00 0x00
+   0x00) so that GDB gets past its version check and on to the header size
+   check, with the trailing zeros standing in for a truncated set of header
+   offsets.  For sizes below 4 there is not even room for the full version
+   word.  */
+
+	.section .gdb_index
+#if INDEX_SIZE > 0
+	/* Low byte of a little-endian version 9 word.  */
+	.byte 0x09
+# if INDEX_SIZE > 1
+	/* Rest of the truncated header.  GDB checks the header size
+	   before checking that the header contents are valid, so
+	   filling with zeros, which is invalid, doesn't really
+	   matter.  */
+	.fill (INDEX_SIZE - 1), 1, 0
+# endif
+#endif
+
+	.section	.note.GNU-stack,"",%progbits
diff --git a/gdb/testsuite/gdb.dwarf2/gdb-index-tiny-header.exp b/gdb/testsuite/gdb.dwarf2/gdb-index-tiny-header.exp
new file mode 100644
index 00000000000..a99aa3b69d7
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/gdb-index-tiny-header.exp
@@ -0,0 +1,121 @@
+# Copyright 2026 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that GDB gracefully handles a .gdb_index section that is too small
+# to contain a valid header.  A valid version 9 header is 28 bytes, a 4-byte
+# version word followed by six 4-byte offsets, so the section is built at
+# every size from 0 up to 28 bytes in turn.  Each of these should cause GDB
+# to ignore the index and fall back to building a cooked index from the
+# DWARF, rather than reading beyond the end of the index section.  The size
+# 28 case is interesting as this is big enough to hold a valid header, but
+# every offset in the header is 0, which is invalid, so GDB will reject the
+# index for that reason.
+#
+# The sizes split into a few interesting ranges:
+#   0 bytes       -- an empty section, silently ignored.
+#   1 to 3 bytes  -- too small even for the version word.
+#   4 to 27 bytes -- a valid version word, but a truncated set of header
+#                    offsets.
+#   28 bytes      -- a full set of header offsets, but they are all zero,
+#                    which is invalid.
+
+load_lib dwarf.exp
+
+require dwarf2_support
+
+# This test installs its own hand-crafted .gdb_index into the executable.
+# The 'readnow' board forces GDB to ignore the index and read the full debug
+# info, making this test irrelevant.
+require !readnow
+
+standard_testfile main.c gdb-index-tiny-header.S
+
+# This test relies on adding a .gdb_index.  If we are using a board that
+# adds an index at compile time then this test is not going to work.  So
+# build an executable at this point and check if it has an index already, if
+# it does, then we cannot carry on.
+if {[prepare_for_testing "failed to prepare" ${testfile} $srcfile]} {
+    return
+}
+
+set index [have_index $binfile]
+if { ![string eq $index ""] } {
+    unsupported "cache created at compile time"
+    return
+}
+
+# Build and test one variant of the tiny .gdb_index section.  SIZE is the
+# number of bytes in the generated section, it is passed to the assembler
+# source via the INDEX_SIZE macro.
+proc run_test { size } {
+    set exe "${::binfile}-${size}"
+
+    if {[build_executable "failed to build" $exe \
+	     [list $::srcfile $::srcfile2] \
+	     [list debug additional_flags=-DINDEX_SIZE=$size]] == -1} {
+	return
+    }
+
+    clean_restart
+
+    # Read the index synchronously as part of loading the file, so that any
+    # problem shows up during the 'file' command below.
+    gdb_test_no_output "maint set dwarf synchronous on"
+
+    # Loading the executable.  gdb_load returns -1 if GDB reports an
+    # internal error.
+    set res [gdb_load $exe]
+    gdb_assert { $res == 0 } "load executable"
+    if { $res != 0 } {
+	return
+    }
+
+    # A non-empty but too-small section should be rejected with a warning.
+    # A zero byte index section is empty and is silently ignored.
+    if { $size == 28 } {
+	set warning_re \
+	    [string cat \
+		 "\r\nwarning: skipping \\.gdb_index section in \[^\r\n\]+, an invalid offset \\(0x0\\) was found in the header\r\n"]
+	gdb_assert { [regexp $warning_re $::gdb_file_cmd_msg] } \
+	    "index rejected due to invalid offset"
+    } elseif { $size > 0 } {
+	set warning_re \
+	    [string cat \
+		 "\r\nwarning: \\.gdb_index section in \[^\r\n\]* is too " \
+		 "small \\($::decimal bytes\\), ignoring \\.gdb_index\r\n"]
+	gdb_assert { [regexp $warning_re $::gdb_file_cmd_msg] } \
+	    "index rejected due to header size"
+    } else {
+	# The SIZE is 0, there should be no warning in this case.
+	gdb_assert { ![regexp "warning: " $::gdb_file_cmd_msg] } \
+	    "no warning seen"
+    }
+
+    # The tiny .gdb_index is invalid, so GDB should ignore it and use a
+    # cooked index built from the DWARF instead.
+    set index [get_index_type [file tail $exe]]
+    gdb_assert { [string equal $index "cooked"] } ".gdb_index ignored"
+
+    # Expanding all symtabs should not crash GDB.
+    gdb_test_no_output "maint expand-symtabs" "expand all symtabs"
+}
+
+# Test every size from an empty section up to exactly the length of a valid
+# version 9 header (28 bytes).
+for { set s 0 } { $s <= 28 } { incr s } {
+    with_test_prefix "size $s" {
+	run_test $s
+    }
+}
diff --git a/gdb/testsuite/lib/dwarf.exp b/gdb/testsuite/lib/dwarf.exp
index 25b88b226e0..b2b23167671 100644
--- a/gdb/testsuite/lib/dwarf.exp
+++ b/gdb/testsuite/lib/dwarf.exp
@@ -3822,6 +3822,19 @@ namespace eval Dwarf {
     #               error handling.  The supported part names are:
     #               'constant_pool'.
     #               (default {} -- no .gdb_index parts are skipped)
+    #   header_override <dict>
+    #             - a dict specifying overrides for header fields.  Each
+    #               key is a header field name (cu_list, tu_list,
+    #               address_table, symbol_table, shortcut, constant_pool)
+    #               and the value is either:
+    #                 - a number (hex or decimal) to replace the field
+    #                 - the string 'skip' to omit the field from the header
+    #               Any field not mentioned uses its computed value.
+    #               Examples:
+    #                 {address_table 0xffffffff}
+    #                 {address_table 0x100 symbol_table 0x200}
+    #                 {shortcut skip constant_pool skip}
+    #               (default {} -- no overrides)
     #
     # BODY is Tcl code that emits the parts which make up the body of
     # the .gdb_index section.  It is evaluated in the caller's
@@ -3936,6 +3949,7 @@ namespace eval Dwarf {
 	    { version 9 }
 	    { no_null false }
 	    { skip_index_part {} }
+	    { header_override {} }
 	}
 
 	# Validate the names passed to skip_index_part to catch typos in
@@ -3946,6 +3960,16 @@ namespace eval Dwarf {
 	    }
 	}
 
+	# Validate the names passed to header_override to catch typos in
+	# tests.
+	foreach header [dict keys $header_override] {
+	    if { [lsearch -exact { cu_list tu_list address_table symbol_table \
+				       shortcut constant_pool } \
+		      $header] == -1 } {
+		error "gdb_index: unknown header_override '$header'"
+	    }
+	}
+
 	# A list of two entry lists describing CUs.  Each sub-list has the
 	# form {label length}.  The <label> is a CU label, created when the
 	# CU itself was created.  The <length> is optional, and will usually
@@ -4182,22 +4206,53 @@ namespace eval Dwarf {
 
 	gdb_index_start:
 
+	# Helper proc to emit a header field with optional override.
+	proc _emit_header_field { field_name label computed_value } {
+	    upvar header_override header_override
+
+	    if { [dict exists $header_override $field_name] } {
+		set override [dict get $header_override $field_name]
+		if { $override eq "skip" } {
+		    # Don't emit this field
+		    return
+		} else {
+		    # Emit the override value
+		    _op .4byte $override "$label (override)"
+		}
+	    } else {
+		# Emit the computed value
+		_op .4byte $computed_value $label
+	    }
+	}
+
 	# Header.
 	_op .4byte $version "Version"
-	_op .4byte "$gdb_index_cu_list - $gdb_index_start" \
-	    "CU list offset"
-	_op .4byte "$gdb_index_tu_list - $gdb_index_start" \
-	    "TU list offset"
-	_op .4byte "$gdb_index_addr_table - $gdb_index_start" \
-	    "Address table offset"
-	_op .4byte "$gdb_index_symtab - $gdb_index_start" \
-	    "Symbol table offset"
+
+	# CU list offset.
+	_emit_header_field cu_list "CU list offset" \
+	    "$gdb_index_cu_list - $gdb_index_start"
+
+	# TU list offset.
+	_emit_header_field tu_list "TU list offset" \
+	    "$gdb_index_tu_list - $gdb_index_start"
+
+	# Address table offset.
+	_emit_header_field address_table "Address table offset" \
+	    "$gdb_index_addr_table - $gdb_index_start"
+
+	# Symbol table offset.
+	_emit_header_field symbol_table "Symbol table offset" \
+	    "$gdb_index_symtab - $gdb_index_start"
+
+	# Shortcut table offset (version 9+).
 	if { $version >= 9 } {
-	    _op .4byte "$gdb_index_shortcut - $gdb_index_start" \
-		"Shortcut table offset"
+	    _emit_header_field shortcut "Shortcut table offset" \
+		"$gdb_index_shortcut - $gdb_index_start"
 	}
-	_op .4byte "$gdb_index_constant_pool - $gdb_index_start" \
-	    "Constant pool offset"
+
+	# Constant pool offset.
+	_emit_header_field constant_pool "Constant pool offset" \
+	    "$gdb_index_constant_pool - $gdb_index_start"
 
 	# CU list.
 	gdb_index_cu_list:
-- 
2.25.4


  parent reply	other threads:[~2026-10-18 20:52 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-18 20:50 [PATCH 0/6] Improved error handling for .gdb_index parsing Andrew Burgess
2026-10-18 20:50 ` [PATCH 1/6] gdb: a number of fixes " Andrew Burgess
2026-10-20 18:39   ` Simon Marchi
2026-10-18 20:50 ` [PATCH 2/6] gdb: handle incorrect CU size in .gdb_index Andrew Burgess
2026-10-20 18:50   ` Simon Marchi
2026-10-18 20:50 ` [PATCH 3/6] gdb: validate the CU vector offset when loading the .gdb_index Andrew Burgess
2026-10-20 19:00   ` Simon Marchi
2026-10-18 20:50 ` [PATCH 4/6] gdb: address table parsing fixes and tests for .gdb_index Andrew Burgess
2026-10-20 19:09   ` Simon Marchi
2026-10-18 20:50 ` Andrew Burgess [this message]
2026-10-20 19:23   ` [PATCH 5/6] gdb: handle corrupted .gdb_index header offsets Simon Marchi
2026-10-18 20:50 ` [PATCH 6/6] gdb: extend .gdb_index tests to skip all data blocks Andrew Burgess
2026-10-20 19:32   ` Simon Marchi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b222c954bf141b5c8056e79a087abd60090423f5.1792356284.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox