[PATCH] libelf: Don't return unaligned data returned from elf_getdata[_rawchunk].

[PATCH] libelf: Don't return unaligned data returned from elf_getdata[_rawchunk].

[Mark Wielaard]

For i386 and x86_64 we allow some unaligned data accesses.
We also return unaligned data from elf_getdata[_rawchunk].
But that might go wrong if we then access the ELF types inside.
When build with gcc -O3 for example the compiler might vectorize
loops accessing ELF words or types. The instructions used do require
the data is naturally aligned. If the function returnes unaligned
data the program will segfault and crash. This happens for example
with the code in dwfl_module_getdwarf.c that tries to iterate over
the hash buckets gotten through elf_getdata_rawchunk based on the
DT_[GNU]_HASH value.

This only happens when the underlying ELF file is mmapped, and it
is meant as optimization so that we don't have to copy data first
so that it is correctly aligned. In most cases the data is already
naturally aligned though. But it might not be for non-native ELF
files.

Given that it might even happen in our own code base and these
are public functions that can be used by code that might rely on
the data returned being correctly aligned for the ELF data type
requested just always return correctly aligned data.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 libelf/ChangeLog              | 7 +++++++
 libelf/elf_getdata.c          | 5 +----
 libelf/elf_getdata_rawchunk.c | 3 +--
 libelf/libelfP.h              | 4 ----
 4 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index ea2b9df..c7286b1 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,10 @@
+2018-06-19  Mark Wielaard  <mark@klomp.org>
+
+	* libelfP.h (__libelf_type_align): Remove !ALLOW_UNALIGNED guard.
+	* elf_getdata.c (__libelf_type_aligns): Likewise.
+	(convert_data): Remove ALLOW_UNALIGNED check.
+	* elf_getdata_rawchunk.c (elf_getdata_rawchunk): Likewise.
+
 2018-04-19  Andreas Schwab  <schwab@suse.de>
 
 	* elf.h: Update from glibc.
diff --git a/libelf/elf_getdata.c b/libelf/elf_getdata.c
index 97c503b..278dfa8 100644
--- a/libelf/elf_getdata.c
+++ b/libelf/elf_getdata.c
@@ -76,7 +76,6 @@ static const Elf_Type shtype_map[EV_NUM - 1][TYPEIDX (SHT_HISUNW) + 1] =
     }
   };
 
-#if !ALLOW_UNALIGNED
 /* Associate libelf types with their internal alignment requirements.  */
 const uint_fast8_t __libelf_type_aligns[EV_NUM - 1][ELFCLASSNUM - 1][ELF_T_NUM] =
   {
@@ -115,7 +114,6 @@ const uint_fast8_t __libelf_type_aligns[EV_NUM - 1][ELFCLASSNUM - 1][ELF_T_NUM]
     }
 # undef TYPE_ALIGNS
   };
-#endif
 
 
 Elf_Type
@@ -173,8 +171,7 @@ convert_data (Elf_Scn *scn, int version __attribute__ ((unused)), int eclass,
       /* Make sure the source is correctly aligned for the conversion
 	 function to directly access the data elements.  */
       char *rawdata_source;
-      if (ALLOW_UNALIGNED ||
-	  ((((size_t) (char *) scn->rawdata_base)) & (align - 1)) == 0)
+      if (((((size_t) (char *) scn->rawdata_base)) & (align - 1)) == 0)
 	rawdata_source = scn->rawdata_base;
       else
 	{
diff --git a/libelf/elf_getdata_rawchunk.c b/libelf/elf_getdata_rawchunk.c
index 31b2fe7..d0c0b75 100644
--- a/libelf/elf_getdata_rawchunk.c
+++ b/libelf/elf_getdata_rawchunk.c
@@ -80,8 +80,7 @@ elf_getdata_rawchunk (Elf *elf, off_t offset, size_t size, Elf_Type type)
     {
     /* If the file is mmap'ed we can use it directly, if aligned for type.  */
       char *rawdata = elf->map_address + elf->start_offset + offset;
-      if (ALLOW_UNALIGNED ||
-	  ((uintptr_t) rawdata & (align - 1)) == 0)
+      if (((uintptr_t) rawdata & (align - 1)) == 0)
 	rawchunk = rawdata;
       else
 	{
diff --git a/libelf/libelfP.h b/libelf/libelfP.h
index ca805ac..ed216c8 100644
--- a/libelf/libelfP.h
+++ b/libelf/libelfP.h
@@ -443,15 +443,11 @@ extern int __libelf_version_initialized attribute_hidden;
 # define LIBELF_EV_IDX	(__libelf_version - 1)
 #endif
 
-#if !ALLOW_UNALIGNED
 /* Array with alignment requirements of the internal types indexed by ELF
    version, binary class, and type. */
 extern const uint_fast8_t __libelf_type_aligns[EV_NUM - 1][ELFCLASSNUM - 1][ELF_T_NUM] attribute_hidden;
 # define __libelf_type_align(class, type)	\
     (__libelf_type_aligns[LIBELF_EV_IDX][class - 1][type] ?: 1)
-#else
-# define __libelf_type_align(class, type)	1
-#endif
 
 /* Given an Elf handle and a section type returns the Elf_Data d_type.
    Should not be called when SHF_COMPRESSED is set, the d_type should
-- 
1.8.3.1

Re: [PATCH] libelf: Don't return unaligned data returned from elf_getdata[_rawchunk].

[Mark Wielaard]

On Wed, 2018-06-20 at 00:57 +0200, Mark Wielaard wrote:
> For i386 and x86_64 we allow some unaligned data accesses.
> We also return unaligned data from elf_getdata[_rawchunk].
> But that might go wrong if we then access the ELF types inside.
> When build with gcc -O3 for example the compiler might vectorize
> loops accessing ELF words or types. The instructions used do require
> the data is naturally aligned. If the function returnes unaligned
> data the program will segfault and crash. This happens for example
> with the code in dwfl_module_getdwarf.c that tries to iterate over
> the hash buckets gotten through elf_getdata_rawchunk based on the
> DT_[GNU]_HASH value.
> 
> This only happens when the underlying ELF file is mmapped, and it
> is meant as optimization so that we don't have to copy data first
> so that it is correctly aligned. In most cases the data is already
> naturally aligned though. But it might not be for non-native ELF
> files.
> 
> Given that it might even happen in our own code base and these
> are public functions that can be used by code that might rely on
> the data returned being correctly aligned for the ELF data type
> requested just always return correctly aligned data.

Pushed to master.