This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] libelf: elf_strptr should use datalist when data has been added to section.


This oneliner patch relies on the "libelf: Fix elf_newdata when raw ELF
file/image data is available." fix.

elf_strptr always used the rawdata when available. But when data has been
added to the section it should find the correct buffer in the datalist.

Adds a large testcase that checks various ways of adding and extracting
strings from a section.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 libelf/ChangeLog    |   5 +
 libelf/elf_strptr.c |  18 ++-
 tests/ChangeLog     |   7 +
 tests/Makefile.am   |   5 +-
 tests/elfstrtab.c   | 396 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 423 insertions(+), 8 deletions(-)
 create mode 100644 tests/elfstrtab.c

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index c43a7fe..8bf1256 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,8 @@
+2015-01-21  Mark Wielaard  <mjw@redhat.com>
+
+	* elf_strptr.c (elf_strptr): Check data_list_rear == NULL instead
+	of rawdata_base != NULL before using rawdata directly.
+
 2015-01-20  Mark Wielaard  <mjw@redhat.com>
 
 	* libelfP.h (__elf_strptr_internal): New function declaration.
diff --git a/libelf/elf_strptr.c b/libelf/elf_strptr.c
index f30a06f..62936a0 100644
--- a/libelf/elf_strptr.c
+++ b/libelf/elf_strptr.c
@@ -131,12 +131,18 @@ elf_strptr (elf, idx, offset)
 	goto out;
     }
 
-  if (likely (strscn->rawdata_base != NULL))
-    // XXX Is this correct if a file is read and then new data is added
-    // XXX to the string section?  Likely needs to check offset against
-    // XXX size of rawdata_base buffer and then iterate over rest of the
-    // XXX list.
-    result = &strscn->rawdata_base[offset];
+  if (likely (strscn->data_list_rear == NULL))
+    {
+      // XXX The above is currently correct since elf_newdata will
+      // make sure to convert the rawdata into the datalist if
+      // necessary. But it would be more efficient to keep the rawdata
+      // unconverted and only then iterate over the rest of the (newly
+      // added data) list.  Note that when the ELF file is mmapped
+      // rawdata_base can be set while rawdata.d hasn't been
+      // initialized yet (when data_read is zero). So we cannot just
+      // look at the rawdata.d.d_size.
+      result = &strscn->rawdata_base[offset];
+    }
   else
     {
       /* This is a file which is currently created.  Use the list of
diff --git a/tests/ChangeLog b/tests/ChangeLog
index d41d0e1..97c7ab8 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,10 @@
+2015-01-21  Mark Wielaard  <mjw@redhat.com>
+
+	* Makefile.am (check_PROGRAMS): Add elfstrtab.
+	(TESTS): Likewise.
+	(elfstrtab_LDADD): New variable.
+	* elfstrtab.c: New test.
+
 2015-01-20  Mark Wielaard  <mjw@redhat.com>
 
 	* Makefile.am (check_PROGRAMS): Add newdata.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 4420e8b..5ba8cdd 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -51,7 +51,7 @@ check_PROGRAMS = arextract arsymtest newfile saridx scnnames sectiondump \
 		  dwfl-report-elf-align varlocs backtrace backtrace-child \
 		  backtrace-data backtrace-dwarf debuglink debugaltlink \
 		  buildid deleted deleted-lib.so aggregate_size vdsosyms \
-		  getsrc_die strptr newdata
+		  getsrc_die strptr newdata elfstrtab
 
 asm_TESTS = asm-tst1 asm-tst2 asm-tst3 asm-tst4 asm-tst5 \
 	    asm-tst6 asm-tst7 asm-tst8 asm-tst9
@@ -113,7 +113,7 @@ TESTS = run-arextract.sh run-arsymtest.sh newfile test-nlist \
 	run-backtrace-demangle.sh run-stack-d-test.sh run-stack-i-test.sh \
 	run-readelf-dwz-multi.sh run-allfcts-multi.sh run-deleted.sh \
 	run-linkmap-cut.sh run-aggregate-size.sh vdsosyms run-readelf-A.sh \
-	run-getsrc-die.sh run-strptr.sh newdata
+	run-getsrc-die.sh run-strptr.sh newdata elfstrtab
 
 if !BIARCH
 export ELFUTILS_DISABLE_BIARCH = 1
@@ -427,6 +427,7 @@ vdsosyms_LDADD = $(libdw) $(libelf)
 getsrc_die_LDADD = $(libdw) $(libelf)
 strptr_LDADD = $(libelf)
 newdata_LDADD = $(libelf)
+elfstrtab_LDADD = $(libelf)
 
 if GCOV
 check: check-am coverage
diff --git a/tests/elfstrtab.c b/tests/elfstrtab.c
new file mode 100644
index 0000000..c27d6cf
--- /dev/null
+++ b/tests/elfstrtab.c
@@ -0,0 +1,396 @@
+/* Test program for elf_strptr function.
+   Copyright (C) 2015 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/>.  */
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include ELFUTILS_HEADER(elf)
+#include <gelf.h>
+
+
+/* Index of last string added.  Returned by add_string ().  */
+static size_t stridx = 0;
+
+/* Some random strings.  */
+static char *str1;
+static size_t str1_off;
+static char *str2;
+static size_t str2_off;
+static char *str3;
+static size_t str3_off;
+
+/* First three strings we write out. They should always be there.  */
+static char *orig_str1;
+static size_t orig_str1_off;
+static char *orig_str2;
+static size_t orig_str2_off;
+static char *orig_str3;
+static size_t orig_str3_off;
+
+static void
+check_orig_strings (Elf *elf, int ndx, const char *msg)
+{
+  printf ("checking orig strings: %s\n", msg);
+
+  const char *str = elf_strptr (elf, ndx, 0);
+  printf ("\t'%s'\n", str);
+  if (str == NULL || strcmp ("", str) != 0)
+    exit (1);
+
+  str = elf_strptr (elf, ndx, 1);
+  printf ("\t'%s'\n", str);
+  if (str == NULL || strcmp (".strings", str) != 0)
+    exit (1);
+
+  str = elf_strptr (elf, ndx, orig_str1_off);
+  printf ("\t'%s'\n", str);
+  if (str == NULL || strcmp (orig_str1, str) != 0)
+    exit (1);
+
+  str = elf_strptr (elf, ndx, orig_str2_off);
+  printf ("\t'%s'\n", str);
+  if (str == NULL || strcmp (orig_str2, str) != 0)
+    exit (1);
+
+  str = elf_strptr (elf, ndx, orig_str3_off);
+  printf ("\t'%s'\n", str);
+  if (str == NULL || strcmp (orig_str3, str) != 0)
+    exit (1);
+}
+
+static void
+check_strings (Elf *elf, int ndx, const char *msg)
+{
+  check_orig_strings (elf, ndx, msg);
+
+  const char *str = elf_strptr (elf, ndx, str1_off);
+  printf ("\t'%s'\n", str);
+  if (str == NULL || strcmp (str1, str) != 0)
+    exit (1);
+
+  str = elf_strptr (elf, ndx, str2_off);
+  printf ("\t'%s'\n", str);
+  if (str == NULL || strcmp (str2, str) != 0)
+    exit (1);
+
+  str = elf_strptr (elf, ndx, str3_off);
+  printf ("\t'%s'\n", str);
+  if (str == NULL || strcmp (str3, str) != 0)
+    exit (1);
+}
+
+/* Adds a string and returns the offset in the section.  */
+static size_t
+add_string (Elf_Scn *scn, char *str)
+{
+  size_t lastidx = stridx;
+  size_t size = strlen (str) + 1;
+  
+  Elf_Data *data = elf_newdata (scn);
+  if (data == NULL)
+    {
+      printf ("cannot create data SHSTRTAB section: %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  data->d_buf = str;
+  data->d_type = ELF_T_BYTE;
+  data->d_size = size;
+  data->d_align = 1;
+  data->d_version = EV_CURRENT;
+
+  stridx += size;
+  printf ("add_string: '%s', stridx: %zd, lastidx: %zd\n",
+	  str, stridx, lastidx);
+  return lastidx;
+}
+
+static void
+check_elf (const char *fname, int class, int use_mmap)
+{
+  printf ("\nfname: %s\n", fname);
+  stridx = 0;
+
+  int fd = open (fname, O_RDWR | O_CREAT | O_TRUNC, 0666);
+  if (fd == -1)
+    {
+      printf ("cannot open `%s': %s\n", fname, strerror (errno));
+      exit (1);
+    }
+
+  Elf *elf = elf_begin (fd, use_mmap ? ELF_C_WRITE_MMAP : ELF_C_WRITE, NULL);
+  if (elf == NULL)
+    {
+      printf ("cannot create ELF descriptor: %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  // Create an ELF header.
+  if (gelf_newehdr (elf, class) == 0)
+    {
+      printf ("cannot create ELF header: %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  GElf_Ehdr ehdr_mem;
+  GElf_Ehdr *ehdr = gelf_getehdr (elf, &ehdr_mem);
+  if (ehdr == NULL)
+    {
+      printf ("cannot get ELF header: %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  // Initialize header.
+  ehdr->e_ident[EI_DATA] = class == ELFCLASS64 ? ELFDATA2LSB : ELFDATA2MSB;
+  ehdr->e_ident[EI_OSABI] = ELFOSABI_GNU;
+  ehdr->e_type = ET_NONE;
+  ehdr->e_machine = EM_X86_64;
+  ehdr->e_version = EV_CURRENT;
+
+  // Create strings section.
+  Elf_Scn *scn = elf_newscn (elf);
+  if (scn == NULL)
+    {
+      printf ("cannot create strings section: %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  // Add an empty string to the table as NUL entry for section zero.
+  add_string (scn, "");
+
+  GElf_Shdr shdr_mem;
+  GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem);
+  if (shdr == NULL)
+    {
+      printf ("cannot get header for strings section: %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  shdr->sh_type = SHT_STRTAB;
+  shdr->sh_flags = 0;
+  shdr->sh_addr = 0;
+  shdr->sh_link = SHN_UNDEF;
+  shdr->sh_info = SHN_UNDEF;
+  shdr->sh_addralign = 1;
+  shdr->sh_entsize = 0;
+  shdr->sh_name = add_string (scn, ".strings");
+
+  // We have to store the section strtab index in the ELF header.
+  // So sections have actual names.
+  int ndx = elf_ndxscn (scn);
+  ehdr->e_shstrndx = ndx;
+
+  if (gelf_update_ehdr (elf, ehdr) == 0)
+    {
+      printf ("cannot update ELF header: %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  // Add some random strings. These are the original ones. They should
+  // always be there (together with the empty "" and .strings section
+  // name strings.
+  orig_str1 = "elfutils";
+  orig_str1_off = add_string (scn, orig_str1);
+  orig_str2 = "strtabelf";
+  orig_str2_off = add_string (scn, orig_str2);
+  orig_str3 = "three";
+  orig_str3_off = add_string (scn, orig_str3);
+
+  // Finished strings section, update the header.
+  if (gelf_update_shdr (scn, shdr) == 0)
+    {
+      printf ("cannot update STRTAB section header: %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  // Let the library compute the internal structure information.
+  if (elf_update (elf, ELF_C_NULL) < 0)
+    {
+      printf ("failure in elf_update(NULL): %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  // Check our strings are there.
+  check_orig_strings (elf, ndx, "first elf_update, before write");
+
+  // Write everything to disk.
+  if (elf_update (elf, ELF_C_WRITE) < 0)
+    {
+      printf ("failure in elf_update(WRITE): %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  // Check out strings are there.
+  check_orig_strings (elf, ndx, "first elf_update, after write");
+
+  // Add some more random strings.  These will not be written to disk.
+  scn = elf_getscn (elf, ndx);
+  if (scn == NULL)
+    {
+      printf ("couldn't re-get strings section: %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  str1 = "elfutils2";
+  str1_off = add_string (scn, str1);
+  str2 = "strtabelf2";
+  str2_off = add_string (scn, str2);
+  str3 = "three2";
+  str3_off = add_string (scn, str3);
+
+  // Update internal structure information again.
+  if (elf_update (elf, ELF_C_NULL) < 0)
+    {
+      printf ("failure in re-elf_update(NULL): %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  // Check our new strings are there.
+  check_strings (elf, ndx, "first extra strings");
+
+  if (elf_end (elf) != 0)
+    {
+      printf ("failure in elf_end: %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  close (fd);
+
+  /* Read the ELF from disk now.  */
+  fd = open (fname, O_RDWR, 0666);
+  if (fd == -1)
+    {
+      printf ("cannot open `%s' read-only: %s\n", fname, strerror (errno));
+      exit (1);
+    }
+
+  elf = elf_begin (fd, use_mmap ? ELF_C_RDWR_MMAP : ELF_C_RDWR, NULL);
+  if (elf == NULL)
+    {
+      printf ("cannot create ELF descriptor read-only: %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  /* Are our strings there?  */
+  check_orig_strings (elf, ndx, "read ELF file, orig strings");
+
+  // Add some more random strings.
+  scn = elf_getscn (elf, ndx);
+  if (scn == NULL)
+    {
+      printf ("couldn't re-get strings section: %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  shdr = gelf_getshdr (scn, &shdr_mem);
+  if (shdr == NULL)
+    {
+      printf ("cannot get header for strings section: %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  // Reset stridx to end of section.
+  printf ("sh_size: %" PRIu64 "\n", shdr->sh_size);
+  stridx = shdr->sh_size;
+
+  str1 = "0123456789";
+  str1_off = add_string (scn, str1);
+  str2 = "supercalifragilisticexpialidocious";
+  str2_off = add_string (scn, str2);
+  str3 = "forty-two";
+  str3_off = add_string (scn, str3);
+
+  // Update internal structure information.
+  if (elf_update (elf, ELF_C_NULL) < 0)
+    {
+      printf ("failure in rw-elf_update(NULL): %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  /* Check our new strings are there.  */
+  check_strings (elf, ndx, "read file, added strings");
+
+  // Write updated ELF file.
+  if (elf_update (elf, ELF_C_WRITE) < 0)
+    {
+      printf ("failure in re-elf_update(NULL): %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  if (elf_end (elf) != 0)
+    {
+      printf ("failure in elf_end: %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  close (fd);
+
+  // And read it in one last time.
+  fd = open (fname, O_RDONLY, 0666);
+  if (fd == -1)
+    {
+      printf ("cannot open `%s' read-only: %s\n", fname, strerror (errno));
+      exit (1);
+    }
+
+  elf = elf_begin (fd, use_mmap ? ELF_C_READ_MMAP : ELF_C_READ, NULL);
+  if (elf == NULL)
+    {
+      printf ("cannot create ELF descriptor read-only: %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  /* Are all our strings there?  */
+  check_strings (elf, ndx, "all together now");
+
+  if (elf_end (elf) != 0)
+    {
+      printf ("failure in elf_end: %s\n", elf_errmsg (-1));
+      exit (1);
+    }
+
+  close (fd);
+
+  unlink (fname);
+}
+
+int
+main (int argc __attribute__ ((unused)), char *argv[] __attribute__ ((unused)))
+{
+  elf_version (EV_CURRENT);
+
+  // Fill holes with something non-zero to more easily spot
+  // unterminated strings.
+  elf_fill ('X');
+
+  check_elf ("strtab.elf.32", ELFCLASS32, 0);
+  check_elf ("strtab.elf.32.mmap", ELFCLASS32, 1);
+  check_elf ("strtab.elf.64", ELFCLASS64, 0);
+  check_elf ("strtab.elf.64.mmap", ELFCLASS64, 1);
+
+  return 0;
+}
+
-- 
1.8.3.1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]