From 4c6da48c655fa65fb2c1bdb70a9c0e60a17cdfeb Mon Sep 17 00:00:00 2001 From: lateralusX Date: Fri, 18 Mar 2022 12:15:02 +0100 Subject: [PATCH] Fix unaligned UTF16 string read in collect tracing EventPipe command. Collect tracing 2 EventPipe command triggers an unaligned UTF16 string read that could cause a SIGBUS on platforms not supporting unalinged reads of UTF16 strings (so far only seen on 32-bit ARM Linux CI machine). On CoreCLR this could even cause a unalinged int read due to optimizations used in UTF8Encoding::GetByteCount. --- src/native/eventpipe/ds-eventpipe-protocol.c | 28 ++++-- src/native/eventpipe/ds-protocol.c | 98 ++++++++++++++++---- src/native/eventpipe/ds-protocol.h | 7 ++ 3 files changed, 108 insertions(+), 25 deletions(-) diff --git a/src/native/eventpipe/ds-eventpipe-protocol.c b/src/native/eventpipe/ds-eventpipe-protocol.c index 6405e42b0f67ff..36c3588b2a3db8 100644 --- a/src/native/eventpipe/ds-eventpipe-protocol.c +++ b/src/native/eventpipe/ds-eventpipe-protocol.c @@ -140,7 +140,7 @@ eventpipe_collect_tracing_command_try_parse_rundown_requested ( EP_ASSERT (buffer_len != NULL); EP_ASSERT (rundown_requested != NULL); - return ds_ipc_message_try_parse_value (buffer, buffer_len, (uint8_t *)rundown_requested, (uint32_t)sizeof (bool)); + return ds_ipc_message_try_parse_value (buffer, buffer_len, (uint8_t *)rundown_requested, 1); } static @@ -160,6 +160,9 @@ eventpipe_collect_tracing_command_try_parse_config ( const uint32_t max_count_configs = 1000; uint32_t count_configs = 0; + uint8_t *provider_name_byte_array = NULL; + uint8_t *filter_data_byte_array = NULL; + ep_char8_t *provider_name_utf8 = NULL; ep_char8_t *filter_data_utf8 = NULL; @@ -176,20 +179,27 @@ eventpipe_collect_tracing_command_try_parse_config ( ep_raise_error_if_nok (ds_ipc_message_try_parse_uint32_t (buffer, buffer_len, &log_level)); ep_raise_error_if_nok (log_level <= EP_EVENT_LEVEL_VERBOSE); - const ep_char16_t *provider_name = NULL; - ep_raise_error_if_nok (ds_ipc_message_try_parse_string_utf16_t (buffer, buffer_len, &provider_name)); + uint32_t provider_name_byte_array_len = 0; + ep_raise_error_if_nok (ds_ipc_message_try_parse_string_utf16_t_byte_array_alloc (buffer, buffer_len, &provider_name_byte_array, &provider_name_byte_array_len)); - provider_name_utf8 = ep_rt_utf16_to_utf8_string (provider_name, -1); + provider_name_utf8 = ep_rt_utf16_to_utf8_string ((const ep_char16_t *)provider_name_byte_array, -1); ep_raise_error_if_nok (provider_name_utf8 != NULL); ep_raise_error_if_nok (!ep_rt_utf8_string_is_null_or_empty (provider_name_utf8)); - const ep_char16_t *filter_data = NULL; // This parameter is optional. - ds_ipc_message_try_parse_string_utf16_t (buffer, buffer_len, &filter_data); + ep_rt_byte_array_free (provider_name_byte_array); + provider_name_byte_array = NULL; + + uint32_t filter_data_byte_array_len = 0; + ep_raise_error_if_nok (ds_ipc_message_try_parse_string_utf16_t_byte_array_alloc (buffer, buffer_len, &filter_data_byte_array, &filter_data_byte_array_len)); - if (filter_data) { - filter_data_utf8 = ep_rt_utf16_to_utf8_string (filter_data, -1); + // This parameter is optional. + if (filter_data_byte_array) { + filter_data_utf8 = ep_rt_utf16_to_utf8_string ((const ep_char16_t *)filter_data_byte_array, -1); ep_raise_error_if_nok (filter_data_utf8 != NULL); + + ep_rt_byte_array_free (filter_data_byte_array); + filter_data_byte_array = NULL; } EventPipeProviderConfiguration provider_config; @@ -209,7 +219,9 @@ eventpipe_collect_tracing_command_try_parse_config ( ep_on_error: count_configs = 0; + ep_rt_byte_array_free (provider_name_byte_array); ep_rt_utf8_string_free (provider_name_utf8); + ep_rt_byte_array_free (filter_data_byte_array); ep_rt_utf8_string_free (filter_data_utf8); ep_exit_error_handler (); } diff --git a/src/native/eventpipe/ds-protocol.c b/src/native/eventpipe/ds-protocol.c index 606376449ad9ba..a49b741013885d 100644 --- a/src/native/eventpipe/ds-protocol.c +++ b/src/native/eventpipe/ds-protocol.c @@ -59,6 +59,14 @@ ipc_message_flatten ( uint16_t payload_len, ds_ipc_flatten_payload_func flatten_payload); +static +bool +ipc_message_try_parse_string_utf16_t_byte_array ( + uint8_t **buffer, + uint32_t *buffer_len, + const uint8_t **string_byte_array, + uint32_t *string_byte_array_len); + /* * DiagnosticsIpc */ @@ -306,6 +314,50 @@ ipc_message_flatten ( ep_exit_error_handler (); } +static +bool +ipc_message_try_parse_string_utf16_t_byte_array ( + uint8_t **buffer, + uint32_t *buffer_len, + const uint8_t **string_byte_array, + uint32_t *string_byte_array_len) +{ + EP_ASSERT (buffer != NULL); + EP_ASSERT (buffer_len != NULL); + EP_ASSERT (string_byte_array != NULL); + EP_ASSERT (string_byte_array_len != NULL); + + bool result = false; + + ep_raise_error_if_nok (ds_ipc_message_try_parse_uint32_t (buffer, buffer_len, string_byte_array_len)); + *string_byte_array_len *= sizeof (ep_char16_t); + + if (*string_byte_array_len != 0) { + if (*string_byte_array_len > *buffer_len) + ep_raise_error (); + + if (((const ep_char16_t *)*buffer) [(*string_byte_array_len / sizeof (ep_char16_t)) - 1] != 0) + ep_raise_error (); + + *string_byte_array = *buffer; + + } else { + *string_byte_array = NULL; + } + + *buffer = *buffer + *string_byte_array_len; + *buffer_len = *buffer_len - *string_byte_array_len; + + result = true; + +ep_on_exit: + return result; + +ep_on_error: + EP_ASSERT (!result); + ep_exit_error_handler (); +} + DiagnosticsIpcMessage * ds_ipc_message_init (DiagnosticsIpcMessage *message) { @@ -383,38 +435,35 @@ ds_ipc_message_try_parse_uint32_t ( return result; } -// TODO: Strings are in little endian format in buffer. bool -ds_ipc_message_try_parse_string_utf16_t ( +ds_ipc_message_try_parse_string_utf16_t_byte_array_alloc ( uint8_t **buffer, uint32_t *buffer_len, - const ep_char16_t **value) + uint8_t **string_byte_array, + uint32_t *string_byte_array_len) { EP_ASSERT (buffer != NULL); EP_ASSERT (buffer_len != NULL); - EP_ASSERT (value != NULL); + EP_ASSERT (string_byte_array != NULL); + EP_ASSERT (string_byte_array_len != NULL); bool result = false; - uint32_t string_len = 0; - ep_raise_error_if_nok (ds_ipc_message_try_parse_uint32_t (buffer, buffer_len, &string_len)); + const uint8_t *temp_buffer = NULL; + uint32_t temp_buffer_len = 0; - if (string_len != 0) { - if (string_len > (*buffer_len / sizeof (ep_char16_t))) - ep_raise_error (); - - if (((const ep_char16_t *)*buffer) [string_len - 1] != 0) - ep_raise_error (); + ep_raise_error_if_nok (ipc_message_try_parse_string_utf16_t_byte_array (buffer, buffer_len, (const uint8_t **)&temp_buffer, &temp_buffer_len)); - *value = (ep_char16_t *)*buffer; + if (temp_buffer_len != 0) { + *string_byte_array = ep_rt_byte_array_alloc (temp_buffer_len); + ep_raise_error_if_nok (*string_byte_array != NULL); + memcpy (*string_byte_array, temp_buffer, temp_buffer_len); } else { - *value = NULL; + *string_byte_array = NULL; } - *buffer = *buffer + (string_len * sizeof (ep_char16_t)); - *buffer_len = *buffer_len - (string_len * sizeof (ep_char16_t)); - + *string_byte_array_len = temp_buffer_len; result = true; ep_on_exit: @@ -425,6 +474,21 @@ ds_ipc_message_try_parse_string_utf16_t ( ep_exit_error_handler (); } +bool +ds_ipc_message_try_parse_string_utf16_t ( + uint8_t **buffer, + uint32_t *buffer_len, + const ep_char16_t **value) +{ + EP_ASSERT (buffer != NULL); + EP_ASSERT (buffer_len != NULL); + EP_ASSERT (value != NULL); + EP_ASSERT (!(((size_t)*buffer) & 0x1)); + + uint32_t string_byte_array_len = 0; + return ipc_message_try_parse_string_utf16_t_byte_array (buffer, buffer_len, (const uint8_t **)value, &string_byte_array_len); +} + bool ds_ipc_message_initialize_header_uint32_t_payload ( DiagnosticsIpcMessage *message, diff --git a/src/native/eventpipe/ds-protocol.h b/src/native/eventpipe/ds-protocol.h index 137fb7fd1fcc50..4f6e317044da40 100644 --- a/src/native/eventpipe/ds-protocol.h +++ b/src/native/eventpipe/ds-protocol.h @@ -145,6 +145,13 @@ ds_ipc_message_try_parse_int32_t ( return ds_ipc_message_try_parse_uint32_t (buffer, buffer_len, (uint32_t *)value); } +bool +ds_ipc_message_try_parse_string_utf16_t_byte_array_alloc ( + uint8_t **buffer, + uint32_t *buffer_len, + uint8_t **string_byte_array, + uint32_t *string_byte_array_len); + bool ds_ipc_message_try_parse_string_utf16_t ( uint8_t **buffer,