From: Mark Wielaard Date: Wed, 6 Jul 2016 13:27:56 +0000 (+0200) Subject: libelf: Allow updating phdrs for any e_type. X-Git-Tag: elfutils-0.167~18 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8b5f017ddf1684e225ef59f9243ef411b2556e9c;p=thirdparty%2Felfutils.git libelf: Allow updating phdrs for any e_type. elf[32|64]_updatenull would sanity check the e_type before allowing to update the phdrs. This prevents creating an ET_REL file with phdrs. It also prevents creating any vendor specific ELF file having phdrs. We only check this when updating/writing out the file. But we would just read such files. Don't prevent people from creating unexpected ELF files. elflint will warn for such files. While writing a new testcase for this another bug was found that prevented updating a just created phdr because elf_getphdrnum would sanity check the phdr offset in the file (which doesn't exist yet). Fix that by only doing such a sanity check if the phdrs haven't been read in or created yet. This second bug should have been found by the existing elfshphehdr test, but that test contained a typo checking elf_getphdrnum. It tested that the called failed when there were no phdrs, but then elf_getphdrnum should simply succeed and return zero. https://bugzilla.redhat.com/show_bug.cgi?id=1352232 Signed-off-by: Mark Wielaard --- diff --git a/libelf/ChangeLog b/libelf/ChangeLog index 82a2a9f4b..d445fe6a3 100644 --- a/libelf/ChangeLog +++ b/libelf/ChangeLog @@ -1,3 +1,10 @@ +2016-07-06 Mark Wielaard + + * elf32_updatenull.c (updatenull_wrlock): Ignore e_type when + updating phdrs. + * elf_getphdrnum.c (__elf_getphdrnum_chk_rdlock): Only do sanity + checking if phdrs haven't been read in yet. + 2016-06-24 John Ogness * elf32_updatenull.c (updatenull_wrlock): Find first section. diff --git a/libelf/elf32_updatenull.c b/libelf/elf32_updatenull.c index 750706289..939aa13eb 100644 --- a/libelf/elf32_updatenull.c +++ b/libelf/elf32_updatenull.c @@ -1,5 +1,5 @@ /* Update data structures for changes. - Copyright (C) 2000-2010, 2015 Red Hat, Inc. + Copyright (C) 2000-2010, 2015, 2016 Red Hat, Inc. This file is part of elfutils. Written by Ulrich Drepper , 2000. @@ -140,21 +140,10 @@ __elfw2(LIBELFBITS,updatenull_wrlock) (Elf *elf, int *change_bop, size_t shnum) off_t size = elf_typesize (LIBELFBITS, ELF_T_EHDR, 1); /* Set the program header position. */ - if (elf->state.ELFW(elf,LIBELFBITS).phdr == NULL - && (ehdr->e_type == ET_EXEC || ehdr->e_type == ET_DYN - || ehdr->e_type == ET_CORE)) + if (elf->state.ELFW(elf,LIBELFBITS).phdr == NULL) (void) __elfw2(LIBELFBITS,getphdr_wrlock) (elf); if (elf->state.ELFW(elf,LIBELFBITS).phdr != NULL) { - /* Only executables, shared objects, and core files have a program - header. */ - if (ehdr->e_type != ET_EXEC && ehdr->e_type != ET_DYN - && unlikely (ehdr->e_type != ET_CORE)) - { - __libelf_seterrno (ELF_E_INVALID_PHDR); - return -1; - } - size_t phnum; if (unlikely (__elf_getphdrnum_rdlock (elf, &phnum) != 0)) return -1; diff --git a/libelf/elf_getphdrnum.c b/libelf/elf_getphdrnum.c index 061183bb9..f91cba981 100644 --- a/libelf/elf_getphdrnum.c +++ b/libelf/elf_getphdrnum.c @@ -1,5 +1,5 @@ /* Return number of program headers in the ELF file. - Copyright (C) 2010, 2014, 2015 Red Hat, Inc. + Copyright (C) 2010, 2014, 2015, 2016 Red Hat, Inc. This file is part of elfutils. This file is free software; you can redistribute it and/or modify @@ -84,35 +84,39 @@ __elf_getphdrnum_chk_rdlock (Elf *elf, size_t *dst) { int result = __elf_getphdrnum_rdlock (elf, dst); - /* Do some sanity checking to make sure phnum and phoff are consistent. */ - Elf64_Off off = (elf->class == ELFCLASS32 - ? elf->state.elf32.ehdr->e_phoff - : elf->state.elf64.ehdr->e_phoff); - if (unlikely (off == 0)) + /* If the phdrs haven't been created or read in yet then do some + sanity checking to make sure phnum and phoff are consistent. */ + if (elf->state.elf.phdr == NULL) { - *dst = 0; - return result; + Elf64_Off off = (elf->class == ELFCLASS32 + ? elf->state.elf32.ehdr->e_phoff + : elf->state.elf64.ehdr->e_phoff); + if (unlikely (off == 0)) + { + *dst = 0; + return result; + } + + if (unlikely (off >= elf->maximum_size)) + { + __libelf_seterrno (ELF_E_INVALID_DATA); + return -1; + } + + /* Check for too many sections. */ + size_t phdr_size = (elf->class == ELFCLASS32 + ? sizeof (Elf32_Phdr) : sizeof (Elf64_Phdr)); + if (unlikely (*dst > SIZE_MAX / phdr_size)) + { + __libelf_seterrno (ELF_E_INVALID_DATA); + return -1; + } + + /* Truncated file? Don't return more than can be indexed. */ + if (unlikely (elf->maximum_size - off < *dst * phdr_size)) + *dst = (elf->maximum_size - off) / phdr_size; } - if (unlikely (off >= elf->maximum_size)) - { - __libelf_seterrno (ELF_E_INVALID_DATA); - return -1; - } - - /* Check for too many sections. */ - size_t phdr_size = (elf->class == ELFCLASS32 - ? sizeof (Elf32_Phdr) : sizeof (Elf64_Phdr)); - if (unlikely (*dst > SIZE_MAX / phdr_size)) - { - __libelf_seterrno (ELF_E_INVALID_DATA); - return -1; - } - - /* Truncated file? Don't return more than can be indexed. */ - if (unlikely (elf->maximum_size - off < *dst * phdr_size)) - *dst = (elf->maximum_size - off) / phdr_size; - return result; } diff --git a/tests/ChangeLog b/tests/ChangeLog index 73aad091b..cd0d2feef 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,11 @@ +2016-07-06 Mark Wielaard + + * Makefile.am (check_PROGRAMS): Add vendorelf. + (TESTS): Likewise. + (vendorelf_LDADD): New variable. + * vendorelf.c: New test. + * elfshphehdr.c (test): Check elf_getphdrnum succeeds. + 2016-06-24 Mark Wielaard * Makefile.am (check_PROGRAMS): Add emptyfile. diff --git a/tests/Makefile.am b/tests/Makefile.am index 9c22c4d73..f80436abb 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -53,7 +53,7 @@ check_PROGRAMS = arextract arsymtest newfile saridx scnnames sectiondump \ buildid deleted deleted-lib.so aggregate_size vdsosyms \ getsrc_die strptr newdata elfstrtab dwfl-proc-attach \ elfshphehdr elfstrmerge dwelfgnucompressed elfgetchdr \ - elfgetzdata elfputzdata zstrptr emptyfile + elfgetzdata elfputzdata zstrptr emptyfile vendorelf asm_TESTS = asm-tst1 asm-tst2 asm-tst3 asm-tst4 asm-tst5 \ asm-tst6 asm-tst7 asm-tst8 asm-tst9 @@ -127,7 +127,7 @@ TESTS = run-arextract.sh run-arsymtest.sh newfile test-nlist \ run-elfgetzdata.sh run-elfputzdata.sh run-zstrptr.sh \ run-compress-test.sh \ run-readelf-zdebug.sh run-readelf-zdebug-rel.sh \ - emptyfile + emptyfile vendorelf if !BIARCH export ELFUTILS_DISABLE_BIARCH = 1 @@ -483,6 +483,7 @@ elfgetzdata_LDADD = $(libelf) elfputzdata_LDADD = $(libelf) zstrptr_LDADD = $(libelf) emptyfile_LDADD = $(libelf) +vendorelf_LDADD = $(libelf) # We want to test the libelf header against the system elf.h header. # Don't include any -I CPPFLAGS. diff --git a/tests/elfshphehdr.c b/tests/elfshphehdr.c index 0d92934b6..5a297e0db 100644 --- a/tests/elfshphehdr.c +++ b/tests/elfshphehdr.c @@ -126,7 +126,7 @@ test (Elf *elf, int class, bool layout) check_elf ("elf_getshdrnum", elf_getshdrnum (elf, &shnum) == 0); check ("shnum == 1", shnum == 2); /* section zero is also created. */ - check_elf ("elf_getphdrnum", elf_getphdrnum (elf, &phnum) != 0); + check_elf ("elf_getphdrnum", elf_getphdrnum (elf, &phnum) == 0); check ("phnum == 1", phnum == 1); check_elf ("gelf_getehdr", gelf_getehdr (elf, &ehdr) != NULL); diff --git a/tests/vendorelf.c b/tests/vendorelf.c new file mode 100644 index 000000000..bc13cce31 --- /dev/null +++ b/tests/vendorelf.c @@ -0,0 +1,197 @@ +/* Test program for adding a program header to a vendor specific ELF file. + Copyright (C) 2016 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 . */ + + +#ifdef HAVE_CONFIG_H +# include +#endif + +#include +#include +#include +#include +#include +#include +#include + +#include ELFUTILS_HEADER(elf) +#include + +void +check_elf (const char *fname, int class, int use_mmap) +{ + printf ("\nfname: %s\n", fname); + + 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_LOOS + 1; + ehdr->e_machine = EM_X86_64; + ehdr->e_version = EV_CURRENT; + + if (gelf_update_ehdr (elf, ehdr) == 0) + { + printf ("cannot update ELF header: %s\n", elf_errmsg (-1)); + exit (1); + } + + // Create a program header. + if (gelf_newphdr (elf, 1) == 0) + { + printf ("cannot create program header: %s\n", elf_errmsg (-1)); + exit (1); + } + + GElf_Phdr phdr; + if (gelf_getphdr (elf, 0, &phdr) == NULL) + { + printf ("cannot get program header: %s\n", elf_errmsg (-1)); + exit (1); + } + + // Some random values to check later. + phdr.p_type = PT_NULL; + phdr.p_offset = 0; + phdr.p_vaddr = 0; + phdr.p_paddr = 1; + phdr.p_filesz = 0; + phdr.p_memsz = 1024; + phdr.p_flags = PF_R; + phdr.p_align = 16; + + if (gelf_update_phdr (elf, 0, &phdr) == 0) + { + printf ("cannot update program header: %s\n", elf_errmsg (-1)); + exit (1); + } + + // 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); + } + + if (elf_end (elf) != 0) + { + printf ("failure in elf_end: %s\n", elf_errmsg (-1)); + exit (1); + } + + close (fd); + + /* Reread the ELF from disk now. */ + 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); + } + + // Is our phdr there? + size_t phnum; + if (elf_getphdrnum (elf, &phnum) != 0) + { + printf ("cannot get phdr num: %s\n", elf_errmsg (-1)); + exit (1); + } + + if (phnum != 1) + { + printf ("Expected just 1 phdr, got: %zd\n", phnum); + exit (1); + } + + if (gelf_getphdr (elf, 0, &phdr) == NULL) + { + printf ("cannot get program header from file: %s\n", elf_errmsg (-1)); + exit (1); + } + + if (phdr.p_type != PT_NULL + || phdr.p_offset != 0 + || phdr.p_vaddr != 0 + || phdr.p_paddr != 1 + || phdr.p_filesz != 0 + || phdr.p_memsz != 1024 + || phdr.p_flags != PF_R + || phdr.p_align != 16) + { + printf ("Unexpected phdr values\n"); + exit (1); + } + + 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); + + check_elf ("vendor.elf.32", ELFCLASS32, 0); + check_elf ("vendor.elf.32.mmap", ELFCLASS32, 1); + check_elf ("vendor.elf.64", ELFCLASS64, 0); + check_elf ("vendor.elf.64.mmap", ELFCLASS64, 1); + + return 0; +}