Re: [PATCH] readelf: Support --dynamic with --use-dynamic

[Mark Wielaard <mark@klomp.org>]

[-- Attachment #1: Type: text/plain, Size: 1939 bytes --]

Hi,

On Tue, May 24, 2022 at 11:53:11PM +0800, Di Chen via Elfutils-devel wrote:
> All the request changes are fixed, ready for review again.
> 
> 1. help message updated: "Use the dynamic segment when possible for
> displaying info"
> 2. move enum dyn_idx to a proper place
> 3. add strtab_data's NULL check in function: handle_dynamic()
> 4. add phdr's NULL check in function: print_dynamic()
> 5. add comments for function: find_offsets()
> 6. remove redundant return-statement in function: get_dynscn_addrs()
> 7. add run-readelf-Dd.sh to EXTRA_DISTS
> 8. check strsz in (dyn->d_un.d_ptr < strtab_data->d_size) in function:
> handle_dynamic()

Sorry the re-review took so long. This looks great. I did add a NEWS
entry and wrote a Changelog entry while re-reviewing. And a few small
whitespace fixups.

The only code change I made was:

-      char *lib_name = NULL;
-
-      if (!use_dynamic_segment)
-    lib_name = elf_strptr (ebl->elf, shdr->sh_link, dyn->d_un.d_val);
-      else if (use_dynamic_segment)
-    lib_name = ((char *)strtab_data->d_buf) + dyn->d_un.d_ptr;
-      else
-    break;
+      char *name = NULL;
+      if (dyn->d_tag == DT_NEEDED
+         || dyn->d_tag == DT_SONAME
+         || dyn->d_tag == DT_RPATH
+         || dyn->d_tag == DT_RUNPATH)
+       {
+         if (! use_dynamic_segment)
+           name = elf_strptr (ebl->elf, shdr->sh_link, dyn->d_un.d_val);
+         else if (dyn->d_un.d_ptr < strtab_data->d_size
+                  && memrchr (strtab_data->d_buf + strtab_data->d_size - 1, '\0',
+                              strtab_data->d_size - 1 - dyn->d_un.d_ptr) != NULL)
+           name = ((char *) strtab_data->d_buf) + dyn->d_un.d_ptr;
+       }

That does the check whether dyn->d_un.d_ptr is valid early, so it
doesn't need to be checked in each case statement. Also it adds an
extra memrchr check to make sure the string is zero terminated.

Pushed with those changes.

Thanks,

Mark


[-- Attachment #2: 0001-readelf-Support-dynamic-with-use-dynamic.patch --]
[-- Type: text/x-diff, Size: 15608 bytes --]

From 369c021c6eedae3665c1dbbaa4fc43afbbb698f4 Mon Sep 17 00:00:00 2001
From: Di Chen <dichen@redhat.com>
Date: Thu, 28 Apr 2022 19:55:33 +0800
Subject: [PATCH] readelf: Support --dynamic with --use-dynamic

Currently, eu-readelf is using section headers to dump the dynamic
segment information (print_dynamic -> handle_dynamic).

This patch adds new options to eu-readelf (-D, --use-dynamic)
for (-d, --dynamic).

https://sourceware.org/bugzilla/show_bug.cgi?id=28873

Signed-off-by: Di Chen <dichen@redhat.com>
---
 ChangeLog               |   4 +
 NEWS                    |   2 +
 src/ChangeLog           |  13 +++
 src/readelf.c           | 212 +++++++++++++++++++++++++++++++++++-----
 tests/ChangeLog         |   6 ++
 tests/Makefile.am       |   4 +-
 tests/run-readelf-Dd.sh |  66 +++++++++++++
 7 files changed, 280 insertions(+), 27 deletions(-)
 create mode 100755 tests/run-readelf-Dd.sh

diff --git a/ChangeLog b/ChangeLog
index 0ececcc9..5421f5b8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2022-04-28  Di Chen  <dichen@redhat.com>
+
+	* NEWS: Add readefl -D, --use-dynamic.
+
 2022-07-28  Di Chen  <dichen@redhat.com>
 
 	* NEWS: Add dwfl_frame_reg.
diff --git a/NEWS b/NEWS
index 82c86cb6..156f78df 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,7 @@
 Version 0.188 some time after 0.187
 
+readelf: Add -D, --use-dynamic option.
+
 debuginfod: Add --disable-source-scan option.
 
 libdwfl: Add new function dwfl_get_debuginfod_client.
diff --git a/src/ChangeLog b/src/ChangeLog
index 8c9f5ddd..db20a6ef 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,16 @@
+2022-04-28  Di Chen  <dichen@redhat.com>
+
+	* readelf.c (options): Add use-dynamic 'D'.
+	(use_dynamic_segment): New static bool.
+	(enum dyn_idx): New.
+	(get_dynscn_strtab): New function.
+	(get_dynscn_addrs): Likewise.
+	(find_offsets): Likewise.
+	(parse_opt): Handle 'D'.
+	(handle_dynamic): New argument phdr. Get data either through the shdr
+	or phdr.  Print segment info when use_dynamic_segment. Use
+	get_dynscn_strtab. Get library name and paths through strtab_data.
+
 2022-05-09  Mark Wielaard  <mark@klomp.org>
 
 	* strip.c (remove_debug_relocations): Check gelf_getshdr, gelf_getrela,
diff --git a/src/readelf.c b/src/readelf.c
index 4b6aab2b..f4d973da 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -137,6 +137,8 @@ static const struct argp_option options[] =
   { "string-dump", 'p', NULL, OPTION_ALIAS | OPTION_HIDDEN, NULL, 0 },
   { "archive-index", 'c', NULL, 0,
     N_("Display the symbol index of an archive"), 0 },
+  { "use-dynamic", 'D', NULL, 0,
+    N_("Use the dynamic segment when possible for displaying info"), 0 },
 
   { NULL, 0, NULL, 0, N_("Output control:"), 0 },
   { "numeric-addresses", 'N', NULL, 0,
@@ -195,6 +197,9 @@ static bool print_symbol_table;
 /* True if (only) the dynsym table should be printed.  */
 static bool print_dynsym_table;
 
+/* True if reconstruct dynamic symbol table from the PT_DYNAMIC segment.  */
+static bool use_dynamic_segment;
+
 /* A specific section name, or NULL to print all symbol tables.  */
 static char *symbol_table_section;
 
@@ -318,6 +323,24 @@ static void dump_strings (Ebl *ebl);
 static void print_strings (Ebl *ebl);
 static void dump_archive_index (Elf *, const char *);
 
+enum dyn_idx
+{
+  i_strsz,
+  i_verneed,
+  i_verdef,
+  i_versym,
+  i_symtab,
+  i_strtab,
+  i_hash,
+  i_gnu_hash,
+  i_max
+};
+
+/* Declarations of local functions for use-dynamic.  */
+static Elf_Data *get_dynscn_strtab (Elf *elf, GElf_Phdr *phdr);
+static void get_dynscn_addrs (Elf *elf, GElf_Phdr *phdr, GElf_Addr addrs[i_max]);
+static void find_offsets (Elf *elf, GElf_Addr main_bias, size_t n,
+			  GElf_Addr addrs[n], GElf_Off offs[n]);
 
 /* Looked up once with gettext in main.  */
 static char *yes_str;
@@ -429,6 +452,9 @@ parse_opt (int key, char *arg,
       print_dynamic_table = true;
       any_control_option = true;
       break;
+    case 'D':
+      use_dynamic_segment = true;
+      break;
     case 'e':
       print_debug_sections |= section_exception;
       any_control_option = true;
@@ -1791,7 +1817,7 @@ get_dyn_ents (Elf_Data * dyn_data)
 
 
 static void
-handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr)
+handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr, GElf_Phdr *phdr)
 {
   int class = gelf_getclass (ebl->elf);
   GElf_Shdr glink_mem;
@@ -1802,34 +1828,64 @@ handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr)
   size_t dyn_ents;
 
   /* Get the data of the section.  */
-  data = elf_getdata (scn, NULL);
+  if (use_dynamic_segment)
+    data = elf_getdata_rawchunk(ebl->elf, phdr->p_offset,
+				phdr->p_filesz, ELF_T_DYN);
+  else
+    data = elf_getdata (scn, NULL);
+
   if (data == NULL)
     return;
 
   /* Get the dynamic section entry number */
   dyn_ents = get_dyn_ents (data);
 
-  /* Get the section header string table index.  */
-  if (unlikely (elf_getshdrstrndx (ebl->elf, &shstrndx) < 0))
-    error_exit (0, _("cannot get section header string table index"));
+  if (!use_dynamic_segment)
+    {
+      /* Get the section header string table index.  */
+      if (unlikely (elf_getshdrstrndx (ebl->elf, &shstrndx) < 0))
+	error_exit (0, _("cannot get section header string table index"));
 
-  glink = gelf_getshdr (elf_getscn (ebl->elf, shdr->sh_link), &glink_mem);
-  if (glink == NULL)
-    error_exit (0, _("invalid sh_link value in section %zu"),
-		elf_ndxscn (scn));
+      glink = gelf_getshdr (elf_getscn (ebl->elf, shdr->sh_link), &glink_mem);
+      if (glink == NULL)
+	error_exit (0, _("invalid sh_link value in section %zu"),
+		    elf_ndxscn (scn));
 
-  printf (ngettext ("\
+      printf (ngettext ("\
 \nDynamic segment contains %lu entry:\n Addr: %#0*" PRIx64 "  Offset: %#08" PRIx64 "  Link to section: [%2u] '%s'\n",
 		    "\
 \nDynamic segment contains %lu entries:\n Addr: %#0*" PRIx64 "  Offset: %#08" PRIx64 "  Link to section: [%2u] '%s'\n",
-		    dyn_ents),
-	  (unsigned long int) dyn_ents,
-	  class == ELFCLASS32 ? 10 : 18, shdr->sh_addr,
-	  shdr->sh_offset,
-	  (int) shdr->sh_link,
-	  elf_strptr (ebl->elf, shstrndx, glink->sh_name));
+			dyn_ents),
+	      (unsigned long int) dyn_ents,
+	      class == ELFCLASS32 ? 10 : 18, shdr->sh_addr,
+	      shdr->sh_offset,
+	      (int) shdr->sh_link,
+	      elf_strptr (ebl->elf, shstrndx, glink->sh_name));
+    }
+  else
+    {
+      printf (ngettext ("\
+\nDynamic segment contains %lu entry:\n Addr: %#0*" PRIx64 "  Offset: %#08" PRIx64 "\n",
+		    "\
+\nDynamic segment contains %lu entries:\n Addr: %#0*" PRIx64 "  Offset: %#08" PRIx64 "\n",
+			dyn_ents),
+	      (unsigned long int) dyn_ents,
+	      class == ELFCLASS32 ? 10 : 18, phdr->p_paddr,
+	      phdr->p_offset);
+    }
+
   fputs_unlocked (_("  Type              Value\n"), stdout);
 
+  /* if --use-dynamic option is enabled,
+     use the string table to get the related library info.  */
+  Elf_Data *strtab_data = NULL;
+  if (use_dynamic_segment)
+    {
+      strtab_data = get_dynscn_strtab(ebl->elf, phdr);
+      if (strtab_data == NULL)
+	error_exit (0, _("cannot get string table by using dynamic segment"));
+    }
+
   for (cnt = 0; cnt < dyn_ents; ++cnt)
     {
       GElf_Dyn dynmem;
@@ -1841,6 +1897,20 @@ handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr)
       printf ("  %-17s ",
 	      ebl_dynamic_tag_name (ebl, dyn->d_tag, buf, sizeof (buf)));
 
+      char *name = NULL;
+      if (dyn->d_tag == DT_NEEDED
+	  || dyn->d_tag == DT_SONAME
+	  || dyn->d_tag == DT_RPATH
+	  || dyn->d_tag == DT_RUNPATH)
+	{
+	  if (! use_dynamic_segment)
+	    name = elf_strptr (ebl->elf, shdr->sh_link, dyn->d_un.d_val);
+	  else if (dyn->d_un.d_ptr < strtab_data->d_size
+		   && memrchr (strtab_data->d_buf + strtab_data->d_size - 1, '\0',
+			       strtab_data->d_size - 1 - dyn->d_un.d_ptr) != NULL)
+	    name = ((char *) strtab_data->d_buf) + dyn->d_un.d_ptr;
+	}
+
       switch (dyn->d_tag)
 	{
 	case DT_NULL:
@@ -1852,23 +1922,19 @@ handle_dynamic (Ebl *ebl, Elf_Scn *scn, GElf_Shdr *shdr)
 	  break;
 
 	case DT_NEEDED:
-	  printf (_("Shared library: [%s]\n"),
-		  elf_strptr (ebl->elf, shdr->sh_link, dyn->d_un.d_val));
+	  printf (_("Shared library: [%s]\n"), name);
 	  break;
 
 	case DT_SONAME:
-	  printf (_("Library soname: [%s]\n"),
-		  elf_strptr (ebl->elf, shdr->sh_link, dyn->d_un.d_val));
+	  printf (_("Library soname: [%s]\n"), name);
 	  break;
 
 	case DT_RPATH:
-	  printf (_("Library rpath: [%s]\n"),
-		  elf_strptr (ebl->elf, shdr->sh_link, dyn->d_un.d_val));
+	  printf (_("Library rpath: [%s]\n"), name);
 	  break;
 
 	case DT_RUNPATH:
-	  printf (_("Library runpath: [%s]\n"),
-		  elf_strptr (ebl->elf, shdr->sh_link, dyn->d_un.d_val));
+	  printf (_("Library runpath: [%s]\n"), name);
 	  break;
 
 	case DT_PLTRELSZ:
@@ -1942,8 +2008,9 @@ print_dynamic (Ebl *ebl)
 	  Elf_Scn *scn = gelf_offscn (ebl->elf, phdr->p_offset);
 	  GElf_Shdr shdr_mem;
 	  GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem);
-	  if (shdr != NULL && shdr->sh_type == SHT_DYNAMIC)
-	    handle_dynamic (ebl, scn, shdr);
+	  if ((use_dynamic_segment && phdr != NULL)
+	      || (shdr != NULL && shdr->sh_type == SHT_DYNAMIC))
+	    handle_dynamic (ebl, scn, shdr, phdr);
 	  break;
 	}
     }
@@ -4801,6 +4868,99 @@ print_ops (Dwfl_Module *dwflmod, Dwarf *dbg, int indent, int indentrest,
 }
 
 
+/* Turn the addresses into file offsets by using the phdrs.  */
+static void
+find_offsets(Elf *elf, GElf_Addr main_bias, size_t n,
+                  GElf_Addr addrs[n], GElf_Off offs[n])
+{
+  size_t unsolved = n;
+  for (size_t i = 0; i < phnum; ++i) {
+    GElf_Phdr phdr_mem;
+    GElf_Phdr *phdr = gelf_getphdr(elf, i, &phdr_mem);
+    if (phdr != NULL && phdr->p_type == PT_LOAD && phdr->p_memsz > 0)
+      for (size_t j = 0; j < n; ++j)
+        if (offs[j] == 0 && addrs[j] >= phdr->p_vaddr + main_bias &&
+            addrs[j] - (phdr->p_vaddr + main_bias) < phdr->p_filesz) {
+          offs[j] = addrs[j] - (phdr->p_vaddr + main_bias) + phdr->p_offset;
+          if (--unsolved == 0)
+            break;
+        }
+  }
+}
+
+/* The dynamic segment (type PT_DYNAMIC), contains the .dynamic section.
+   And .dynamic section contains an array of the dynamic structures.
+   We use the array to get:
+    DT_STRTAB: the address of the string table
+    DT_SYMTAB: the address of the symbol table
+    DT_STRSZ: the size, in bytes, of the string table
+    ...  */
+static void
+get_dynscn_addrs(Elf *elf, GElf_Phdr *phdr, GElf_Addr addrs[i_max])
+{
+  Elf_Data *data = elf_getdata_rawchunk(
+    elf, phdr->p_offset, phdr->p_filesz, ELF_T_DYN);
+
+  int dyn_idx = 0;
+  for (;; ++dyn_idx) {
+    GElf_Dyn dyn_mem;
+    GElf_Dyn *dyn = gelf_getdyn(data, dyn_idx, &dyn_mem);
+    /* DT_NULL Marks end of dynamic section.  */
+    if (dyn->d_tag == DT_NULL)
+      break;
+
+    switch (dyn->d_tag) {
+    case DT_SYMTAB:
+      addrs[i_symtab] = dyn->d_un.d_ptr;
+      break;
+
+    case DT_HASH:
+      addrs[i_hash] = dyn->d_un.d_ptr;
+      break;
+
+    case DT_GNU_HASH:
+      addrs[i_gnu_hash] = dyn->d_un.d_ptr;
+      break;
+
+    case DT_STRTAB:
+      addrs[i_strtab] = dyn->d_un.d_ptr;
+      break;
+
+    case DT_VERSYM:
+      addrs[i_versym] = dyn->d_un.d_ptr;
+      break;
+
+    case DT_VERDEF:
+      addrs[i_verdef] = dyn->d_un.d_ptr;
+      break;
+
+    case DT_VERNEED:
+      addrs[i_verneed] = dyn->d_un.d_ptr;
+      break;
+
+    case DT_STRSZ:
+      addrs[i_strsz] = dyn->d_un.d_val;
+      break;
+    }
+  }
+}
+
+
+/* Use dynamic segment to get data for the string table section.  */
+static Elf_Data *
+get_dynscn_strtab(Elf *elf, GElf_Phdr *phdr)
+{
+  Elf_Data *strtab_data;
+  GElf_Addr addrs[i_max] = {0,};
+  GElf_Off offs[i_max] = {0,};
+  get_dynscn_addrs(elf, phdr, addrs);
+  find_offsets(elf, 0, i_max, addrs, offs);
+  strtab_data = elf_getdata_rawchunk(
+          elf, offs[i_strtab], addrs[i_strsz], ELF_T_BYTE);
+  return strtab_data;
+}
+
+
 struct listptr
 {
   Dwarf_Off offset:(64 - 3);
diff --git a/tests/ChangeLog b/tests/ChangeLog
index e65ea09b..fb573d80 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,9 @@
+2022-04-28  Di Chen  <dichen@redhat.com>
+
+	* run-readelf-Dd.sh: New test.
+	* Makefile.am (TESTS): Add run-readelf-Dd.sh.
+	(EXTRA_DIST): Likewise.
+
 2022-06-01  Mark Wielaard  <mark@klomp.org>
 
 	* testfile-arm-flags.bz2: New test file.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 07851594..87988fb9 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -198,7 +198,8 @@ TESTS = run-arextract.sh run-arsymtest.sh run-ar.sh newfile test-nlist \
 	msg_tst system-elf-libelf-test \
 	$(asm_TESTS) run-disasm-bpf.sh run-low_high_pc-dw-form-indirect.sh \
 	run-nvidia-extended-linemap-libdw.sh run-nvidia-extended-linemap-readelf.sh \
-	run-readelf-dw-form-indirect.sh run-strip-largealign.sh
+	run-readelf-dw-form-indirect.sh run-strip-largealign.sh \
+	run-readelf-Dd.sh
 
 if !BIARCH
 export ELFUTILS_DISABLE_BIARCH = 1
@@ -381,6 +382,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
 	     testfile56.bz2 testfile57.bz2 testfile58.bz2 \
 	     run-typeiter.sh testfile59.bz2 \
 	     run-readelf-d.sh testlib_dynseg.so.bz2 \
+	     run-readelf-Dd.sh \
 	     testfile-s390x-hash-both.bz2 \
 	     run-readelf-gdb_index.sh testfilegdbindex5.bz2 \
 	     testfilegdbindex7.bz2 \
diff --git a/tests/run-readelf-Dd.sh b/tests/run-readelf-Dd.sh
new file mode 100755
index 00000000..8c699937
--- /dev/null
+++ b/tests/run-readelf-Dd.sh
@@ -0,0 +1,66 @@
+#! /bin/sh
+# Copyright (C) 2022 Red Hat, Inc.
+# This file is part of elfutils.
+#
+# This file 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.
+#
+# elfutils 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/>.
+
+. $srcdir/test-subr.sh
+
+# #include <stdio.h>
+#
+# __thread int i;
+#
+# void print_i ()
+# {
+#   printf("%d\n", i);
+# }
+#
+# gcc -fPIC -shared -o testlib_dynseg.so testlib_dynseg.c
+# With ld --version
+# GNU gold (GNU Binutils 2.22.52.20120402) 1.11
+
+# The same testfile is used in run-readelf-d.sh
+testfiles testlib_dynseg.so
+
+testrun_compare ${abs_top_builddir}/src/readelf -Dd testlib_dynseg.so <<\EOF
+
+Dynamic segment contains 23 entries:
+ Addr: 0x00000000000017e0  Offset: 0x0007e0
+  Type              Value
+  PLTGOT            0x00000000000019c8
+  PLTRELSZ          72 (bytes)
+  JMPREL            0x0000000000000568
+  PLTREL            RELA
+  RELA              0x00000000000004d8
+  RELASZ            144 (bytes)
+  RELAENT           24 (bytes)
+  RELACOUNT         1
+  SYMTAB            0x0000000000000228
+  SYMENT            24 (bytes)
+  STRTAB            0x0000000000000360
+  STRSZ             190 (bytes)
+  GNU_HASH          0x0000000000000420
+  NEEDED            Shared library: [libc.so.6]
+  NEEDED            Shared library: [ld-linux-x86-64.so.2]
+  INIT              0x00000000000005b0
+  FINI              0x0000000000000748
+  VERSYM            0x0000000000000460
+  VERDEF            0x000000000000047c
+  VERDEFNUM         1
+  VERNEED           0x0000000000000498
+  VERNEEDNUM        2
+  NULL              
+EOF
+
+exit 0
-- 
2.30.2