elf_strptr might be called before the shdrs are read in. In that case it needs to explicitly call __elf[32|64]_getshdr_rdlock to check the section type and size. The new strptr testcase triggers this corner case and crashes before the fix.
Signed-off-by: Mark Wielaard mjw@redhat.com --- libelf/ChangeLog | 5 +++ libelf/elf_strptr.c | 12 ++++--- tests/ChangeLog | 9 +++++ tests/Makefile.am | 7 ++-- tests/run-strptr.sh | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++ tests/strptr.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 218 insertions(+), 8 deletions(-) create mode 100755 tests/run-strptr.sh create mode 100644 tests/strptr.c
diff --git a/libelf/ChangeLog b/libelf/ChangeLog index adfccf1..6699052 100644 --- a/libelf/ChangeLog +++ b/libelf/ChangeLog @@ -1,3 +1,8 @@ +2015-01-20 Mark Wielaard mjw@redhat.com + + * elf_strptr.c (elf_strptr): Call __elf[32|64]_getshdr_rdlock if + necessary. + 2015-01-03 Mark Wielaard mjw@redhat.com
* version_xlate.h (elf_cvt_Verdef): Use memmove to copy src to dest. diff --git a/libelf/elf_strptr.c b/libelf/elf_strptr.c index 1f40429..f30a06f 100644 --- a/libelf/elf_strptr.c +++ b/libelf/elf_strptr.c @@ -1,5 +1,5 @@ /* Return string pointer from string section. - Copyright (C) 1998, 1999, 2000, 2001, 2002, 2004, 2008, 2009 Red Hat, Inc. + Copyright (C) 1998-2002, 2004, 2008, 2009, 2015 Red Hat, Inc. This file is part of elfutils. Contributed by Ulrich Drepper drepper@redhat.com, 1998.
@@ -88,14 +88,15 @@ elf_strptr (elf, idx, offset)
if (elf->class == ELFCLASS32) { - if (unlikely (strscn->shdr.e32->sh_type != SHT_STRTAB)) + Elf32_Shdr *shdr = strscn->shdr.e32 ?: __elf32_getshdr_rdlock (strscn); + if (unlikely (shdr->sh_type != SHT_STRTAB)) { /* This is no string section. */ __libelf_seterrno (ELF_E_INVALID_SECTION); goto out; }
- if (unlikely (offset >= strscn->shdr.e32->sh_size)) + if (unlikely (offset >= shdr->sh_size)) { /* The given offset is too big, it is beyond this section. */ __libelf_seterrno (ELF_E_OFFSET_RANGE); @@ -104,14 +105,15 @@ elf_strptr (elf, idx, offset) } else { - if (unlikely (strscn->shdr.e64->sh_type != SHT_STRTAB)) + Elf64_Shdr *shdr = strscn->shdr.e64 ?: __elf64_getshdr_rdlock (strscn); + if (unlikely (shdr->sh_type != SHT_STRTAB)) { /* This is no string section. */ __libelf_seterrno (ELF_E_INVALID_SECTION); goto out; }
- if (unlikely (offset >= strscn->shdr.e64->sh_size)) + if (unlikely (offset >= shdr->sh_size)) { /* The given offset is too big, it is beyond this section. */ __libelf_seterrno (ELF_E_OFFSET_RANGE); diff --git a/tests/ChangeLog b/tests/ChangeLog index 87a7ce7..f94d9be 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,12 @@ +2015-01-20 Mark Wielaard mjw@redhat.com + + * strptr.c: New file. + * run-strptr.sh: New test. + * Makefile.am (check_PROGRAMS): Add strptr. + (TESTS): Add run-strptr.sh. + (EXTRA_DIST): Likewise. + (strptr_LDADD): New variable. + 2015-01-15 Mark Wielaard mjw@redhat.com
* deleted.c (main): Call prctl(PR_SET_PTRACER, PR_SET_PTRACER_ANY). diff --git a/tests/Makefile.am b/tests/Makefile.am index cbc1eb2..c3364a2 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 + getsrc_die strptr
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-getsrc-die.sh run-strptr.sh
if !BIARCH export ELFUTILS_DISABLE_BIARCH = 1 @@ -284,7 +284,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh \ testfile-sizes3.o.bz2 \ run-readelf-A.sh testfileppc32attrs.o.bz2 \ testfile-debug-types.bz2 \ - run-getsrc-die.sh + run-getsrc-die.sh run-strptr.sh
if USE_VALGRIND valgrind_cmd='valgrind -q --error-exitcode=1 --run-libc-freeres=no' @@ -425,6 +425,7 @@ deleted_lib_so_CFLAGS = -fPIC -fasynchronous-unwind-tables aggregate_size_LDADD = $(libdw) $(libelf) vdsosyms_LDADD = $(libdw) $(libelf) getsrc_die_LDADD = $(libdw) $(libelf) +strptr_LDADD = $(libelf)
if GCOV check: check-am coverage diff --git a/tests/run-strptr.sh b/tests/run-strptr.sh new file mode 100755 index 0000000..af90a02 --- /dev/null +++ b/tests/run-strptr.sh @@ -0,0 +1,98 @@ +#! /bin/sh +# 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/. + +. $srcdir/test-subr.sh + +# A random 32bit and 64bit testfile +testfiles testfile testfile10 + +testrun_compare ${abs_top_builddir}/tests/strptr testfile <<\EOF +Strings in section 32: +[0] '' +[1] '.symtab' +[9] '.strtab' +[11] '.shstrtab' +[1b] '.interp' +[23] '.note.ABI-tag' +[31] '.hash' +[37] '.dynsym' +[3f] '.dynstr' +[47] '.gnu.version' +[54] '.gnu.version_r' +[63] '.rel.got' +[6c] '.rel.plt' +[75] '.init' +[7b] '.plt' +[80] '.text' +[86] '.fini' +[8c] '.rodata' +[94] '.data' +[9a] '.eh_frame' +[a4] '.ctors' +[ab] '.dtors' +[b2] '.got' +[b7] '.dynamic' +[c0] '.sbss' +[c6] '.bss' +[cb] '.stab' +[d1] '.stabstr' +[da] '.comment' +[e3] '.debug_aranges' +[f2] '.debug_pubnames' +[102] '.debug_info' +[10e] '.debug_abbrev' +[11c] '.debug_line' +[128] '.note' +EOF + +testrun_compare ${abs_top_builddir}/tests/strptr testfile10 <<\EOF +Strings in section 30: +[0] '' +[1] '.symtab' +[9] '.strtab' +[11] '.shstrtab' +[1b] '.hash' +[21] '.dynsym' +[29] '.dynstr' +[31] '.gnu.version' +[3e] '.gnu.version_r' +[4d] '.rela.dyn' +[57] '.init' +[5d] '.text' +[63] '.fini' +[69] '.eh_frame' +[73] '.data' +[79] '.dynamic' +[82] '.ctors' +[89] '.dtors' +[90] '.jcr' +[95] '.plt' +[9a] '.got' +[9f] '.sdata' +[a6] '.sbss' +[ac] '.bss' +[b1] '.comment' +[ba] '.debug_aranges' +[c9] '.debug_pubnames' +[d9] '.debug_abbrev' +[e7] '.debug_line' +[f3] '.debug_frame' +[100] '.debug_str' +[10b] '.rela.debug_info' +EOF + +exit 0 diff --git a/tests/strptr.c b/tests/strptr.c new file mode 100644 index 0000000..759664a --- /dev/null +++ b/tests/strptr.c @@ -0,0 +1,95 @@ +/* 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> + +int +main (int argc, char *argv[]) +{ + if (argc != 2) + { + printf ("No ELF file given as argument"); + exit (1); + } + + const char *fname = argv[1]; + + // Initialize libelf. + elf_version (EV_CURRENT); + + /* Read the ELF from disk now. */ + int fd = open (fname, O_RDONLY); + if (fd == -1) + { + printf ("cannot open `%s' read-only: %s\n", fname, strerror (errno)); + exit (1); + } + + Elf *elf = elf_begin (fd, ELF_C_READ, NULL); + if (elf == NULL) + { + printf ("cannot create ELF descriptor read-only: %s\n", elf_errmsg (-1)); + exit (1); + } + + size_t ndx; + if (elf_getshdrstrndx (elf, &ndx) != 0) + { + printf ("cannot get section header table index: %s\n", elf_errmsg (-1)); + exit (1); + } + + if (ndx == SHN_UNDEF) + { + printf ("ELF file `%s' doesn't have a section header table index", fname); + exit (1); + } + + printf ("Strings in section %zd:\n", ndx); + + size_t off = 0; + const char *str = elf_strptr (elf, ndx, off); + while (str != NULL) + { + printf ("[%zx] '%s'\n", off, str); + off += strlen (str) + 1; + str = elf_strptr (elf, ndx, off); + } + + if (elf_end (elf) != 0) + { + printf ("failure in elf_end: %s\n", elf_errmsg (-1)); + exit (1); + } + + close (fd); + + return 0; +}
On Tue, Jan 20, 2015 at 03:09:14PM +0100, Mark Wielaard wrote:
elf_strptr might be called before the shdrs are read in. In that case it needs to explicitly call __elf[32|64]_getshdr_rdlock to check the section type and size. The new strptr testcase triggers this corner case and crashes before the fix.
I pushed this to master.
elfutils-devel@lists.fedorahosted.org