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; +} +
On Thu, Jan 22, 2015 at 12:20:52AM +0100, Mark Wielaard wrote:
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.
I pushed this to master.
elfutils-devel@lists.fedorahosted.org