[PATCH] libdwfl: Don't dereference possibly unaligned auxv entry pointer from core.

[PATCH] libdwfl: Don't dereference possibly unaligned auxv entry pointer from core.

[Mark Wielaard]

The notes in a core file that contain the auxv entries might not be
naturally aligned. The code already tried to account for that, but
the GCC 8 undefined behaviour sanitizer found we were till dereferencing
the actual auxv entry pointer directly. Fix this by calculating all pointers
by hand and not use an array of auxv entries trick.

This makes make distcheck (which enables sanitize-undefined by default)
pass again using GCC8.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 libdwfl/ChangeLog  | 12 ++++++++++++
 libdwfl/link_map.c | 38 +++++++++++++++++++-------------------
 2 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 112620f..d487ec1 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,15 @@
+2017-11-20  Mark Wielaard  <mark@klomp.org>
+
+	* link_map.c (do_check64): Take a char * and calculate type and val
+	offsets before reading, possibly unaligned, values.
+	(do_check32): Likewise.
+	(check64): Remove define.
+	(check32): Likewise.
+	(auxv_format_probe): Call do_check32 and do_check64 directly with
+	a, possibly unaligned, auxv entry pointer.
+	(dwfl_link_map_report): Redefine AUXV_SCAN to not dereference a
+	possibly unaligned auxv entry pointer.
+
 2017-10-16  Mark Wielaard  <mark@klomp.org>
 
 	* argp-std.c (parse_opt): For -k call argp_failure not failure to
diff --git a/libdwfl/link_map.c b/libdwfl/link_map.c
index 794668f..29307c7 100644
--- a/libdwfl/link_map.c
+++ b/libdwfl/link_map.c
@@ -43,13 +43,14 @@
 
 
 static inline bool
-do_check64 (size_t i, const Elf64_auxv_t (*a64)[], uint_fast8_t *elfdata)
+do_check64 (const char *a64, uint_fast8_t *elfdata)
 {
   /* The AUXV pointer might not even be naturally aligned for 64-bit
      data, because note payloads in a core file are not aligned.  */
-
-  uint64_t type = read_8ubyte_unaligned_noncvt (&(*a64)[i].a_type);
-  uint64_t val = read_8ubyte_unaligned_noncvt (&(*a64)[i].a_un.a_val);
+  const char *typep = a64 + offsetof (Elf64_auxv_t, a_type);
+  uint64_t type = read_8ubyte_unaligned_noncvt (typep);
+  const char *valp = a64 + offsetof (Elf64_auxv_t, a_un.a_val);
+  uint64_t val = read_8ubyte_unaligned_noncvt (valp);
 
   if (type == BE64 (PROBE_TYPE)
       && val == BE64 (PROBE_VAL64))
@@ -68,16 +69,15 @@ do_check64 (size_t i, const Elf64_auxv_t (*a64)[], uint_fast8_t *elfdata)
   return false;
 }
 
-#define check64(n) do_check64 (n, a64, elfdata)
-
 static inline bool
-do_check32 (size_t i, const Elf32_auxv_t (*a32)[], uint_fast8_t *elfdata)
+do_check32 (const char *a32, uint_fast8_t *elfdata)
 {
   /* The AUXV pointer might not even be naturally aligned for 32-bit
      data, because note payloads in a core file are not aligned.  */
-
-  uint32_t type = read_4ubyte_unaligned_noncvt (&(*a32)[i].a_type);
-  uint32_t val = read_4ubyte_unaligned_noncvt (&(*a32)[i].a_un.a_val);
+  const char *typep = a32 + offsetof (Elf32_auxv_t, a_type);
+  uint32_t type = read_4ubyte_unaligned_noncvt (typep);
+  const char *valp = a32 + offsetof (Elf32_auxv_t, a_un.a_val);
+  uint32_t val = read_4ubyte_unaligned_noncvt (valp);
 
   if (type == BE32 (PROBE_TYPE)
       && val == BE32 (PROBE_VAL32))
@@ -96,26 +96,22 @@ do_check32 (size_t i, const Elf32_auxv_t (*a32)[], uint_fast8_t *elfdata)
   return false;
 }
 
-#define check32(n) do_check32 (n, a32, elfdata)
-
 /* Examine an auxv data block and determine its format.
    Return true iff we figured it out.  */
 static bool
 auxv_format_probe (const void *auxv, size_t size,
 		   uint_fast8_t *elfclass, uint_fast8_t *elfdata)
 {
-  const Elf32_auxv_t (*a32)[size / sizeof (Elf32_auxv_t)] = (void *) auxv;
-  const Elf64_auxv_t (*a64)[size / sizeof (Elf64_auxv_t)] = (void *) auxv;
-
   for (size_t i = 0; i < size / sizeof (Elf64_auxv_t); ++i)
     {
-      if (check64 (i))
+      if (do_check64 (auxv + i * sizeof (Elf64_auxv_t), elfdata))
 	{
 	  *elfclass = ELFCLASS64;
 	  return true;
 	}
 
-      if (check32 (i * 2) || check32 (i * 2 + 1))
+      if (do_check32 (auxv + (i * 2) * sizeof (Elf32_auxv_t), elfdata)
+	  || do_check32 (auxv + (i * 2 + 1) * sizeof (Elf32_auxv_t), elfdata))
 	{
 	  *elfclass = ELFCLASS32;
 	  return true;
@@ -717,8 +713,12 @@ dwfl_link_map_report (Dwfl *dwfl, const void *auxv, size_t auxv_size,
 	  const Elf##NN##_auxv_t *av = auxv;                            \
 	  for (size_t i = 0; i < auxv_size / sizeof av[0]; ++i)         \
 	    {                                                           \
-              uint##NN##_t type = READ_AUXV##NN (&av[i].a_type);        \
-              uint##NN##_t val = BL##NN (READ_AUXV##NN (&av[i].a_un.a_val)); \
+	      const char *typep = auxv + i * sizeof (Elf##NN##_auxv_t); \
+	      typep += offsetof (Elf##NN##_auxv_t, a_type);             \
+	      uint##NN##_t type = READ_AUXV##NN (typep);                \
+	      const char *valp = auxv + i * sizeof (Elf##NN##_auxv_t);  \
+	      valp += offsetof (Elf##NN##_auxv_t, a_un.a_val);          \
+	      uint##NN##_t val = BL##NN (READ_AUXV##NN (valp));         \
 	      if (type == BL##NN (AT_ENTRY))                            \
 		entry = val;                                            \
 	      else if (type == BL##NN (AT_PHDR))                        \
-- 
1.8.3.1

Re: [PATCH] libdwfl: Don't dereference possibly unaligned auxv entry pointer from core.

[Mark Wielaard]

On Mon, 2017-11-20 at 14:15 +0100, Mark Wielaard wrote:
> The notes in a core file that contain the auxv entries might not be
> naturally aligned. The code already tried to account for that, but
> the GCC 8 undefined behaviour sanitizer found we were till dereferencing
> the actual auxv entry pointer directly. Fix this by calculating all pointers
> by hand and not use an array of auxv entries trick.
> 
> This makes make distcheck (which enables sanitize-undefined by default)
> pass again using GCC8.

Pushed this to master now.