From bf406fdca0b4d1d8973b47461756b66ef9c726ee Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Tue, 25 Sep 2012 21:31:57 +0200 Subject: libqmi-glib,utils: handle alignment issues when reading integers from the buffer Some architectures require that the value of a pointer is aligned in memory. Given that we're reading from a raw buffer, the integers in it may not end up aligned so we cannot safely cast any address to a valid 16/32/64 bit integer value. Handle this by copying the integer from the raw buffer directly into the output variable, which is of course properly aligned. Also added new test cases to check this. Thanks to: Shawn J. Goff for reporting the issue and his endless tests. --- libqmi-glib/qmi-utils.c | 101 +++++++++++--- libqmi-glib/test/test-utils.c | 300 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 374 insertions(+), 27 deletions(-) diff --git a/libqmi-glib/qmi-utils.c b/libqmi-glib/qmi-utils.c index 5b3ddf8..2273aad 100644 --- a/libqmi-glib/qmi-utils.c +++ b/libqmi-glib/qmi-utils.c @@ -21,6 +21,7 @@ * Copyright (C) 2012 Aleksander Morgado */ +#include #include #include #include @@ -61,6 +62,29 @@ qmi_utils_str_hex (gconstpointer mem, return new_str; } +#if defined UTILS_ENABLE_TRACE +static void +print_read_bytes_trace (const gchar *type, + gconstpointer buffer, + gconstpointer out, + guint n_bytes) +{ + gchar *str1; + gchar *str2; + + str1 = qmi_utils_str_hex (buffer, n_bytes, ':'); + str2 = qmi_utils_str_hex (out, n_bytes, ':'); + + g_debug ("Read %s (%s) --> (%s)", type, str1, str2); + g_warn_if_fail (g_str_equal (str1, str2)); + + g_free (str1); + g_free (str2); +} +#else +#define print_read_bytes_trace(...) +#endif + void qmi_utils_read_guint8_from_buffer (const guint8 **buffer, guint16 *buffer_size, @@ -73,6 +97,8 @@ qmi_utils_read_guint8_from_buffer (const guint8 **buffer, *out = (*buffer)[0]; + print_read_bytes_trace ("guint8", &(*buffer)[0], out, 1); + *buffer = &((*buffer)[1]); *buffer_size = (*buffer_size) - 1; } @@ -87,7 +113,9 @@ qmi_utils_read_gint8_from_buffer (const guint8 **buffer, g_assert (buffer_size != NULL); g_assert (*buffer_size >= 1); - *out = *((gint8 *)(&((*buffer)[0]))); + *out = (gint8)(*buffer)[0]; + + print_read_bytes_trace ("gint8", &(*buffer)[0], out, 1); *buffer = &((*buffer)[1]); *buffer_size = (*buffer_size) - 1; @@ -103,7 +131,10 @@ qmi_utils_read_guint16_from_buffer (const guint8 **buffer, g_assert (buffer_size != NULL); g_assert (*buffer_size >= 2); - *out = GUINT16_FROM_LE (*((guint16 *)&((*buffer)[0]))); + memcpy (out, &((*buffer)[0]), 2); + *out = GUINT16_FROM_LE (*out); + + print_read_bytes_trace ("guint16", &(*buffer)[0], out, 2); *buffer = &((*buffer)[2]); *buffer_size = (*buffer_size) - 2; @@ -119,7 +150,10 @@ qmi_utils_read_gint16_from_buffer (const guint8 **buffer, g_assert (buffer_size != NULL); g_assert (*buffer_size >= 2); - *out = GINT16_FROM_LE (*((guint16 *)&((*buffer)[0]))); + memcpy (out, &((*buffer)[0]), 2); + *out = GINT16_FROM_LE (*out); + + print_read_bytes_trace ("gint16", &(*buffer)[0], out, 2); *buffer = &((*buffer)[2]); *buffer_size = (*buffer_size) - 2; @@ -135,7 +169,10 @@ qmi_utils_read_guint32_from_buffer (const guint8 **buffer, g_assert (buffer_size != NULL); g_assert (*buffer_size >= 4); - *out = GUINT32_FROM_LE (*((guint32 *)&((*buffer)[0]))); + memcpy (out, &((*buffer)[0]), 4); + *out = GUINT32_FROM_LE (*out); + + print_read_bytes_trace ("guint32", &(*buffer)[0], out, 4); *buffer = &((*buffer)[4]); *buffer_size = (*buffer_size) - 4; @@ -151,7 +188,10 @@ qmi_utils_read_gint32_from_buffer (const guint8 **buffer, g_assert (buffer_size != NULL); g_assert (*buffer_size >= 4); - *out = GUINT32_FROM_LE (*((guint32 *)&((*buffer)[0]))); + memcpy (out, &((*buffer)[0]), 4); + *out = GINT32_FROM_LE (*out); + + print_read_bytes_trace ("gint32", &(*buffer)[0], out, 4); *buffer = &((*buffer)[4]); *buffer_size = (*buffer_size) - 4; @@ -167,7 +207,10 @@ qmi_utils_read_guint64_from_buffer (const guint8 **buffer, g_assert (buffer_size != NULL); g_assert (*buffer_size >= 8); - *out = GUINT64_FROM_LE (*((guint64 *)&((*buffer)[0]))); + memcpy (out, &((*buffer)[0]), 8); + *out = GUINT64_FROM_LE (*out); + + print_read_bytes_trace ("guint64", &(*buffer)[0], out, 8); *buffer = &((*buffer)[8]); *buffer_size = (*buffer_size) - 8; @@ -183,7 +226,10 @@ qmi_utils_read_gint64_from_buffer (const guint8 **buffer, g_assert (buffer_size != NULL); g_assert (*buffer_size >= 8); - *out = GUINT64_FROM_LE (*((guint64 *)&((*buffer)[0]))); + memcpy (out, &((*buffer)[0]), 8); + *out = GINT64_FROM_LE (*out); + + print_read_bytes_trace ("gint64", &(*buffer)[0], out, 8); *buffer = &((*buffer)[8]); *buffer_size = (*buffer_size) - 8; @@ -195,15 +241,14 @@ qmi_utils_read_sized_guint_from_buffer (const guint8 **buffer, guint n_bytes, guint64 *out) { - guint64 tmp = 0; - g_assert (out != NULL); g_assert (buffer != NULL); g_assert (buffer_size != NULL); g_assert (*buffer_size >= n_bytes); - memcpy (&tmp, *buffer, n_bytes); - *out = GUINT64_FROM_LE (tmp); + *out = 0; + memcpy (out, *buffer, n_bytes); + *out = GUINT64_FROM_LE (*out); *buffer = &((*buffer)[n_bytes]); *buffer_size = (*buffer_size) - n_bytes; @@ -219,7 +264,7 @@ qmi_utils_write_guint8_to_buffer (guint8 **buffer, g_assert (buffer_size != NULL); g_assert (*buffer_size >= 1); - (*buffer)[0] = *in; + memcpy (&(*buffer)[0], in, sizeof (*in)); *buffer = &((*buffer)[1]); *buffer_size = (*buffer_size) - 1; @@ -235,7 +280,7 @@ qmi_utils_write_gint8_to_buffer (guint8 **buffer, g_assert (buffer_size != NULL); g_assert (*buffer_size >= 1); - *((gint8 *)(&((*buffer)[0]))) = *in; + memcpy (&(*buffer)[0], in, sizeof (*in)); *buffer = &((*buffer)[1]); *buffer_size = (*buffer_size) - 1; @@ -246,12 +291,15 @@ qmi_utils_write_guint16_to_buffer (guint8 **buffer, guint16 *buffer_size, guint16 *in) { + guint16 tmp; + g_assert (in != NULL); g_assert (buffer != NULL); g_assert (buffer_size != NULL); g_assert (*buffer_size >= 2); - *((guint16 *)(&((*buffer)[0]))) = GUINT16_TO_LE (*in); + tmp = GUINT16_TO_LE (*in); + memcpy (&(*buffer)[0], &tmp, sizeof (tmp)); *buffer = &((*buffer)[2]); *buffer_size = (*buffer_size) - 2; @@ -262,12 +310,15 @@ qmi_utils_write_gint16_to_buffer (guint8 **buffer, guint16 *buffer_size, gint16 *in) { + gint16 tmp; + g_assert (in != NULL); g_assert (buffer != NULL); g_assert (buffer_size != NULL); g_assert (*buffer_size >= 2); - *((gint16 *)(&((*buffer)[0]))) = GINT16_TO_LE (*in); + tmp = GINT16_TO_LE (*in); + memcpy (&(*buffer)[0], &tmp, sizeof (tmp)); *buffer = &((*buffer)[2]); *buffer_size = (*buffer_size) - 2; @@ -278,12 +329,15 @@ qmi_utils_write_guint32_to_buffer (guint8 **buffer, guint16 *buffer_size, guint32 *in) { + guint32 tmp; + g_assert (in != NULL); g_assert (buffer != NULL); g_assert (buffer_size != NULL); g_assert (*buffer_size >= 4); - *((guint32 *)(&((*buffer)[0]))) = GUINT32_TO_LE (*in); + tmp = GUINT32_TO_LE (*in); + memcpy (&(*buffer)[0], &tmp, sizeof (tmp)); *buffer = &((*buffer)[4]); *buffer_size = (*buffer_size) - 4; @@ -294,12 +348,15 @@ qmi_utils_write_gint32_to_buffer (guint8 **buffer, guint16 *buffer_size, gint32 *in) { + gint32 tmp; + g_assert (in != NULL); g_assert (buffer != NULL); g_assert (buffer_size != NULL); g_assert (*buffer_size >= 4); - *((gint32 *)(&((*buffer)[0]))) = GINT32_TO_LE (*in); + tmp = GINT32_TO_LE (*in); + memcpy (&(*buffer)[0], &tmp, sizeof (tmp)); *buffer = &((*buffer)[4]); *buffer_size = (*buffer_size) - 4; @@ -310,12 +367,15 @@ qmi_utils_write_guint64_to_buffer (guint8 **buffer, guint16 *buffer_size, guint64 *in) { + guint64 tmp; + g_assert (in != NULL); g_assert (buffer != NULL); g_assert (buffer_size != NULL); g_assert (*buffer_size >= 8); - *((guint64 *)(&((*buffer)[0]))) = GUINT64_TO_LE (*in); + tmp = GUINT64_TO_LE (*in); + memcpy (&(*buffer)[0], &tmp, sizeof (tmp)); *buffer = &((*buffer)[8]); *buffer_size = (*buffer_size) - 8; @@ -326,12 +386,15 @@ qmi_utils_write_gint64_to_buffer (guint8 **buffer, guint16 *buffer_size, gint64 *in) { + gint64 tmp; + g_assert (in != NULL); g_assert (buffer != NULL); g_assert (buffer_size != NULL); g_assert (*buffer_size >= 8); - *((gint64 *)(&((*buffer)[0]))) = GINT64_TO_LE (*in); + tmp = GINT64_TO_LE (*in); + memcpy (&(*buffer)[0], &tmp, sizeof (tmp)); *buffer = &((*buffer)[8]); *buffer_size = (*buffer_size) - 8; diff --git a/libqmi-glib/test/test-utils.c b/libqmi-glib/test/test-utils.c index e328642..0e91cc8 100644 --- a/libqmi-glib/test/test-utils.c +++ b/libqmi-glib/test/test-utils.c @@ -149,6 +149,80 @@ test_utils_int16 (void) g_assert (memcmp (in_buffer, out_buffer, sizeof (in_buffer)) == 0); } +static void +test_utils_uint16_unaligned (void) +{ + static const guint8 in_buffer[9] = { + 0x00, 0x0F, 0x50, 0xEB, 0xE2, 0xB6, 0x00, 0x00, 0x00 + }; + static guint16 values[4] = { + 0x500F, 0xE2EB, 0x00B6, 0x0000 + }; + guint8 out_buffer[8] = { 0 }; + + guint i; + guint16 in_buffer_size; + const guint8 *in_buffer_walker; + guint16 out_buffer_size; + guint8 *out_buffer_walker; + + in_buffer_size = sizeof (in_buffer) - 1; + in_buffer_walker = &in_buffer[1]; + out_buffer_size = sizeof (out_buffer); + out_buffer_walker = &out_buffer[0]; + i = 0; + + while (in_buffer_size) { + guint16 tmp; + + qmi_utils_read_guint16_from_buffer (&in_buffer_walker, &in_buffer_size, &tmp); + + g_assert (tmp == values[i++]); + + qmi_utils_write_guint16_to_buffer (&out_buffer_walker, &out_buffer_size, &tmp); + } + + g_assert_cmpuint (out_buffer_size, ==, 0); + g_assert (memcmp (&in_buffer[1], out_buffer, sizeof (in_buffer) - 1) == 0); +} + +static void +test_utils_int16_unaligned (void) +{ + static const guint8 in_buffer[9] = { + 0x00, 0x0F, 0x50, 0xEB, 0xE2, 0xB6, 0x00, 0x00, 0x00 + }; + static gint16 values[4] = { + 0x500F, 0xE2EB, 0x00B6, 0x0000 + }; + guint8 out_buffer[8] = { 0 }; + + guint i; + guint16 in_buffer_size; + const guint8 *in_buffer_walker; + guint16 out_buffer_size; + guint8 *out_buffer_walker; + + in_buffer_size = sizeof (in_buffer) - 1; + in_buffer_walker = &in_buffer[1]; + out_buffer_size = sizeof (out_buffer); + out_buffer_walker = &out_buffer[0]; + i = 0; + + while (in_buffer_size) { + gint16 tmp; + + qmi_utils_read_gint16_from_buffer (&in_buffer_walker, &in_buffer_size, &tmp); + + g_assert (tmp == values[i++]); + + qmi_utils_write_gint16_to_buffer (&out_buffer_walker, &out_buffer_size, &tmp); + } + + g_assert_cmpuint (out_buffer_size, ==, 0); + g_assert (memcmp (&in_buffer[1], out_buffer, sizeof (in_buffer) - 1) == 0); +} + static void test_utils_uint32 (void) { @@ -223,6 +297,80 @@ test_utils_int32 (void) g_assert (memcmp (in_buffer, out_buffer, sizeof (in_buffer)) == 0); } +static void +test_utils_uint32_unaligned (void) +{ + static const guint8 in_buffer[9] = { + 0x00, 0x0F, 0x50, 0xEB, 0xE2, 0xB6, 0x00, 0x00, 0x00 + }; + static guint32 values[2] = { + 0xE2EB500F, 0x000000B6 + }; + guint8 out_buffer[8] = { 0 }; + + guint i; + guint16 in_buffer_size; + const guint8 *in_buffer_walker; + guint16 out_buffer_size; + guint8 *out_buffer_walker; + + in_buffer_size = sizeof (in_buffer) - 1; + in_buffer_walker = &in_buffer[1]; + out_buffer_size = sizeof (out_buffer); + out_buffer_walker = &out_buffer[0]; + i = 0; + + while (in_buffer_size) { + guint32 tmp; + + qmi_utils_read_guint32_from_buffer (&in_buffer_walker, &in_buffer_size, &tmp); + + g_assert (tmp == values[i++]); + + qmi_utils_write_guint32_to_buffer (&out_buffer_walker, &out_buffer_size, &tmp); + } + + g_assert_cmpuint (out_buffer_size, ==, 0); + g_assert (memcmp (&in_buffer[1], out_buffer, sizeof (in_buffer) - 1) == 0); +} + +static void +test_utils_int32_unaligned (void) +{ + static const guint8 in_buffer[9] = { + 0x00, 0x0F, 0x50, 0xEB, 0xE2, 0xB6, 0x00, 0x00, 0x00 + }; + static gint32 values[2] = { + 0xE2EB500F, 0x000000B6 + }; + guint8 out_buffer[8] = { 0 }; + + guint i; + guint16 in_buffer_size; + const guint8 *in_buffer_walker; + guint16 out_buffer_size; + guint8 *out_buffer_walker; + + in_buffer_size = sizeof (in_buffer) - 1; + in_buffer_walker = &in_buffer[1]; + out_buffer_size = sizeof (out_buffer); + out_buffer_walker = &out_buffer[0]; + i = 0; + + while (in_buffer_size) { + gint32 tmp; + + qmi_utils_read_gint32_from_buffer (&in_buffer_walker, &in_buffer_size, &tmp); + + g_assert (tmp == values[i++]); + + qmi_utils_write_gint32_to_buffer (&out_buffer_walker, &out_buffer_size, &tmp); + } + + g_assert_cmpuint (out_buffer_size, ==, 0); + g_assert (memcmp (&in_buffer[1], out_buffer, sizeof (in_buffer) - 1) == 0); +} + static void test_utils_uint64 (void) { @@ -297,6 +445,80 @@ test_utils_int64 (void) g_assert (memcmp (in_buffer, out_buffer, sizeof (in_buffer)) == 0); } +static void +test_utils_uint64_unaligned (void) +{ + static const guint8 in_buffer[9] = { + 0x00, 0x0F, 0x50, 0xEB, 0xE2, 0xB6, 0x00, 0x00, 0x00 + }; + static guint64 values[1] = { + 0x000000B6E2EB500FULL + }; + guint8 out_buffer[8] = { 0 }; + + guint i; + guint16 in_buffer_size; + const guint8 *in_buffer_walker; + guint16 out_buffer_size; + guint8 *out_buffer_walker; + + in_buffer_size = sizeof (in_buffer) - 1; + in_buffer_walker = &in_buffer[1]; + out_buffer_size = sizeof (out_buffer); + out_buffer_walker = &out_buffer[0]; + i = 0; + + while (in_buffer_size) { + guint64 tmp; + + qmi_utils_read_guint64_from_buffer (&in_buffer_walker, &in_buffer_size, &tmp); + + g_assert (tmp == values[i++]); + + qmi_utils_write_guint64_to_buffer (&out_buffer_walker, &out_buffer_size, &tmp); + } + + g_assert_cmpuint (out_buffer_size, ==, 0); + g_assert (memcmp (&in_buffer[1], out_buffer, sizeof (in_buffer) - 1) == 0); +} + +static void +test_utils_int64_unaligned (void) +{ + static const guint8 in_buffer[9] = { + 0x00, 0x0F, 0x50, 0xEB, 0xE2, 0xB6, 0x00, 0x00, 0x00 + }; + static gint64 values[1] = { + 0x000000B6E2EB500FLL + }; + guint8 out_buffer[8] = { 0 }; + + guint i; + guint16 in_buffer_size; + const guint8 *in_buffer_walker; + guint16 out_buffer_size; + guint8 *out_buffer_walker; + + in_buffer_size = sizeof (in_buffer) - 1; + in_buffer_walker = &in_buffer[1]; + out_buffer_size = sizeof (out_buffer); + out_buffer_walker = &out_buffer[0]; + i = 0; + + while (in_buffer_size) { + gint64 tmp; + + qmi_utils_read_gint64_from_buffer (&in_buffer_walker, &in_buffer_size, &tmp); + + g_assert (tmp == values[i++]); + + qmi_utils_write_gint64_to_buffer (&out_buffer_walker, &out_buffer_size, &tmp); + } + + g_assert_cmpuint (out_buffer_size, ==, 0); + g_assert (memcmp (&in_buffer[1], out_buffer, sizeof (in_buffer) - 1) == 0); +} + static void common_test_utils_uint_sized (guint n_bytes) { @@ -349,6 +571,58 @@ test_utils_uint_sized (void) } } +static void +common_test_utils_uint_sized_unaligned (guint n_bytes) +{ + static const guint8 in_buffer[9] = { + 0x00, 0x0F, 0x50, 0xEB, 0xE2, 0xB6, 0x00, 0x00, 0x00 + }; + guint64 value = 0x000000B6E2EB500FULL; + guint8 expected_out_buffer[8] = { 0 }; + guint8 out_buffer[8] = { 0 }; + + guint64 tmp; + guint i; + guint16 in_buffer_size; + const guint8 *in_buffer_walker; + guint16 out_buffer_size; + guint8 *out_buffer_walker; + + /* Build expected intermediate value */ + tmp = 0xFF; + for (i = 1; i < n_bytes; i++) { + tmp <<= 8; + tmp |= 0xFF; + } + value &= tmp; + + /* Build expected output buffer */ + memcpy (expected_out_buffer, &in_buffer[1], n_bytes); + + in_buffer_size = sizeof (in_buffer) - 1; + in_buffer_walker = &in_buffer[1]; + out_buffer_size = sizeof (out_buffer); + out_buffer_walker = &out_buffer[0]; + i = 0; + + qmi_utils_read_sized_guint_from_buffer (&in_buffer_walker, &in_buffer_size, n_bytes, &tmp); + g_assert (tmp == value); + qmi_utils_write_sized_guint_to_buffer (&out_buffer_walker, &out_buffer_size, n_bytes, &tmp); + + g_assert_cmpuint (out_buffer_size, ==, 8 - n_bytes); + g_assert (memcmp (expected_out_buffer, out_buffer, sizeof (expected_out_buffer)) == 0); +} + +static void +test_utils_uint_sized_unaligned (void) +{ + guint i; + + for (i = 1; i <= 8; i++) { + common_test_utils_uint_sized_unaligned (i); + } +} + int main (int argc, char **argv) { g_type_init (); @@ -356,14 +630,24 @@ int main (int argc, char **argv) g_test_add_func ("/libqmi-glib/utils/uint8", test_utils_uint8); g_test_add_func ("/libqmi-glib/utils/int8", test_utils_int8); - g_test_add_func ("/libqmi-glib/utils/uint16", test_utils_uint16); - g_test_add_func ("/libqmi-glib/utils/int16", test_utils_int16); - g_test_add_func ("/libqmi-glib/utils/uint32", test_utils_uint32); - g_test_add_func ("/libqmi-glib/utils/int32", test_utils_int32); - g_test_add_func ("/libqmi-glib/utils/uint64", test_utils_uint64); - g_test_add_func ("/libqmi-glib/utils/int64", test_utils_int64); - - g_test_add_func ("/libqmi-glib/utils/uint-sized", test_utils_uint_sized); + + g_test_add_func ("/libqmi-glib/utils/uint16", test_utils_uint16); + g_test_add_func ("/libqmi-glib/utils/int16", test_utils_int16); + g_test_add_func ("/libqmi-glib/utils/uint16-unaligned", test_utils_uint16_unaligned); + g_test_add_func ("/libqmi-glib/utils/int16-unaligned", test_utils_int16_unaligned); + + g_test_add_func ("/libqmi-glib/utils/uint32", test_utils_uint32); + g_test_add_func ("/libqmi-glib/utils/int32", test_utils_int32); + g_test_add_func ("/libqmi-glib/utils/uint32/unaligned", test_utils_uint32_unaligned); + g_test_add_func ("/libqmi-glib/utils/int32/unaligned", test_utils_int32_unaligned); + + g_test_add_func ("/libqmi-glib/utils/uint64", test_utils_uint64); + g_test_add_func ("/libqmi-glib/utils/int64", test_utils_int64); + g_test_add_func ("/libqmi-glib/utils/uint64/unaligned", test_utils_uint64_unaligned); + g_test_add_func ("/libqmi-glib/utils/int64/unaligned", test_utils_int64_unaligned); + + g_test_add_func ("/libqmi-glib/utils/uint-sized", test_utils_uint_sized); + g_test_add_func ("/libqmi-glib/utils/uint-sized/unaligned", test_utils_uint_sized_unaligned); return g_test_run (); } -- cgit v1.2.3