Fix SIGBUS crash when copying data

We have the following memory alignment-related issue, that seems
to be architecture/compiler/memcpy(3) specific.

Within struct tBTIF_CONTEXT_SWITCH_CBACK, the beginning of the
zero-length array "char p_param[]" is not aligned (because of the
struct internals).
However, this p_param pointer is casted within function
btif_gattc_deep_copy() to the struct pointer (btif_adv_data_t *).
By definition, the memory pointed to by such pointer is suppose
to be aligned:

    btif_adv_data_t *dst = (btif_adv_data_t*) p_dest;

It seems that on some architectures/compilers the executed memcpy()
instructions are optimized for such memory alignment.
If the memory was not aligned, we get SIGBUS.

Apparently, just using (void *) casting for the memcpy() destination,
avoids using the optimized memory aligned instructions:
  memcpy((void *)dst, src, ...);

The solutions are twofold:
 * Make sure that "char p_param[]" within struct
   tBTIF_CONTEXT_SWITCH_CBACK is aligned. Otherwise, the casting
   to "(btif_adv_data_t*)" can be problematic.
 * Add (void *) casting to all memcpy() calls which might be
   referring to such mis-aligned memory.
   This is done by using the new macro maybe_non_aligned_memcpy()
   in all places that such casting might be needed.

Either solution is sufficient to prevent the crash as identified in
this particular case. We need to apply both solutions, to reduce the
chance of running again into a similar issue.

Bug: 25601669
Change-Id: I6c49645c00f10c594a5d1e53a9fac202c506657c

diff --git a/btif/include/btif_common.h b/btif/include/btif_common.h
index d85ef3c..50c2ea5 100644 (file)
--- a/btif/include/btif_common.h
+++ b/btif/include/btif_common.h
@@ -43,6 +43,22 @@
 #define BTIF_SIG_CB_BIT   (0x8000)
 #define BTIF_SIG_CB_START(id)    (((id) << 8) | BTIF_SIG_CB_BIT)
 
+/*
+ * A memcpy(3) wrapper when copying memory that might not be aligned.
+ *
+ * On certain architectures, if the memcpy(3) arguments appear to be
+ * pointing to aligned memory (e.g., struct pointers), the compiler might
+ * generate optimized memcpy(3) code. However, if the original memory was not
+ * aligned (e.g., because of incorrect "char *" to struct pointer casting),
+ * the result code might trigger SIGBUS crash.
+ *
+ * As a short-term solution, we use the help of the maybe_non_aligned_memcpy()
+ * macro to identify and fix such cases. In the future, we should fix the
+ * problematic "char *" to struct pointer casting, and this macro itself should
+ * be removed.
+ */
+#define maybe_non_aligned_memcpy(_a, _b, _c) memcpy((void *)(_a), (_b), (_c))
+
 /* BTIF sub-systems */
 #define BTIF_CORE           0
 #define BTIF_DM             1
@@ -174,7 +190,7 @@ typedef struct
 
     /* parameters passed to callback */
     UINT16               event;   /* message event id */
-    char                 p_param[0]; /* parameter area needs to be last */
+    char __attribute__ ((aligned)) p_param[]; /* parameter area needs to be last */
 } tBTIF_CONTEXT_SWITCH_CBACK;
 
 

diff --git a/btif/src/btif_av.c b/btif/src/btif_av.c
index 412a158..251badc 100644 (file)
--- a/btif/src/btif_av.c
+++ b/btif/src/btif_av.c
@@ -892,7 +892,7 @@ void btif_av_event_deep_copy(UINT16 event, char *p_dest, char *p_src)
     tBTA_AV *av_dest = (tBTA_AV *)p_dest;
 
     // First copy the structure
-    memcpy(p_dest, p_src, sizeof(tBTA_AV));
+    maybe_non_aligned_memcpy(av_dest, av_src, sizeof(*av_src));
 
     switch (event)
     {

diff --git a/btif/src/btif_dm.c b/btif/src/btif_dm.c
index ccb9988..f84bb4a 100644 (file)
--- a/btif/src/btif_dm.c
+++ b/btif/src/btif_dm.c
@@ -241,7 +241,7 @@ static void btif_dm_data_copy(uint16_t event, char *dst, char *src)
         return;
 
     assert(dst_dm_sec);
-    memcpy(dst_dm_sec, src_dm_sec, sizeof(tBTA_DM_SEC));
+    maybe_non_aligned_memcpy(dst_dm_sec, src_dm_sec, sizeof(*src_dm_sec));
 
     if (event == BTA_DM_BLE_KEY_EVT)
     {
@@ -777,7 +777,7 @@ static void search_devices_copy_cb(UINT16 event, char *p_dest, char *p_src)
         return;
 
     BTIF_TRACE_DEBUG("%s: event=%s", __FUNCTION__, dump_dm_search_event(event));
-    memcpy(p_dest_data, p_src_data, sizeof(tBTA_DM_SEARCH));
+    maybe_non_aligned_memcpy(p_dest_data, p_src_data, sizeof(*p_src_data));
     switch (event)
     {
         case BTA_DM_INQ_RES_EVT:
@@ -810,7 +810,7 @@ static void search_services_copy_cb(UINT16 event, char *p_dest, char *p_src)
 
     if (!p_src)
         return;
-    memcpy(p_dest_data, p_src_data, sizeof(tBTA_DM_SEARCH));
+    maybe_non_aligned_memcpy(p_dest_data, p_src_data, sizeof(*p_src_data));
     switch (event)
     {
          case BTA_DM_DISC_RES_EVT:

diff --git a/btif/src/btif_gatt_client.c b/btif/src/btif_gatt_client.c
index 89e0d77..9ae88a6 100644 (file)
--- a/btif/src/btif_gatt_client.c
+++ b/btif/src/btif_gatt_client.c
@@ -313,7 +313,7 @@ static void btapp_gattc_req_data(UINT16 event, char *p_dest, char *p_src)
        return;
 
     // Copy basic structure first
-    memcpy(p_dest_data, p_src_data, sizeof(tBTA_GATTC));
+    maybe_non_aligned_memcpy(p_dest_data, p_src_data, sizeof(*p_src_data));
 
     // Allocate buffer for request data if necessary
     switch (event)
@@ -1831,7 +1831,7 @@ static void btif_gattc_deep_copy(UINT16 event, char *p_dest, char *p_src)
         {
             const btif_adv_data_t *src = (btif_adv_data_t*) p_src;
             btif_adv_data_t *dst = (btif_adv_data_t*) p_dest;
-            memcpy(dst, src, sizeof(*src));
+            maybe_non_aligned_memcpy(dst, src, sizeof(*src));
 
             if (src->p_manufacturer_data)
             {

diff --git a/btif/src/btif_gatt_server.c b/btif/src/btif_gatt_server.c
index 01122c2..6cbe06f 100644 (file)
--- a/btif/src/btif_gatt_server.c
+++ b/btif/src/btif_gatt_server.c
@@ -124,7 +124,7 @@ static void btapp_gatts_copy_req_data(UINT16 event, char *p_dest, char *p_src)
         return;
 
     // Copy basic structure first
-    memcpy(p_dest_data, p_src_data, sizeof(tBTA_GATTS));
+    maybe_non_aligned_memcpy(p_dest_data, p_src_data, sizeof(*p_src_data));
 
     // Allocate buffer for request data if necessary
     switch (event)

diff --git a/btif/src/btif_mce.c b/btif/src/btif_mce.c
index fd73b0a..b1fb03a 100644 (file)
--- a/btif/src/btif_mce.c
+++ b/btif/src/btif_mce.c
@@ -86,7 +86,7 @@ static void mas_discovery_comp_copy_cb(UINT16 event, char *p_dest, char *p_src)
     if (event != BTA_MCE_MAS_DISCOVERY_COMP_EVT)
         return;
 
-    memcpy(p_dest_data, p_src_data, sizeof(tBTA_MCE_MAS_DISCOVERY_COMP));
+    maybe_non_aligned_memcpy(p_dest_data, p_src_data, sizeof(*p_src_data));
 
     p_dest_str = p_dest + sizeof(tBTA_MCE_MAS_DISCOVERY_COMP);
 

diff --git a/btif/src/btif_sdp.c b/btif/src/btif_sdp.c
index 08980e9..21cf684 100644 (file)
--- a/btif/src/btif_sdp.c
+++ b/btif/src/btif_sdp.c
@@ -87,7 +87,7 @@ static void sdp_search_comp_copy_cb(UINT16 event, char *p_dest, char *p_src)
     if (event != BTA_SDP_SEARCH_COMP_EVT)
         return;
 
-    memcpy(p_dest_data, p_src_data, sizeof(tBTA_SDP_SEARCH_COMP));
+    maybe_non_aligned_memcpy(p_dest_data, p_src_data, sizeof(*p_src_data));
 
     copy_sdp_records(p_src_data->records, p_dest_data->records, p_src_data->record_count);
 }