commit d7193bd7c9dc2a979352eee7fc446dacd3e97779 Author: Mark Wielaard Date: Sun May 12 00:37:45 2019 +0200 libelf: Mark shdr_flags dirty if offset or size changes during update. We forgot to mark the shdr_flags dirty when only the sh_size or sh_offset changed during elf_update (). This meant that if there were no other shdr changes we only wrote out the section data, but didn't write out the shdr table to the file. Add a testcase that puts some sections in the reverse order and then writes out the resulting file again without doing any other updates. This would show the issue after write out of the (re-reversed) ELF file (the .shstrtab section offset would be wrong causing all section names to be garbage). Also run a self test. Signed-off-by: Mark Wielaard ChangeLog 2019-05-12 Mark Wielaard * elf32_updatenull.c (updatenull_wrlock): Mark shdr_flags dirty if either offset or size changed. tests/ChangeLog 2019-05-12 Mark Wielaard * Makefile.am (check_PROGRAMS): Add elfrdwrnop. (TESTS): Add run-reverse-sections.sh and run-reverse-sections-self.sh. (EXTRA_DIST): Likewise. (elfrdwrnop): New variable. * elfcopy.c (copy_elf): Add reverse_off argument. Record offsets of sections and swap them when possible. (main): Check for --reverse-off argument. Pass reverse_offs to copy_elf. * run-reverse-sections.sh: New test. * run-reverse-sections-self.sh: Likewise. * elfrdwrnop.c: New file. Index: elfutils-0.176/libelf/elf32_updatenull.c =================================================================== --- elfutils-0.176.orig/libelf/elf32_updatenull.c 2019-05-21 14:44:35.000000000 +0200 +++ elfutils-0.176/libelf/elf32_updatenull.c 2019-05-21 14:44:38.000000000 +0200 @@ -367,12 +367,15 @@ __elfw2(LIBELFBITS,updatenull_wrlock) (E } /* See whether the section size is correct. */ + int size_changed = 0; update_if_changed (shdr->sh_size, (GElf_Word) offset, - changed); + size_changed); + changed |= size_changed; if (shdr->sh_type != SHT_NOBITS) size += offset; + scn->shdr_flags |= (offset_changed | size_changed); scn->flags |= changed; } Index: elfutils-0.176/tests/Makefile.am =================================================================== --- elfutils-0.176.orig/tests/Makefile.am 2019-05-21 14:44:35.000000000 +0200 +++ elfutils-0.176/tests/Makefile.am 2019-05-21 14:45:44.000000000 +0200 @@ -60,7 +60,7 @@ check_PROGRAMS = arextract arsymtest new fillfile dwarf_default_lower_bound dwarf-die-addr-die \ get-units-invalid get-units-split attr-integrate-skel \ all-dwarf-ranges unit-info next_cfi \ - elfcopy addsections + elfcopy addsections elfrdwrnop asm_TESTS = asm-tst1 asm-tst2 asm-tst3 asm-tst4 asm-tst5 \ asm-tst6 asm-tst7 asm-tst8 asm-tst9 @@ -157,6 +157,7 @@ TESTS = run-arextract.sh run-arsymtest.s run-all-dwarf-ranges.sh run-unit-info.sh \ run-reloc-bpf.sh \ run-next-cfi.sh run-next-cfi-self.sh \ + run-reverse-sections.sh run-reverse-sections-self.sh \ run-copyadd-sections.sh run-copymany-sections.sh \ run-typeiter-many.sh run-strip-test-many.sh \ run-strip-version.sh @@ -418,6 +419,7 @@ EXTRA_DIST = run-arextract.sh run-arsymt run-unit-info.sh run-next-cfi.sh run-next-cfi-self.sh \ testfile-riscv64.bz2 testfile-riscv64-s.bz2 \ testfile-riscv64-core.bz2 \ + run-reverse-sections.sh run-reverse-sections-self.sh \ run-copyadd-sections.sh run-copymany-sections.sh \ run-typeiter-many.sh run-strip-test-many.sh \ testfile-debug-rel-ppc64-g.o.bz2 \ @@ -593,6 +595,7 @@ unit_info_LDADD = $(libdw) next_cfi_LDADD = $(libelf) $(libdw) elfcopy_LDADD = $(libelf) addsections_LDADD = $(libelf) +elfrdwrnop_LDADD = $(libelf) # We want to test the libelf header against the system elf.h header. # Don't include any -I CPPFLAGS. Except when we install our own elf.h. Index: elfutils-0.176/tests/elfcopy.c =================================================================== --- elfutils-0.176.orig/tests/elfcopy.c 2019-05-21 14:44:35.000000000 +0200 +++ elfutils-0.176/tests/elfcopy.c 2019-05-21 14:44:38.000000000 +0200 @@ -69,9 +69,11 @@ setshstrndx (Elf *elf, size_t ndx) /* Copies all elements of an ELF file either using mmap or read. */ static void -copy_elf (const char *in, const char *out, bool use_mmap) +copy_elf (const char *in, const char *out, bool use_mmap, bool reverse_offs) { - printf ("\ncopy_elf: %s -> %s (%s)\n", in, out, use_mmap ? "mmap" : "read"); + printf ("\ncopy_elf: %s -> %s (%s,%s)\n", in, out, + use_mmap ? "mmap" : "read", + reverse_offs ? "reverse" : "same"); /* Existing ELF file. */ int fda = open (in, O_RDONLY); @@ -182,8 +184,28 @@ copy_elf (const char *in, const char *ou } } + GElf_Off *offs = NULL; + size_t shnum; + if (reverse_offs) + { + if (elf_getshdrnum (elfa, &shnum) < 0) + { + printf ("couldn't get shdrnum: %s\n", elf_errmsg (-1)); + exit (1); + } + + offs = (GElf_Off *) malloc (shnum * sizeof (GElf_Off)); + if (offs == NULL) + { + printf ("couldn't allocate memory for offs\n"); + exit (1); + } + } + /* Copy all sections, headers and data. */ Elf_Scn *scn = NULL; + size_t last_off = 0; + GElf_Shdr last_shdr = { .sh_type = SHT_NULL }; while ((scn = elf_nextscn (elfa, scn)) != NULL) { /* Get the header. */ @@ -194,6 +216,34 @@ copy_elf (const char *in, const char *ou exit (1); } + if (reverse_offs) + { + offs[last_off] = shdr.sh_offset; + + if (last_shdr.sh_type != SHT_NULL + && last_shdr.sh_addralign == shdr.sh_addralign + && shdr.sh_addralign == 1 + && last_shdr.sh_type != SHT_NOBITS + && shdr.sh_type != SHT_NOBITS + && (phnum == 0 + || ((shdr.sh_flags & SHF_ALLOC) == 0 + && (last_shdr.sh_flags & SHF_ALLOC) == 0))) + { + printf ("Swapping offsets of section %zd and %zd\n", + last_off, last_off + 1); + GElf_Word off = offs[last_off - 1]; + offs[last_off - 1] = off + shdr.sh_size; + offs[last_off] = off; + last_shdr.sh_type = SHT_NULL; + } + else + { + last_shdr = shdr; + offs[last_off] = shdr.sh_offset; + } + last_off++; + } + /* Create new section. */ Elf_Scn *new_scn = elf_newscn (elfb); if (new_scn == NULL) @@ -223,9 +273,34 @@ copy_elf (const char *in, const char *ou } } - /* Write everything to disk. If there are any phdrs then we want - the exact same layout. Do we want ELF_F_PERMISSIVE? */ - if (phnum > 0) + if (reverse_offs) + { + last_off = 0; + scn = NULL; + while ((scn = elf_nextscn (elfb, scn)) != NULL) + { + GElf_Shdr shdr; + if (gelf_getshdr (scn, &shdr) == NULL) + { + printf ("couldn't get shdr for updating: %s\n", elf_errmsg (-1)); + exit (1); + } + + shdr.sh_offset = offs[last_off++]; + + if (gelf_update_shdr (scn, &shdr) == 0) + { + printf ("couldn't update shdr sh_off: %s\n", elf_errmsg (-1)); + exit (1); + } + } + free (offs); + } + + /* Write everything to disk. If there are any phdrs, or we want to + update the offsets, then we want the exact same layout. Do we + want ELF_F_PERMISSIVE? */ + if (phnum > 0 || reverse_offs) elf_flagelf (elfb, ELF_C_SET, ELF_F_LAYOUT); if (elf_update (elfb, ELF_C_WRITE) < 0) { @@ -264,9 +339,9 @@ main (int argc, const char *argv[]) elf_version (EV_CURRENT); /* Takes the given file, and create a new identical one. */ - if (argc < 3 || argc > 4) + if (argc < 3 || argc > 5) { - fprintf (stderr, "elfcopy [--mmap] in.elf out.elf\n"); + fprintf (stderr, "elfcopy [--mmap] [--reverse-offs] in.elf out.elf\n"); exit (1); } @@ -278,9 +353,16 @@ main (int argc, const char *argv[]) argn++; } + bool reverse_offs = false; + if (strcmp (argv[argn], "--reverse-offs") == 0) + { + reverse_offs = true; + argn++; + } + const char *in = argv[argn++]; const char *out = argv[argn]; - copy_elf (in, out, use_mmap); + copy_elf (in, out, use_mmap, reverse_offs); return 0; } Index: elfutils-0.176/tests/elfrdwrnop.c =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ elfutils-0.176/tests/elfrdwrnop.c 2019-05-21 14:44:38.000000000 +0200 @@ -0,0 +1,100 @@ +/* Test program for reading and writing out the same file in-place + Copyright (C) 2019 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 +#include +#include + +#include ELFUTILS_HEADER(elf) +#include + + +int +main (int argc, const char *argv[]) +{ + /* Takes the given file, and create a new identical one. */ + if (argc != 2) + { + fprintf (stderr, "elfrdwrnop elf-file\n"); + exit (1); + } + + elf_version (EV_CURRENT); + + const char *name = argv[1]; + printf ("elfrdwrdnop %s\n", name); + + int fd = open (name, O_RDWR); + if (fd < 0) + { + fprintf (stderr, "Couldn't open file '%s': %s\n", + name, strerror (errno)); + exit (1); + } + + Elf *elf = elf_begin (fd, ELF_C_RDWR, NULL); + if (elf == NULL) + { + fprintf (stderr, "Couldn't open ELF file '%s': %s\n", + name, elf_errmsg (-1)); + exit (1); + } + + /* Write everything to disk. If there are any phdrs, then we want + the exact same layout. */ + size_t phnum; + if (elf_getphdrnum (elf, &phnum) != 0) + { + printf ("cannot get phdrs: %s\n", elf_errmsg (-1)); + exit (1); + } + + if (phnum > 0) + elf_flagelf (elf, ELF_C_SET, ELF_F_LAYOUT); + + if (elf_update (elf, ELF_C_WRITE) < 0) + { + printf ("failure in elf_update: %s\n", elf_errmsg (-1)); + exit (1); + } + + if (elf_end (elf) != 0) + { + printf ("couldn't cleanup elf '%s': %s\n", name, elf_errmsg (-1)); + exit (1); + } + + if (close (fd) != 0) + { + printf ("couldn't close '%s': %s\n", name, strerror (errno)); + exit (1); + } + + return 0; +} Index: elfutils-0.176/tests/run-reverse-sections-self.sh =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ elfutils-0.176/tests/run-reverse-sections-self.sh 2019-05-21 14:44:38.000000000 +0200 @@ -0,0 +1,45 @@ +#! /bin/sh +# Copyright (C) 2019 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 . + +. $srcdir/test-subr.sh + +test_reverse_self () +{ + in_file="$1" + base_name="$(basename ${in_file})" + out_file="${base_name}.rev" + out_file_mmap="${out_file}.mmap" + + tempfiles ${out_file} ${out_file_mmap} + + # Reverse the offsets (the files should still be the same otherwise) + testrun ${abs_builddir}/elfcopy --reverse-offs ${in_file} ${out_file} + testrun ${abs_top_builddir}/src/elfcmp ${in_file} ${out_file} + testrun ${abs_top_builddir}/src/elflint --gnu ${out_file} + # An in-place nop will likely revert them back + testrun ${abs_builddir}/elfrdwrnop ${out_file} + testrun ${abs_top_builddir}/src/elfcmp ${in_file} ${out_file} + testrun ${abs_top_builddir}/src/elflint --gnu ${out_file} +} + +# Only really makes sense for ET_REL files, but try all, just to check +# it also works if we keep the order for the allocated sections. +for file in $self_test_files; do + test_reverse_self $file +done + +exit 0 Index: elfutils-0.176/tests/run-reverse-sections.sh =================================================================== --- /dev/null 1970-01-01 00:00:00.000000000 +0000 +++ elfutils-0.176/tests/run-reverse-sections.sh 2019-05-21 14:44:38.000000000 +0200 @@ -0,0 +1,69 @@ +#! /bin/sh +# Copyright (C) 2019 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 . + +. $srcdir/test-subr.sh + +test_reverse () +{ + in_file="$1" + out_file="${in_file}.rev" + out_file_mmap="${out_file}.mmap" + + testfiles ${in_file} + tempfiles ${out_file} ${out_file_mmap} + + # Reverse the offsets (the files should still be the same otherwise) + testrun ${abs_builddir}/elfcopy --reverse-offs ${in_file} ${out_file} + testrun ${abs_top_builddir}/src/elfcmp ${in_file} ${out_file} + testrun ${abs_top_builddir}/src/elflint --gnu ${out_file} + # An in-place nop will likely revert them back + testrun ${abs_builddir}/elfrdwrnop ${out_file} + testrun ${abs_top_builddir}/src/elfcmp ${in_file} ${out_file} + testrun ${abs_top_builddir}/src/elflint --gnu ${out_file} +} + +# A collection of random testfiles to test 32/64bit, little/big endian +# and non-ET_REL (with phdrs)/ET_REL (without phdrs). + +# 32bit, big endian, rel +test_reverse testfile29 + +# 64bit, big endian, rel +test_reverse testfile23 + +# 32bit, little endian, rel +test_reverse testfile9 + +# 64bit, little endian, rel +test_reverse testfile38 + +# 32bit, big endian, non-rel +test_reverse testfile26 + +# 64bit, big endian, non-rel +test_reverse testfile27 + +# 32bit, little endian, non-rel +test_reverse testfile + +# 64bit, little endian, non-rel +# Don't use testfile10. It has section headers in the middle of the file. +# Same for testfile12. It is legal, but not the point of this testcase. +# test_reverse testfile10 +test_reverse testfile13 + +exit 0