libsframe asan: avoid generating misaligned loads

There are two places where unaligned loads were seen on aarch64:
  - #1. access to the SFrame FRE stack offsets in the in-memory
    representation/abstraction provided by libsframe.
  - #2. access to the SFrame FRE start address in the on-disk representation
    of the frame row entry.

For #1, we can fix this by reordering the struct members of
sframe_frame_row_entry in libsframe/sframe-api.h.

For #2, we need to default to using memcpy instead, and copy out the bytes
to a location for output.

SFrame format is an unaligned on-disk format. As such, there are other blobs
of memory in the on-disk SFrame FRE that are on not on their natural
boundaries.  But that does not pose further problems yet, because the users
are provided access to the on-disk SFrame FRE data via libsframe's
sframe_frame_row_entry, the latter has its' struct members aligned on their
respective natural boundaries (and initialized using memcpy).

PR 29856 libsframe asan: load misaligned at sframe.c:516

ChangeLog:

	PR libsframe/29856
	* bfd/elf64-x86-64.c: Adjust as the struct members have been
	reordered.
	* libsframe/sframe.c (sframe_decode_fre_start_address): Use
	memcpy to perform 16-bit/32-bit reads.
	* libsframe/testsuite/libsframe.encode/encode-1.c: Adjust as the
	struct members have been reordered.

include/ChangeLog:

	PR libsframe/29856
	* sframe-api.h: Reorder fre_offsets for natural alignment.

diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index afc8c76c52bb82c30d3fa4db3ac379595f95ed1d..8cf733d89e03c726ba624b1a8903d92bd00a7cb9 100644 (file)
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -822,48 +822,48 @@ static const bfd_byte elf_x86_64_eh_frame_non_lazy_plt[] =
 static const sframe_frame_row_entry elf_x86_64_sframe_null_fre =
 {
   0,
-  SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_1B), /* FRE info.  */
-  {16, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} /* 12 bytes.  */
+  {16, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, /* 12 bytes.  */
+  SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_1B) /* FRE info.  */
 };
 
 /* .sframe FRE covering the .plt section entry.  */
 static const sframe_frame_row_entry elf_x86_64_sframe_plt0_fre1 =
 {
   0, /* SFrame FRE start address.  */
-  SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_1B), /* FRE info.  */
-  {16, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} /* 12 bytes.  */
+  {16, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, /* 12 bytes.  */
+  SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_1B) /* FRE info.  */
 };
 
 /* .sframe FRE covering the .plt section entry.  */
 static const sframe_frame_row_entry elf_x86_64_sframe_plt0_fre2 =
 {
   6, /* SFrame FRE start address.  */
-  SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_1B), /* FRE info.  */
-  {24, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} /* 12 bytes.  */
+  {24, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, /* 12 bytes.  */
+  SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_1B) /* FRE info.  */
 };
 
 /* .sframe FRE covering the .plt section entry.  */
 static const sframe_frame_row_entry elf_x86_64_sframe_pltn_fre1 =
 {
   0, /* SFrame FRE start address.  */
-  SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_1B), /* FRE info.  */
-  {8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} /* 12 bytes.  */
+  {8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, /* 12 bytes.  */
+  SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_1B) /* FRE info.  */
 };
 
 /* .sframe FRE covering the .plt section entry.  */
 static const sframe_frame_row_entry elf_x86_64_sframe_pltn_fre2 =
 {
   11, /* SFrame FRE start address.  */
-  SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_1B), /* FRE info.  */
-  {16, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} /* 12 bytes.  */
+  {16, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, /* 12 bytes.  */
+  SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_1B) /* FRE info.  */
 };
 
 /* .sframe FRE covering the second .plt section entry.  */
 static const sframe_frame_row_entry elf_x86_64_sframe_sec_pltn_fre1 =
 {
   0, /* SFrame FRE start address.  */
-  SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_1B), /* FRE info.  */
-  {8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} /* 12 bytes.  */
+  {8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, /* 12 bytes.  */
+  SFRAME_V1_FRE_INFO (SFRAME_BASE_REG_SP, 1, SFRAME_FRE_OFFSET_1B) /* FRE info.  */
 };
 
 /* SFrame helper object for non-lazy PLT.  Also used for IBT enabled PLT.  */

diff --git a/include/sframe-api.h b/include/sframe-api.h
index 0a86389857c820117f1061c2b7e0defa8e590b6c..c9db39eaaf01b859806a0a6865769a0a50807983 100644 (file)
--- a/include/sframe-api.h
+++ b/include/sframe-api.h
@@ -34,13 +34,17 @@ typedef struct sframe_encoder_ctx sframe_encoder_ctx;
 
 /* User interfacing SFrame Row Entry.
    An abstraction provided by libsframe so the consumer is decoupled from
-   the binary format representation of the same.  */
+   the binary format representation of the same.
+
+   The members are best ordered such that they are aligned at their natural
+   boundaries.  This helps avoid usage of undesirable misaligned memory
+   accesses.  See PR libsframe/29856.  */
 
 typedef struct sframe_frame_row_entry
 {
   uint32_t fre_start_addr;
-  unsigned char fre_info;
   unsigned char fre_offsets[MAX_OFFSET_BYTES];
+  unsigned char fre_info;
 } sframe_frame_row_entry;
 
 #define SFRAME_ERR ((int) -1)

diff --git a/libsframe/sframe.c b/libsframe/sframe.c
index d4eaaee2297ca9fd92ea9f16e6b6ef7a67e30c8e..b17d3234236eff1a00cf9450ec55c0858b1dfc43 100644 (file)
--- a/libsframe/sframe.c
+++ b/libsframe/sframe.c
@@ -652,6 +652,11 @@ sframe_frame_row_entry_copy (sframe_frame_row_entry *dst, sframe_frame_row_entry
   return 0;
 }
 
+/* Decode the SFrame FRE start address offset value from FRE_BUF in on-disk
+   binary format, given the FRE_TYPE.  Updates the FRE_START_ADDR.
+
+   Returns 0 on success, SFRAME_ERR otherwise.  */
+
 static int
 sframe_decode_fre_start_address (const char *fre_buf,
                                 uint32_t *fre_start_addr,
@@ -659,6 +664,9 @@ sframe_decode_fre_start_address (const char *fre_buf,
 {
   uint32_t saddr = 0;
   int err = 0;
+  size_t addr_size = 0;
+
+  addr_size = sframe_fre_start_addr_size (fre_type);
 
   if (fre_type == SFRAME_FRE_TYPE_ADDR1)
     {
@@ -668,12 +676,18 @@ sframe_decode_fre_start_address (const char *fre_buf,
   else if (fre_type == SFRAME_FRE_TYPE_ADDR2)
     {
       uint16_t *ust = (uint16_t *)fre_buf;
-      saddr = (uint32_t)*ust;
+      /* SFrame is an unaligned on-disk format.  Using memcpy helps avoid the
+        use of undesirable unaligned loads.  See PR libsframe/29856.  */
+      uint16_t tmp = 0;
+      memcpy (&tmp, ust, addr_size);
+      saddr = (uint32_t)tmp;
     }
   else if (fre_type == SFRAME_FRE_TYPE_ADDR4)
     {
       uint32_t *uit = (uint32_t *)fre_buf;
-      saddr = (uint32_t)*uit;
+      int32_t tmp = 0;
+      memcpy (&tmp, uit, addr_size);
+      saddr = (uint32_t)tmp;
     }
   else
     return sframe_set_errno (&err, SFRAME_ERR_INVAL);

diff --git a/libsframe/testsuite/libsframe.encode/encode-1.c b/libsframe/testsuite/libsframe.encode/encode-1.c
index 01481106a62f65089b26ef6cc2f4ac56a4aa9023..0f5225ff9ec3eb58a2fc6f59de8eab3bfcedfb1f 100644 (file)
--- a/libsframe/testsuite/libsframe.encode/encode-1.c
+++ b/libsframe/testsuite/libsframe.encode/encode-1.c
@@ -33,10 +33,10 @@ add_fde1 (sframe_encoder_ctx *encode, int idx)
   int i, err;
   /* A contiguous block containing 4 FREs.  */
   sframe_frame_row_entry fres[]
-    = { {0x0, 0x3, {0x8, 0, 0}},
-       {0x1, 0x5, {0x10, 0xf0, 0}},
-       {0x4, 0x4, {0x10, 0xf0, 0}},
-       {0x1a, 0x5, {0x8, 0xf0, 0}}
+    = { {0x0, {0x8, 0, 0}, 0x3},
+       {0x1, {0x10, 0xf0, 0}, 0x5},
+       {0x4, {0x10, 0xf0, 0}, 0x4},
+       {0x1a, {0x8, 0xf0, 0}, 0x5}
       };
 
   unsigned char finfo = sframe_fde_create_func_info (SFRAME_FRE_TYPE_ADDR1,
@@ -58,10 +58,10 @@ add_fde2 (sframe_encoder_ctx *encode, int idx)
   int i, err;
   /* A contiguous block containing 4 FREs.  */
   sframe_frame_row_entry fres[]
-    = { {0x0, 0x3, {0x8, 0, 0}},
-       {0x1, 0x5, {0x10, 0xf0, 0}},
-       {0x4, 0x4, {0x10, 0xf0, 0}},
-       {0xf, 0x5, {0x8, 0xf0, 0}}
+    = { {0x0, {0x8, 0, 0}, 0x3},
+       {0x1, {0x10, 0xf0, 0}, 0x5},
+       {0x4, {0x10, 0xf0, 0}, 0x4},
+       {0xf, {0x8, 0xf0, 0}, 0x5}
       };
 
   unsigned char finfo = sframe_fde_create_func_info (SFRAME_FRE_TYPE_ADDR1,