libelf: Get alignment correct when calling conversion functions.

When writing out data that needs to be converted we have to make sure
the conversion function is called on correctly aligned buffers. When
using mmap this might mean we have to convert into a temporarily buffer
if the user wants to write out the section at a location that is not
correctly aligned for the section type.

Older gas would generate misaligned ELF notes for the .version
directive. When copying over such notes using mmap from files with
a different endianness using mmap we would get the alignment of the
conversion destination wrong.

The new testcase would fail with configure --enable-sanitize-undefined
on little endian systems. The GCC undefinited sanitizer caught a similar
issue with testfile1 on big endian systems.

gelf_xlate.h:47:1: runtime error: member access within misaligned address
0x7f8145d770d5 for type 'struct Elf32_Nhdr', which requires 4 byte alignment

Signed-off-by: Mark Wielaard <mark@klomp.org>

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 68c4fbdde491bcf1fb3c6904684c62f6a99ae321..5923c85c0846050e9a02eccb92fb04e29c9d6792 100644 (file)
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,8 @@
+2018-11-17  Mark Wielaard  <mark@klomp.org>
+
+       * elf32_updatefile.c (updatemmap): Make sure to call convert
+       function on a properly aligned destination.
+
 2018-11-16  Mark Wielaard  <mark@klomp.org>
 
        * libebl.h (__elf32_msize): Mark with const attribute.

diff --git a/libelf/elf32_updatefile.c b/libelf/elf32_updatefile.c
index f2e9a2889e459e5731a5cae198fe9b7063423622..284bacc909b20e10f4e6fb4fe5fe97baf83219e5 100644 (file)
--- a/libelf/elf32_updatefile.c
+++ b/libelf/elf32_updatefile.c
@@ -1,5 +1,5 @@
 /* Write changed data structures.
-   Copyright (C) 2000-2010, 2014, 2015, 2016 Red Hat, Inc.
+   Copyright (C) 2000-2010, 2014, 2015, 2016, 2018 Red Hat, Inc.
    This file is part of elfutils.
    Written by Ulrich Drepper <drepper@redhat.com>, 2000.
 
@@ -354,7 +354,9 @@ __elfw2(LIBELFBITS,updatemmap) (Elf *elf, int change_bo, size_t shnum)
                       user's section data with the latest one, rather than
                       crashing.  */
 
-                   if (unlikely (change_bo))
+                   if (unlikely (change_bo
+                                 && dl->data.d.d_size != 0
+                                 && dl->data.d.d_type != ELF_T_BYTE))
                      {
 #if EV_NUM != 2
                        xfct_t fctp;
@@ -364,9 +366,33 @@ __elfw2(LIBELFBITS,updatemmap) (Elf *elf, int change_bo, size_t shnum)
 # define fctp __elf_xfctstom[0][EV_CURRENT - 1][ELFW(ELFCLASS, LIBELFBITS) - 1][dl->data.d.d_type]
 #endif
 
-                       /* Do the real work.  */
-                       (*fctp) (last_position, dl->data.d.d_buf,
-                                dl->data.d.d_size, 1);
+                       size_t align;
+                       align = __libelf_type_align (ELFW(ELFCLASS,LIBELFBITS),
+                                                    dl->data.d.d_type);
+                       if ((((uintptr_t) last_position)
+                            & (uintptr_t) (align - 1)) == 0)
+                         {
+                           /* No need to copy, we can convert directly.  */
+                           (*fctp) (last_position, dl->data.d.d_buf,
+                                    dl->data.d.d_size, 1);
+                         }
+                       else
+                         {
+                           /* We have to do the conversion on properly
+                              aligned memory first.  */
+                           size_t size = dl->data.d.d_size;
+                           char *converted = aligned_alloc (align, size);
+                           if (converted == NULL)
+                             {
+                               __libelf_seterrno (ELF_E_NOMEM);
+                               return 1;
+                             }
+                            (*fctp) (converted, dl->data.d.d_buf, size, 1);
+
+                           /* And then write it to the mmapped file.  */
+                           memcpy (last_position, converted, size);
+                           free (converted);
+                         }
 
                        last_position += dl->data.d.d_size;
                      }

diff --git a/tests/ChangeLog b/tests/ChangeLog
index 514229b76044ed4aa69b9e37dccabc431907611a..08358d2997a377da5351978100062c7f3995e972 100644 (file)
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,10 @@
+2018-11-17  Mark Wielaard  <mark@klomp.org>
+
+       * run-strip-version.sh: New test.
+       * testfile-version.bz2: New test file.
+       * Makefile.am (TESTS): Add run-strip-version.sh.
+       (EXTRA_DIST): Add run-strip-version.sh and testfile-version.bz2.
+
 2018-11-09  Mark Wielaard  <mark@klomp.org>
 
        * run-strip-reloc.sh: Also test testfile-debug-rel-ppc64-z.o

diff --git a/tests/Makefile.am b/tests/Makefile.am
index ac467d93adc2e732a290d4babf73260b435e264b..3ca0e1c22aa28c8a4d9da69e6371a93aedad362e 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -158,7 +158,8 @@ TESTS = run-arextract.sh run-arsymtest.sh run-ar.sh newfile test-nlist \
        run-reloc-bpf.sh \
        run-next-cfi.sh run-next-cfi-self.sh \
        run-copyadd-sections.sh run-copymany-sections.sh \
-       run-typeiter-many.sh run-strip-test-many.sh
+       run-typeiter-many.sh run-strip-test-many.sh \
+       run-strip-version.sh
 
 if !BIARCH
 export ELFUTILS_DISABLE_BIARCH = 1
@@ -417,7 +418,8 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
             run-typeiter-many.sh run-strip-test-many.sh \
             testfile-debug-rel-ppc64-g.o.bz2 \
             testfile-debug-rel-ppc64-z.o.bz2 \
-            testfile-debug-rel-ppc64.o.bz2
+            testfile-debug-rel-ppc64.o.bz2 \
+            run-strip-version.sh testfile-version.bz2
 
 if USE_VALGRIND
 valgrind_cmd='valgrind -q --leak-check=full --error-exitcode=1'

diff --git a/tests/run-strip-version.sh b/tests/run-strip-version.sh
new file mode 100755 (executable)
index 0000000..191e0aa
--- /dev/null
+++ b/tests/run-strip-version.sh
@@ -0,0 +1,58 @@
+#! /bin/sh
+# Copyright (C) 2018 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
+
+# Generated on s390x with an older gas (2.29.1) that generates
+# badly aligned version notes.
+#
+# = testfile-version.s =
+#
+#      .section ".extra"
+#      .byte 42
+#
+#      .version "Sliding Snow"
+#      .version "Hurr durr 3.1"
+#
+#        .globl  _start
+#_start:
+#
+# gcc -nostartfiles -nodefaultlibs -o testfile-version testfile-version.s
+
+testfiles testfile-version
+tempfiles debug.out elf.out
+
+testrun ${abs_top_builddir}/src/strip -o elf.out -f debug.out \
+       testfile-version
+
+testrun ${abs_top_builddir}/src/elflint --gnu elf.out
+testrun ${abs_top_builddir}/src/elflint --gnu --debug debug.out
+
+testrun_compare ${abs_top_builddir}/src/readelf -n debug.out <<\EOF
+
+Note section [ 1] '.note.gnu.build-id' of 36 bytes at offset 0xb0:
+  Owner          Data size  Type
+  GNU                   20  GNU_BUILD_ID
+    Build ID: d3c84c0b307c06f50a37c6c0f59c82c4cb10720b
+
+Note section [ 3] '.note' of 56 bytes at offset 0xd5:
+  Owner          Data size  Type
+  Sliding Snow           0  VERSION
+  Hurr durr 3.1          0  VERSION
+EOF
+
+exit 0

diff --git a/tests/testfile-version.bz2 b/tests/testfile-version.bz2
new file mode 100755 (executable)
index 0000000..b2c0c0c
Binary files /dev/null and b/tests/testfile-version.bz2 differ