readelf: Save and dump the original section header values

Message ID CAMe9rOrBpgv3vd49oqG0SeJEQytMo=pQiWbnrg8SYJypJQuQvQ@mail.gmail.com
State New
Headers
Series readelf: Save and dump the original section header values |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

H.J. Lu July 3, 2026, 2:30 p.m. UTC
  validate_section_info clears the garbage values in the section header
to avoid crash later.  Save and dump the original section header values
to make the garbage values in the section header visible when dumping
section headers.

* readelf.c (filedata): Add orig_section_headers.
(validate_section_info): Add a pointer to the original section
header and save the original section header values.
(get_32bit_section_headers): Allocate the original section header
buffer.  Pass the original section header pointer to
validate_section_info.
(get_64bit_section_headers): Likewise.
(process_section_headers): Dump the original section headers.
(process_relocs): Pass a dummy original section pointer to
validate_section_info.
(free_filedata): Free filedata->orig_section_headers.
* testsuite/binutils-all/corrupt-1.elf.bz2: New file.
* testsuite/binutils-all/corrupt-1.r: Likewise.
* testsuite/binutils-all/readelf.exp: Run corrupt-1.elf test.
  

Comments

Alan Modra July 3, 2026, 11:49 p.m. UTC | #1
On Fri, Jul 03, 2026 at 10:30:14PM +0800, H.J. Lu wrote:
> validate_section_info clears the garbage values in the section header
> to avoid crash later.  Save and dump the original section header values
> to make the garbage values in the section header visible when dumping
> section headers.

I think this would be better done the other way around.  ie. have a
Elf_Internal_Shdr **sane_section_headers that is initialised to point
at entries in section_headers, with sane_section_headers[i] allocated
as necessary when needing to correct a bogus header.
  
H.J. Lu July 4, 2026, 12:39 a.m. UTC | #2
On Sat, Jul 4, 2026 at 7:49 AM Alan Modra <amodra@gmail.com> wrote:
>
> On Fri, Jul 03, 2026 at 10:30:14PM +0800, H.J. Lu wrote:
> > validate_section_info clears the garbage values in the section header
> > to avoid crash later.  Save and dump the original section header values
> > to make the garbage values in the section header visible when dumping
> > section headers.
>
> I think this would be better done the other way around.  ie. have a
> Elf_Internal_Shdr **sane_section_headers that is initialised to point
> at entries in section_headers, with sane_section_headers[i] allocated
> as necessary when needing to correct a bogus header.
>

I have thought about something similar and decided against it
since filedata->section_headers is used in many places.   However,
I can change filedata->orig_section_headers to on demand.
  

Patch

From aa021d0f19e7a871aa3d976587697f8b12a073bd Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 2 Jul 2026 10:14:10 +0800
Subject: [PATCH] readelf: Save and dump the original section header values

validate_section_info clears the garbage values in the section header
to avoid crash later.  Save and dump the original section header values
to make the garbage values in the section header visible when dumping
section headers.

	* readelf.c (filedata): Add orig_section_headers.
	(validate_section_info): Add a pointer to the original section
	header and save the original section header values.
	(get_32bit_section_headers): Allocate the original section header
	buffer.  Pass the original section header pointer to
	validate_section_info.
	(get_64bit_section_headers): Likewise.
	(process_section_headers): Dump the original section headers.
	(process_relocs): Pass a dummy original section pointer to
	validate_section_info.
	(free_filedata): Free filedata->orig_section_headers.
	* testsuite/binutils-all/corrupt-1.elf.bz2: New file.
	* testsuite/binutils-all/corrupt-1.r: Likewise.
	* testsuite/binutils-all/readelf.exp: Run corrupt-1.elf test.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 binutils/readelf.c                            |  54 +++++++++++++++---
 .../testsuite/binutils-all/corrupt-1.elf.bz2  | Bin 0 -> 81 bytes
 binutils/testsuite/binutils-all/corrupt-1.r   |   3 +
 binutils/testsuite/binutils-all/readelf.exp   |  12 ++++
 4 files changed, 61 insertions(+), 8 deletions(-)
 create mode 100644 binutils/testsuite/binutils-all/corrupt-1.elf.bz2
 create mode 100644 binutils/testsuite/binutils-all/corrupt-1.r

diff --git a/binutils/readelf.c b/binutils/readelf.c
index 2826e0cf195..ea679b3b4f4 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -282,6 +282,7 @@  typedef struct filedata
   uint64_t             archive_file_size;
   /* Everything below this point is cleared out by free_filedata.  */
   Elf_Internal_Shdr *  section_headers;
+  Elf_Internal_Shdr *  orig_section_headers;
   Elf_Internal_Phdr *  program_headers;
   char *               string_table;
   uint64_t             string_table_length;
@@ -7851,12 +7852,16 @@  offset_from_vma (Filedata * filedata, uint64_t vma, uint64_t size)
 /* Valid section info and clear the invalid fields.  */
 
 static void
-validate_section_info (Elf_Internal_Shdr *internal, unsigned int i,
+validate_section_info (Elf_Internal_Shdr *internal,
+		       Elf_Internal_Shdr *orig_internal, unsigned int i,
 		       Filedata *filedata, bool dynamic, bool probe)
 {
   const char *dynamic_tag = NULL;
   const char *dynamicsz_tag = NULL;
   const char *dynamicent_tag = NULL;
+  /* Save the original section header values before garbage values are
+     cleared.  */
+  *orig_internal = *internal;
   if (probe)
     return;
 
@@ -7944,6 +7949,7 @@  get_32bit_section_headers (Filedata * filedata, bool probe)
 {
   Elf32_External_Shdr * shdrs;
   Elf_Internal_Shdr *   internal;
+  Elf_Internal_Shdr *   orig_internal;
   unsigned int          i;
   unsigned int          size = filedata->file_header.e_shentsize;
   unsigned int          num = probe ? 1 : filedata->file_header.e_shnum;
@@ -7983,9 +7989,22 @@  get_32bit_section_headers (Filedata * filedata, bool probe)
       return false;
     }
 
+  filedata->orig_section_headers = (Elf_Internal_Shdr *)
+    cmalloc (num, sizeof (Elf_Internal_Shdr));
+  if (filedata->orig_section_headers == NULL)
+    {
+      if (!probe)
+	error (_("Out of memory reading %u section headers\n"), num);
+      free (shdrs);
+      free (filedata->section_headers);
+      filedata->section_headers = NULL;
+      return false;
+    }
+
+  orig_internal = filedata->orig_section_headers;
   for (i = 0, internal = filedata->section_headers;
        i < num;
-       i++, internal++)
+       i++, internal++, orig_internal++)
     {
       internal->sh_name      = BYTE_GET (shdrs[i].sh_name);
       internal->sh_type      = BYTE_GET (shdrs[i].sh_type);
@@ -7997,7 +8016,8 @@  get_32bit_section_headers (Filedata * filedata, bool probe)
       internal->sh_info      = BYTE_GET (shdrs[i].sh_info);
       internal->sh_addralign = BYTE_GET (shdrs[i].sh_addralign);
       internal->sh_entsize   = BYTE_GET (shdrs[i].sh_entsize);
-      validate_section_info (internal, i, filedata, false, probe);
+      validate_section_info (internal, orig_internal, i, filedata,
+			     false, probe);
     }
 
   free (shdrs);
@@ -8011,6 +8031,7 @@  get_64bit_section_headers (Filedata * filedata, bool probe)
 {
   Elf64_External_Shdr *  shdrs;
   Elf_Internal_Shdr *    internal;
+  Elf_Internal_Shdr *    orig_internal;
   unsigned int           i;
   unsigned int           size = filedata->file_header.e_shentsize;
   unsigned int           num = probe ? 1 : filedata->file_header.e_shnum;
@@ -8052,9 +8073,22 @@  get_64bit_section_headers (Filedata * filedata, bool probe)
       return false;
     }
 
+  filedata->orig_section_headers = (Elf_Internal_Shdr *)
+    cmalloc (num, sizeof (Elf_Internal_Shdr));
+  if (filedata->orig_section_headers == NULL)
+    {
+      if (!probe)
+	error (_("Out of memory reading %u section headers\n"), num);
+      free (shdrs);
+      free (filedata->section_headers);
+      filedata->section_headers = NULL;
+      return false;
+    }
+
+  orig_internal = filedata->orig_section_headers;
   for (i = 0, internal = filedata->section_headers;
        i < num;
-       i++, internal++)
+       i++, internal++, orig_internal++)
     {
       internal->sh_name      = BYTE_GET (shdrs[i].sh_name);
       internal->sh_type      = BYTE_GET (shdrs[i].sh_type);
@@ -8066,7 +8100,8 @@  get_64bit_section_headers (Filedata * filedata, bool probe)
       internal->sh_info      = BYTE_GET (shdrs[i].sh_info);
       internal->sh_offset    = BYTE_GET (shdrs[i].sh_offset);
       internal->sh_addralign = BYTE_GET (shdrs[i].sh_addralign);
-      validate_section_info (internal, i, filedata, false, probe);
+      validate_section_info (internal, orig_internal, i, filedata,
+			     false, probe);
     }
 
   free (shdrs);
@@ -9016,7 +9051,8 @@  process_section_headers (Filedata * filedata)
   if (do_section_details)
     printf (_("       Flags\n"));
 
-  for (i = 0, section = filedata->section_headers;
+  /* Dump the original section headers.  */
+  for (i = 0, section = filedata->orig_section_headers;
        i < filedata->file_header.e_shnum;
        i++, section++)
     {
@@ -10197,13 +10233,14 @@  process_relocs (Filedata * filedata)
 		  uint64_t num_reloc;
 		  uint64_t *relrs = NULL;
 		  Elf_Internal_Shdr section = {};
+		  Elf_Internal_Shdr orig_section;
 		  section.sh_offset
 		    = filedata->dynamic_info[DT_RELR];
 		  section.sh_size = rel_size;
 		  section.sh_entsize = rel_entsz;
 		  section.sh_type = SHT_RELR;
-		  validate_section_info (&section, DT_RELR, filedata,
-					 true, false);
+		  validate_section_info (&section, &orig_section, DT_RELR,
+					 filedata, true, false);
 		  num_reloc = count_relr_relocations (filedata,
 						      &section,
 						      &relrs);
@@ -24847,6 +24884,7 @@  free_filedata (Filedata *filedata)
   free (filedata->program_interpreter);
   free (filedata->program_headers);
   free (filedata->section_headers);
+  free (filedata->orig_section_headers);
   free (filedata->string_table);
   free (filedata->dump.dump_sects);
   free (filedata->dynamic_strings);
diff --git a/binutils/testsuite/binutils-all/corrupt-1.elf.bz2 b/binutils/testsuite/binutils-all/corrupt-1.elf.bz2
new file mode 100644
index 0000000000000000000000000000000000000000..e9c9f44343a4aadd90457df5e49923830881d194
GIT binary patch
literal 81
zcmV-X0IvT+T4*^jL0KkKS<!e4e*gd?Wun|A06@b80RR91fItKQfCwM}Ata_F&>`e0
nlM^7w$`z$$Gahz<VUjiOf0D@DL^^qw1A^{IrwS4pF9C1h02mvG

literal 0
HcmV?d00001

diff --git a/binutils/testsuite/binutils-all/corrupt-1.r b/binutils/testsuite/binutils-all/corrupt-1.r
new file mode 100644
index 00000000000..ae6438703e2
--- /dev/null
+++ b/binutils/testsuite/binutils-all/corrupt-1.r
@@ -0,0 +1,3 @@ 
+#...
+  \[10\] <no-strings>      REL             0000000000000000 000000 6666666666666600 00      0   0  0
+#pass
diff --git a/binutils/testsuite/binutils-all/readelf.exp b/binutils/testsuite/binutils-all/readelf.exp
index 0f3d75090b2..6c5e63fb9e8 100644
--- a/binutils/testsuite/binutils-all/readelf.exp
+++ b/binutils/testsuite/binutils-all/readelf.exp
@@ -612,6 +612,18 @@  if ![is_remote host] {
     } else {
 	readelf_test {-wi} $tempfile pr26160.r
     }
+
+    set test $srcdir/$subdir/corrupt-1.elf.bz2
+    # We need to strip the ".bz2", but can leave the dirname.
+    set t $subdir/[file tail $test]
+    set testname [file rootname $t]
+    verbose $testname
+    set tempfile tmpdir/corrupt-1.elf
+    if {[catch "system \"bzip2 -dc $test > $tempfile\""] != 0} {
+	untested "bzip2 -dc ($testname)"
+    } else {
+	readelf_test {-SW} $tempfile corrupt-1.r
+    }
 }
 
 # Check dwarf-5 support for DW_OP_addrx.
-- 
2.54.0