From 7c9a154803c0293a8c0b51bbec65dc00a950989e Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Sat, 26 Mar 2016 01:33:34 +0100 Subject: [PATCH 01/14] Sanitize mallocs and frees. --- src/core/iomgr/tcp_server_posix.c | 2 +- src/core/iomgr/tcp_server_windows.c | 2 +- src/core/iomgr/udp_server.c | 2 +- src/core/profiling/basic_timers.c | 4 +- src/core/support/thd_posix.c | 1 + src/core/surface/byte_buffer.c | 6 +- src/core/tsi/fake_transport_security.c | 12 ++-- src/core/tsi/ssl_transport_security.c | 65 +++++++++---------- src/core/tsi/transport_security.c | 16 ++--- test/core/iomgr/endpoint_tests.c | 4 +- test/core/iomgr/tcp_posix_test.c | 4 +- .../network_benchmarks/low_level_ping_pong.c | 19 +++--- test/core/support/alloc_test.c | 4 +- 13 files changed, 65 insertions(+), 76 deletions(-) diff --git a/src/core/iomgr/tcp_server_posix.c b/src/core/iomgr/tcp_server_posix.c index 74ee68a6f11..413f7867be2 100644 --- a/src/core/iomgr/tcp_server_posix.c +++ b/src/core/iomgr/tcp_server_posix.c @@ -451,7 +451,7 @@ int grpc_tcp_server_add_port(grpc_tcp_server *s, const void *addr, &sockname_len)) { port = grpc_sockaddr_get_port((struct sockaddr *)&sockname_temp); if (port > 0) { - allocated_addr = malloc(addr_len); + allocated_addr = gpr_malloc(addr_len); memcpy(allocated_addr, addr, addr_len); grpc_sockaddr_set_port(allocated_addr, port); addr = allocated_addr; diff --git a/src/core/iomgr/tcp_server_windows.c b/src/core/iomgr/tcp_server_windows.c index a4abc5b9743..f9def9a19bb 100644 --- a/src/core/iomgr/tcp_server_windows.c +++ b/src/core/iomgr/tcp_server_windows.c @@ -467,7 +467,7 @@ int grpc_tcp_server_add_port(grpc_tcp_server *s, const void *addr, (struct sockaddr *)&sockname_temp, &sockname_len)) { port = grpc_sockaddr_get_port((struct sockaddr *)&sockname_temp); if (port > 0) { - allocated_addr = malloc(addr_len); + allocated_addr = gpr_malloc(addr_len); memcpy(allocated_addr, addr, addr_len); grpc_sockaddr_set_port(allocated_addr, port); addr = allocated_addr; diff --git a/src/core/iomgr/udp_server.c b/src/core/iomgr/udp_server.c index 174159170f3..15f3802dddc 100644 --- a/src/core/iomgr/udp_server.c +++ b/src/core/iomgr/udp_server.c @@ -337,7 +337,7 @@ int grpc_udp_server_add_port(grpc_udp_server *s, const void *addr, &sockname_len)) { port = grpc_sockaddr_get_port((struct sockaddr *)&sockname_temp); if (port > 0) { - allocated_addr = malloc(addr_len); + allocated_addr = gpr_malloc(addr_len); memcpy(allocated_addr, addr, addr_len); grpc_sockaddr_set_port(allocated_addr, port); addr = allocated_addr; diff --git a/src/core/profiling/basic_timers.c b/src/core/profiling/basic_timers.c index 3067f52c219..3c9557ab895 100644 --- a/src/core/profiling/basic_timers.c +++ b/src/core/profiling/basic_timers.c @@ -173,7 +173,7 @@ static void flush_logs(gpr_timer_log_list *list) { gpr_timer_log *log; while ((log = timer_log_pop_front(list)) != NULL) { write_log(log); - free(log); + gpr_free(log); } } @@ -208,7 +208,7 @@ static void init_output() { } static void rotate_log() { - gpr_timer_log *new = malloc(sizeof(*new)); + gpr_timer_log *new = gpr_malloc(sizeof(*new)); gpr_once_init(&g_once_init, init_output); new->num_entries = 0; pthread_mutex_lock(&g_mu); diff --git a/src/core/support/thd_posix.c b/src/core/support/thd_posix.c index 4d874d36566..1c703844b4d 100644 --- a/src/core/support/thd_posix.c +++ b/src/core/support/thd_posix.c @@ -81,6 +81,7 @@ int gpr_thd_new(gpr_thd_id *t, void (*thd_body)(void *arg), void *arg, thread_started = (pthread_create(&p, &attr, &thread_body, a) == 0); GPR_ASSERT(pthread_attr_destroy(&attr) == 0); if (!thread_started) { + /* don't use gpr_free, as this was allocated using malloc (see above) */ free(a); } *t = (gpr_thd_id)p; diff --git a/src/core/surface/byte_buffer.c b/src/core/surface/byte_buffer.c index fb39c4531d9..a093a37af32 100644 --- a/src/core/surface/byte_buffer.c +++ b/src/core/surface/byte_buffer.c @@ -44,7 +44,7 @@ grpc_byte_buffer *grpc_raw_byte_buffer_create(gpr_slice *slices, grpc_byte_buffer *grpc_raw_compressed_byte_buffer_create( gpr_slice *slices, size_t nslices, grpc_compression_algorithm compression) { size_t i; - grpc_byte_buffer *bb = malloc(sizeof(grpc_byte_buffer)); + grpc_byte_buffer *bb = gpr_malloc(sizeof(grpc_byte_buffer)); bb->type = GRPC_BB_RAW; bb->data.raw.compression = compression; gpr_slice_buffer_init(&bb->data.raw.slice_buffer); @@ -57,7 +57,7 @@ grpc_byte_buffer *grpc_raw_compressed_byte_buffer_create( grpc_byte_buffer *grpc_raw_byte_buffer_from_reader( grpc_byte_buffer_reader *reader) { - grpc_byte_buffer *bb = malloc(sizeof(grpc_byte_buffer)); + grpc_byte_buffer *bb = gpr_malloc(sizeof(grpc_byte_buffer)); gpr_slice slice; bb->type = GRPC_BB_RAW; bb->data.raw.compression = GRPC_COMPRESS_NONE; @@ -85,7 +85,7 @@ void grpc_byte_buffer_destroy(grpc_byte_buffer *bb) { gpr_slice_buffer_destroy(&bb->data.raw.slice_buffer); break; } - free(bb); + gpr_free(bb); } size_t grpc_byte_buffer_length(grpc_byte_buffer *bb) { diff --git a/src/core/tsi/fake_transport_security.c b/src/core/tsi/fake_transport_security.c index c0106f7a33c..86ffa6b83be 100644 --- a/src/core/tsi/fake_transport_security.c +++ b/src/core/tsi/fake_transport_security.c @@ -134,12 +134,12 @@ static void tsi_fake_frame_reset(tsi_fake_frame *frame, int needs_draining) { static int tsi_fake_frame_ensure_size(tsi_fake_frame *frame) { if (frame->data == NULL) { frame->allocated_size = frame->size; - frame->data = malloc(frame->allocated_size); + frame->data = gpr_malloc(frame->allocated_size); if (frame->data == NULL) return 0; } else if (frame->size > frame->allocated_size) { unsigned char *new_data = realloc(frame->data, frame->size); if (new_data == NULL) { - free(frame->data); + gpr_free(frame->data); frame->data = NULL; return 0; } @@ -160,7 +160,7 @@ static tsi_result fill_frame_from_bytes(const unsigned char *incoming_bytes, if (frame->needs_draining) return TSI_INTERNAL_ERROR; if (frame->data == NULL) { frame->allocated_size = TSI_FAKE_FRAME_INITIAL_ALLOCATED_SIZE; - frame->data = malloc(frame->allocated_size); + frame->data = gpr_malloc(frame->allocated_size); if (frame->data == NULL) return TSI_OUT_OF_RESOURCES; } @@ -226,7 +226,7 @@ static tsi_result bytes_to_frame(unsigned char *bytes, size_t bytes_size, } static void tsi_fake_frame_destruct(tsi_fake_frame *frame) { - if (frame->data != NULL) free(frame->data); + if (frame->data != NULL) gpr_free(frame->data); } /* --- tsi_frame_protector methods implementation. ---*/ @@ -366,7 +366,7 @@ static void fake_protector_destroy(tsi_frame_protector *self) { tsi_fake_frame_protector *impl = (tsi_fake_frame_protector *)self; tsi_fake_frame_destruct(&impl->protect_frame); tsi_fake_frame_destruct(&impl->unprotect_frame); - free(self); + gpr_free(self); } static const tsi_frame_protector_vtable frame_protector_vtable = { @@ -488,7 +488,7 @@ static void fake_handshaker_destroy(tsi_handshaker *self) { tsi_fake_handshaker *impl = (tsi_fake_handshaker *)self; tsi_fake_frame_destruct(&impl->incoming); tsi_fake_frame_destruct(&impl->outgoing); - free(self); + gpr_free(self); } static const tsi_handshaker_vtable handshaker_vtable = { diff --git a/src/core/tsi/ssl_transport_security.c b/src/core/tsi/ssl_transport_security.c index 8df582609b9..acf1b0e3c1d 100644 --- a/src/core/tsi/ssl_transport_security.c +++ b/src/core/tsi/ssl_transport_security.c @@ -148,8 +148,7 @@ static void init_openssl(void) { OpenSSL_add_all_algorithms(); num_locks = CRYPTO_num_locks(); GPR_ASSERT(num_locks > 0); - openssl_mutexes = malloc((size_t)num_locks * sizeof(gpr_mu)); - GPR_ASSERT(openssl_mutexes != NULL); + openssl_mutexes = gpr_malloc((size_t)num_locks * sizeof(gpr_mu)); for (i = 0; i < CRYPTO_num_locks(); i++) { gpr_mu_init(&openssl_mutexes[i]); } @@ -701,7 +700,7 @@ static tsi_result build_alpn_protocol_name_list( } *protocol_name_list_length += (size_t)alpn_protocols_lengths[i] + 1; } - *protocol_name_list = malloc(*protocol_name_list_length); + *protocol_name_list = gpr_malloc(*protocol_name_list_length); if (*protocol_name_list == NULL) return TSI_OUT_OF_RESOURCES; current = *protocol_name_list; for (i = 0; i < num_alpn_protocols; i++) { @@ -852,9 +851,9 @@ static tsi_result ssl_protector_unprotect( static void ssl_protector_destroy(tsi_frame_protector *self) { tsi_ssl_frame_protector *impl = (tsi_ssl_frame_protector *)self; - if (impl->buffer != NULL) free(impl->buffer); + if (impl->buffer != NULL) gpr_free(impl->buffer); if (impl->ssl != NULL) SSL_free(impl->ssl); - free(self); + gpr_free(self); } static const tsi_frame_protector_vtable frame_protector_vtable = { @@ -966,8 +965,8 @@ static tsi_result ssl_handshaker_extract_peer(tsi_handshaker *self, if (alpn_selected != NULL) { size_t i; tsi_peer_property *new_properties = - calloc(1, sizeof(tsi_peer_property) * (peer->property_count + 1)); - if (new_properties == NULL) return TSI_OUT_OF_RESOURCES; + gpr_malloc(sizeof(*new_properties) * (peer->property_count + 1)); + memset(new_properties, 0, sizeof(*new_properties) * (peer->property_count + 1)); for (i = 0; i < peer->property_count; i++) { new_properties[i] = peer->properties[i]; } @@ -975,10 +974,10 @@ static tsi_result ssl_handshaker_extract_peer(tsi_handshaker *self, TSI_SSL_ALPN_SELECTED_PROTOCOL, (const char *)alpn_selected, alpn_selected_len, &new_properties[peer->property_count]); if (result != TSI_OK) { - free(new_properties); + gpr_free(new_properties); return result; } - if (peer->properties != NULL) free(peer->properties); + if (peer->properties != NULL) gpr_free(peer->properties); peer->property_count++; peer->properties = new_properties; } @@ -992,10 +991,8 @@ static tsi_result ssl_handshaker_create_frame_protector( TSI_SSL_MAX_PROTECTED_FRAME_SIZE_UPPER_BOUND; tsi_ssl_handshaker *impl = (tsi_ssl_handshaker *)self; tsi_ssl_frame_protector *protector_impl = - calloc(1, sizeof(tsi_ssl_frame_protector)); - if (protector_impl == NULL) { - return TSI_OUT_OF_RESOURCES; - } + gpr_malloc(sizeof(*protector_impl)); + memset(protector_impl, 0, sizeof(*protector_impl)); if (max_output_protected_frame_size != NULL) { if (*max_output_protected_frame_size > @@ -1011,11 +1008,11 @@ static tsi_result ssl_handshaker_create_frame_protector( } protector_impl->buffer_size = actual_max_output_protected_frame_size - TSI_SSL_MAX_PROTECTION_OVERHEAD; - protector_impl->buffer = malloc(protector_impl->buffer_size); + protector_impl->buffer = gpr_malloc(protector_impl->buffer_size); if (protector_impl->buffer == NULL) { gpr_log(GPR_ERROR, "Could not allocated buffer for tsi_ssl_frame_protector."); - free(protector_impl); + gpr_free(protector_impl); return TSI_INTERNAL_ERROR; } @@ -1034,7 +1031,7 @@ static tsi_result ssl_handshaker_create_frame_protector( static void ssl_handshaker_destroy(tsi_handshaker *self) { tsi_ssl_handshaker *impl = (tsi_ssl_handshaker *)self; SSL_free(impl->ssl); /* The BIO objects are owned by ssl */ - free(impl); + gpr_free(impl); } static const tsi_handshaker_vtable handshaker_vtable = { @@ -1111,11 +1108,8 @@ static tsi_result create_tsi_ssl_handshaker(SSL_CTX *ctx, int is_client, SSL_set_accept_state(ssl); } - impl = calloc(1, sizeof(tsi_ssl_handshaker)); - if (impl == NULL) { - SSL_free(ssl); - return TSI_OUT_OF_RESOURCES; - } + impl = gpr_malloc(sizeof(*impl)); + memset(impl, 0, sizeof(*impl)); impl->ssl = ssl; impl->into_ssl = into_ssl; impl->from_ssl = from_ssl; @@ -1167,8 +1161,8 @@ static void ssl_client_handshaker_factory_destroy( tsi_ssl_client_handshaker_factory *impl = (tsi_ssl_client_handshaker_factory *)self; if (impl->ssl_context != NULL) SSL_CTX_free(impl->ssl_context); - if (impl->alpn_protocol_list != NULL) free(impl->alpn_protocol_list); - free(impl); + if (impl->alpn_protocol_list != NULL) gpr_free(impl->alpn_protocol_list); + gpr_free(impl); } static int client_handshaker_factory_npn_callback(SSL *ssl, unsigned char **out, @@ -1209,12 +1203,12 @@ static void ssl_server_handshaker_factory_destroy( tsi_peer_destruct(&impl->ssl_context_x509_subject_names[i]); } } - if (impl->ssl_contexts != NULL) free(impl->ssl_contexts); + if (impl->ssl_contexts != NULL) gpr_free(impl->ssl_contexts); if (impl->ssl_context_x509_subject_names != NULL) { - free(impl->ssl_context_x509_subject_names); + gpr_free(impl->ssl_context_x509_subject_names); } - if (impl->alpn_protocol_list != NULL) free(impl->alpn_protocol_list); - free(impl); + if (impl->alpn_protocol_list != NULL) gpr_free(impl->alpn_protocol_list); + gpr_free(impl); } static int does_entry_match_name(const char *entry, size_t entry_length, @@ -1333,11 +1327,8 @@ tsi_result tsi_create_ssl_client_handshaker_factory( return TSI_INVALID_ARGUMENT; } - impl = calloc(1, sizeof(tsi_ssl_client_handshaker_factory)); - if (impl == NULL) { - SSL_CTX_free(ssl_context); - return TSI_OUT_OF_RESOURCES; - } + impl = gpr_malloc(sizeof(*impl)); + memset(impl, 0, sizeof(*impl)); impl->ssl_context = ssl_context; do { @@ -1411,14 +1402,16 @@ tsi_result tsi_create_ssl_server_handshaker_factory( return TSI_INVALID_ARGUMENT; } - impl = calloc(1, sizeof(tsi_ssl_server_handshaker_factory)); - if (impl == NULL) return TSI_OUT_OF_RESOURCES; + impl = gpr_malloc(sizeof(*impl)); + memset(impl, 0, sizeof(*impl)); impl->base.create_handshaker = ssl_server_handshaker_factory_create_handshaker; impl->base.destroy = ssl_server_handshaker_factory_destroy; - impl->ssl_contexts = calloc(key_cert_pair_count, sizeof(SSL_CTX *)); + impl->ssl_contexts = gpr_malloc(key_cert_pair_count * sizeof(SSL_CTX *)); + memset(impl->ssl_contexts, 0, key_cert_pair_count * sizeof(SSL_CTX *)); impl->ssl_context_x509_subject_names = - calloc(key_cert_pair_count, sizeof(tsi_peer)); + gpr_malloc(key_cert_pair_count * sizeof(tsi_peer)); + memset(impl->ssl_context_x509_subject_names, 0, key_cert_pair_count * sizeof(tsi_peer)); if (impl->ssl_contexts == NULL || impl->ssl_context_x509_subject_names == NULL) { tsi_ssl_handshaker_factory_destroy(&impl->base); diff --git a/src/core/tsi/transport_security.c b/src/core/tsi/transport_security.c index db219a50a67..e17749d871a 100644 --- a/src/core/tsi/transport_security.c +++ b/src/core/tsi/transport_security.c @@ -239,16 +239,10 @@ void tsi_peer_destruct(tsi_peer *self) { tsi_result tsi_construct_allocated_string_peer_property( const char *name, size_t value_length, tsi_peer_property *property) { *property = tsi_init_peer_property(); - if (name != NULL) { - property->name = tsi_strdup(name); - if (property->name == NULL) return TSI_OUT_OF_RESOURCES; - } + if (name != NULL) property->name = gpr_strdup(name); if (value_length > 0) { - property->value.data = calloc(1, value_length); - if (property->value.data == NULL) { - tsi_peer_property_destruct(property); - return TSI_OUT_OF_RESOURCES; - } + property->value.data = gpr_malloc(value_length); + memset(value.data, 0, value_length); property->value.length = value_length; } return TSI_OK; @@ -276,8 +270,8 @@ tsi_result tsi_construct_string_peer_property(const char *name, tsi_result tsi_construct_peer(size_t property_count, tsi_peer *peer) { memset(peer, 0, sizeof(tsi_peer)); if (property_count > 0) { - peer->properties = calloc(property_count, sizeof(tsi_peer_property)); - if (peer->properties == NULL) return TSI_OUT_OF_RESOURCES; + peer->properties = gpr_malloc(property_count * sizeof(tsi_peer_property)); + memset(peer->properties, 0, property_count * sizeof(tsi_peer_property)); peer->property_count = property_count; } return TSI_OK; diff --git a/test/core/iomgr/endpoint_tests.c b/test/core/iomgr/endpoint_tests.c index f689e4ba7fb..cb23f58eca5 100644 --- a/test/core/iomgr/endpoint_tests.c +++ b/test/core/iomgr/endpoint_tests.c @@ -89,7 +89,7 @@ static void end_test(grpc_endpoint_test_config config) { config.clean_up(); } static gpr_slice *allocate_blocks(size_t num_bytes, size_t slice_size, size_t *num_blocks, uint8_t *current_data) { size_t nslices = num_bytes / slice_size + (num_bytes % slice_size ? 1 : 0); - gpr_slice *slices = malloc(sizeof(gpr_slice) * nslices); + gpr_slice *slices = gpr_malloc(sizeof(gpr_slice) * nslices); size_t num_bytes_left = num_bytes; size_t i; size_t j; @@ -164,7 +164,7 @@ static void read_and_write_test_write_handler(grpc_exec_ctx *exec_ctx, gpr_slice_buffer_addn(&state->outgoing, slices, nslices); grpc_endpoint_write(exec_ctx, state->write_ep, &state->outgoing, &state->done_write); - free(slices); + gpr_free(slices); return; } } diff --git a/test/core/iomgr/tcp_posix_test.c b/test/core/iomgr/tcp_posix_test.c index 4351642ab6e..b2cf87a0c25 100644 --- a/test/core/iomgr/tcp_posix_test.c +++ b/test/core/iomgr/tcp_posix_test.c @@ -97,7 +97,7 @@ static ssize_t fill_socket(int fd) { static size_t fill_socket_partial(int fd, size_t bytes) { ssize_t write_bytes; size_t total_bytes = 0; - unsigned char *buf = malloc(bytes); + unsigned char *buf = gpr_malloc(bytes); unsigned i; for (i = 0; i < bytes; ++i) { buf[i] = (uint8_t)(i % 256); @@ -292,7 +292,7 @@ static void write_done(grpc_exec_ctx *exec_ctx, } void drain_socket_blocking(int fd, size_t num_bytes, size_t read_size) { - unsigned char *buf = malloc(read_size); + unsigned char *buf = gpr_malloc(read_size); ssize_t bytes_read; size_t bytes_left = num_bytes; int flags; diff --git a/test/core/network_benchmarks/low_level_ping_pong.c b/test/core/network_benchmarks/low_level_ping_pong.c index 7ed3372d5d1..8179ba3b4df 100644 --- a/test/core/network_benchmarks/low_level_ping_pong.c +++ b/test/core/network_benchmarks/low_level_ping_pong.c @@ -265,19 +265,19 @@ static int epoll_setup(thread_args *args) { #endif static void server_thread(thread_args *args) { - char *buf = malloc(args->msg_size); + char *buf = gpr_malloc(args->msg_size); if (args->setup(args) < 0) { gpr_log(GPR_ERROR, "Setup failed"); } for (;;) { if (args->read_bytes(args, buf) < 0) { gpr_log(GPR_ERROR, "Server read failed"); - free(buf); + gpr_free(buf); return; } if (args->write_bytes(args, buf) < 0) { gpr_log(GPR_ERROR, "Server write failed"); - free(buf); + gpr_free(buf); return; } } @@ -304,7 +304,8 @@ static double now(void) { } static void client_thread(thread_args *args) { - char *buf = calloc(args->msg_size, sizeof(char)); + char *buf = gpr_malloc(args->msg_size * sizeof(char)); + memset(buf, 0, args->msg_size * sizeof(char)); gpr_histogram *histogram = gpr_histogram_create(0.01, 60e9); double start_time; double end_time; @@ -333,7 +334,7 @@ static void client_thread(thread_args *args) { } print_histogram(histogram); error: - free(buf); + gpr_free(buf); gpr_histogram_destroy(histogram); } @@ -596,8 +597,8 @@ static int run_all_benchmarks(size_t msg_size) { test_strategy *strategy = &test_strategies[i]; size_t j; for (j = 0; j < GPR_ARRAY_SIZE(socket_types); ++j) { - thread_args *client_args = malloc(sizeof(thread_args)); - thread_args *server_args = malloc(sizeof(thread_args)); + thread_args *client_args = gpr_malloc(sizeof(thread_args)); + thread_args *server_args = gpr_malloc(sizeof(thread_args)); char *socket_type = socket_types[j]; client_args->read_bytes = strategy->read_strategy; @@ -620,8 +621,8 @@ static int run_all_benchmarks(size_t msg_size) { } int main(int argc, char **argv) { - thread_args *client_args = malloc(sizeof(thread_args)); - thread_args *server_args = malloc(sizeof(thread_args)); + thread_args *client_args = gpr_malloc(sizeof(thread_args)); + thread_args *server_args = gpr_malloc(sizeof(thread_args)); int msg_size = -1; char *read_strategy = NULL; char *socket_type = NULL; diff --git a/test/core/support/alloc_test.c b/test/core/support/alloc_test.c index e2d0c16b41d..7eaf7a14f3e 100644 --- a/test/core/support/alloc_test.c +++ b/test/core/support/alloc_test.c @@ -46,7 +46,7 @@ static void fake_free(void *addr) { static void test_custom_allocs() { const gpr_allocation_functions default_fns = gpr_get_allocation_functions(); intptr_t addr_to_free = 0; - int *i; + char *i; gpr_allocation_functions fns = {fake_malloc, fake_realloc, fake_free}; gpr_set_allocation_functions(fns); @@ -58,7 +58,7 @@ static void test_custom_allocs() { /* Restore and check we don't get funky values and that we don't leak */ gpr_set_allocation_functions(default_fns); - GPR_ASSERT((void *)1 != (i = gpr_malloc(sizeof(*i)))); + GPR_ASSERT((void *)sizeof(*i) != (i = gpr_malloc(sizeof(*i)))); GPR_ASSERT((void *)2 != (i = gpr_realloc(i, 2))); gpr_free(i); } From 5d203f514c204083d1d81a804bd64df2d196197a Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Sat, 26 Mar 2016 01:35:33 +0100 Subject: [PATCH 02/14] Sanitize sources. --- src/core/surface/byte_buffer.c | 2 +- src/core/tsi/ssl_transport_security.c | 9 +++++---- src/core/tsi/transport_security.c | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/core/surface/byte_buffer.c b/src/core/surface/byte_buffer.c index a093a37af32..314a67dfcca 100644 --- a/src/core/surface/byte_buffer.c +++ b/src/core/surface/byte_buffer.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without diff --git a/src/core/tsi/ssl_transport_security.c b/src/core/tsi/ssl_transport_security.c index acf1b0e3c1d..968c0b9be8f 100644 --- a/src/core/tsi/ssl_transport_security.c +++ b/src/core/tsi/ssl_transport_security.c @@ -966,7 +966,8 @@ static tsi_result ssl_handshaker_extract_peer(tsi_handshaker *self, size_t i; tsi_peer_property *new_properties = gpr_malloc(sizeof(*new_properties) * (peer->property_count + 1)); - memset(new_properties, 0, sizeof(*new_properties) * (peer->property_count + 1)); + memset(new_properties, 0, + sizeof(*new_properties) * (peer->property_count + 1)); for (i = 0; i < peer->property_count; i++) { new_properties[i] = peer->properties[i]; } @@ -990,8 +991,7 @@ static tsi_result ssl_handshaker_create_frame_protector( size_t actual_max_output_protected_frame_size = TSI_SSL_MAX_PROTECTED_FRAME_SIZE_UPPER_BOUND; tsi_ssl_handshaker *impl = (tsi_ssl_handshaker *)self; - tsi_ssl_frame_protector *protector_impl = - gpr_malloc(sizeof(*protector_impl)); + tsi_ssl_frame_protector *protector_impl = gpr_malloc(sizeof(*protector_impl)); memset(protector_impl, 0, sizeof(*protector_impl)); if (max_output_protected_frame_size != NULL) { @@ -1411,7 +1411,8 @@ tsi_result tsi_create_ssl_server_handshaker_factory( memset(impl->ssl_contexts, 0, key_cert_pair_count * sizeof(SSL_CTX *)); impl->ssl_context_x509_subject_names = gpr_malloc(key_cert_pair_count * sizeof(tsi_peer)); - memset(impl->ssl_context_x509_subject_names, 0, key_cert_pair_count * sizeof(tsi_peer)); + memset(impl->ssl_context_x509_subject_names, 0, + key_cert_pair_count * sizeof(tsi_peer)); if (impl->ssl_contexts == NULL || impl->ssl_context_x509_subject_names == NULL) { tsi_ssl_handshaker_factory_destroy(&impl->base); diff --git a/src/core/tsi/transport_security.c b/src/core/tsi/transport_security.c index e17749d871a..189f5a0228a 100644 --- a/src/core/tsi/transport_security.c +++ b/src/core/tsi/transport_security.c @@ -1,6 +1,6 @@ /* * - * Copyright 2015, Google Inc. + * Copyright 2015-2016, Google Inc. * All rights reserved. * * Redistribution and use in source and binary forms, with or without From 367e5d896b115413f278b239dd63194cb3671c17 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Sat, 26 Mar 2016 01:37:43 +0100 Subject: [PATCH 03/14] Forgot a few callocs. --- src/core/tsi/fake_transport_security.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/core/tsi/fake_transport_security.c b/src/core/tsi/fake_transport_security.c index 86ffa6b83be..85c8ff7a57c 100644 --- a/src/core/tsi/fake_transport_security.c +++ b/src/core/tsi/fake_transport_security.c @@ -501,7 +501,8 @@ static const tsi_handshaker_vtable handshaker_vtable = { }; tsi_handshaker *tsi_create_fake_handshaker(int is_client) { - tsi_fake_handshaker *impl = calloc(1, sizeof(tsi_fake_handshaker)); + tsi_fake_handshaker *impl = gpr_malloc(sizeof(*impl)); + memset(impl, 0, sizeof(*impl)); impl->base.vtable = &handshaker_vtable; impl->is_client = is_client; impl->result = TSI_HANDSHAKE_IN_PROGRESS; @@ -517,8 +518,8 @@ tsi_handshaker *tsi_create_fake_handshaker(int is_client) { tsi_frame_protector *tsi_create_fake_protector( size_t *max_protected_frame_size) { - tsi_fake_frame_protector *impl = calloc(1, sizeof(tsi_fake_frame_protector)); - if (impl == NULL) return NULL; + tsi_fake_frame_protector *impl = gpr_malloc(sizeof(*impl)); + memset(impl, 0, sizeof(*impl)); impl->max_frame_size = (max_protected_frame_size == NULL) ? TSI_FAKE_DEFAULT_FRAME_SIZE : *max_protected_frame_size; From b29d8cfeb886e951dcbacc0f42f12624a2684086 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Fri, 8 Apr 2016 01:38:29 +0200 Subject: [PATCH 04/14] Last few mallocs / frees. --- include/grpc/support/string_util.h | 2 ++ src/core/lib/support/log_linux.c | 1 + src/core/lib/tsi/fake_transport_security.c | 3 ++- src/core/lib/tsi/ssl_transport_security.c | 1 + src/core/lib/tsi/transport_security.c | 24 +++++++--------------- 5 files changed, 13 insertions(+), 18 deletions(-) diff --git a/include/grpc/support/string_util.h b/include/grpc/support/string_util.h index 0abdb4a8c86..f981bc0db0b 100644 --- a/include/grpc/support/string_util.h +++ b/include/grpc/support/string_util.h @@ -34,6 +34,8 @@ #ifndef GRPC_SUPPORT_STRING_UTIL_H #define GRPC_SUPPORT_STRING_UTIL_H +#include + #ifdef __cplusplus extern "C" { #endif diff --git a/src/core/lib/support/log_linux.c b/src/core/lib/support/log_linux.c index 6d4b63bbe0d..ff3febb38eb 100644 --- a/src/core/lib/support/log_linux.c +++ b/src/core/lib/support/log_linux.c @@ -68,6 +68,7 @@ void gpr_log(const char *file, int line, gpr_log_severity severity, } va_end(args); gpr_log_message(file, line, severity, message); + /* message has been allocated by vasprintf above, and needs free */ free(message); } diff --git a/src/core/lib/tsi/fake_transport_security.c b/src/core/lib/tsi/fake_transport_security.c index a0f69c3d64d..0e20d6fd710 100644 --- a/src/core/lib/tsi/fake_transport_security.c +++ b/src/core/lib/tsi/fake_transport_security.c @@ -36,6 +36,7 @@ #include #include +#include #include #include #include @@ -137,7 +138,7 @@ static int tsi_fake_frame_ensure_size(tsi_fake_frame *frame) { frame->data = gpr_malloc(frame->allocated_size); if (frame->data == NULL) return 0; } else if (frame->size > frame->allocated_size) { - unsigned char *new_data = realloc(frame->data, frame->size); + unsigned char *new_data = gpr_realloc(frame->data, frame->size); if (new_data == NULL) { gpr_free(frame->data); frame->data = NULL; diff --git a/src/core/lib/tsi/ssl_transport_security.c b/src/core/lib/tsi/ssl_transport_security.c index ef3aeb70901..045901cc72f 100644 --- a/src/core/lib/tsi/ssl_transport_security.c +++ b/src/core/lib/tsi/ssl_transport_security.c @@ -45,6 +45,7 @@ #include #endif +#include #include #include #include diff --git a/src/core/lib/tsi/transport_security.c b/src/core/lib/tsi/transport_security.c index 66f1e356d6a..830cf095848 100644 --- a/src/core/lib/tsi/transport_security.c +++ b/src/core/lib/tsi/transport_security.c @@ -33,6 +33,9 @@ #include "src/core/lib/tsi/transport_security.h" +#include +#include + #include #include @@ -40,19 +43,6 @@ int tsi_tracing_enabled = 0; -/* --- Utils. --- */ - -char *tsi_strdup(const char *src) { - char *dst; - size_t len; - if (!src) return NULL; - len = strlen(src) + 1; - dst = malloc(len); - if (!dst) return NULL; - memcpy(dst, src, len); - return dst; -} - /* --- tsi_result common implementation. --- */ const char *tsi_result_to_string(tsi_result result) { @@ -214,15 +204,15 @@ static void tsi_peer_destroy_list_property(tsi_peer_property *children, for (i = 0; i < child_count; i++) { tsi_peer_property_destruct(&children[i]); } - free(children); + gpr_free(children); } void tsi_peer_property_destruct(tsi_peer_property *property) { if (property->name != NULL) { - free(property->name); + gpr_free(property->name); } if (property->value.data != NULL) { - free(property->value.data); + gpr_free(property->value.data); } *property = tsi_init_peer_property(); /* Reset everything to 0. */ } @@ -242,7 +232,7 @@ tsi_result tsi_construct_allocated_string_peer_property( if (name != NULL) property->name = gpr_strdup(name); if (value_length > 0) { property->value.data = gpr_malloc(value_length); - memset(value.data, 0, value_length); + memset(property->value.data, 0, value_length); property->value.length = value_length; } return TSI_OK; From b206c154dd7605bf0dbdc7a69605b6f749d2bef4 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Fri, 8 Apr 2016 19:42:28 +0200 Subject: [PATCH 05/14] Addressing comment. --- src/core/lib/profiling/basic_timers.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/lib/profiling/basic_timers.c b/src/core/lib/profiling/basic_timers.c index 0a4ae3057ad..50082cd7ee4 100644 --- a/src/core/lib/profiling/basic_timers.c +++ b/src/core/lib/profiling/basic_timers.c @@ -173,7 +173,7 @@ static void flush_logs(gpr_timer_log_list *list) { gpr_timer_log *log; while ((log = timer_log_pop_front(list)) != NULL) { write_log(log); - gpr_free(log); + free(log); } } @@ -208,7 +208,8 @@ static void init_output() { } static void rotate_log() { - gpr_timer_log *new = gpr_malloc(sizeof(*new)); + /* Using malloc here, as this code could end up being called by gpr_malloc */ + gpr_timer_log *new = malloc(sizeof(*new)); gpr_once_init(&g_once_init, init_output); new->num_entries = 0; pthread_mutex_lock(&g_mu); From 85a46dd78080b4060e12330ef6d139f5e6287016 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Tue, 12 Apr 2016 01:50:51 +0200 Subject: [PATCH 06/14] Moving memory allocation tracking to its own file. --- Makefile | 2 + build.yaml | 2 + test/core/json/fuzzer.c | 41 +------ test/core/util/memory_counters.c | 104 ++++++++++++++++++ test/core/util/memory_counters.h | 48 ++++++++ tools/run_tests/sources_and_headers.json | 3 + .../grpc_test_util/grpc_test_util.vcxproj | 3 + .../grpc_test_util.vcxproj.filters | 6 + .../grpc_test_util_unsecure.vcxproj | 3 + .../grpc_test_util_unsecure.vcxproj.filters | 6 + 10 files changed, 183 insertions(+), 35 deletions(-) create mode 100644 test/core/util/memory_counters.c create mode 100644 test/core/util/memory_counters.h diff --git a/Makefile b/Makefile index 6b2b9eb2dee..ce02019f250 100644 --- a/Makefile +++ b/Makefile @@ -2695,6 +2695,7 @@ LIBGRPC_TEST_UTIL_SRC = \ test/core/end2end/fixtures/proxy.c \ test/core/iomgr/endpoint_tests.c \ test/core/util/grpc_profiler.c \ + test/core/util/memory_counters.c \ test/core/util/mock_endpoint.c \ test/core/util/parse_hexstring.c \ test/core/util/port_posix.c \ @@ -2743,6 +2744,7 @@ LIBGRPC_TEST_UTIL_UNSECURE_SRC = \ test/core/end2end/fixtures/proxy.c \ test/core/iomgr/endpoint_tests.c \ test/core/util/grpc_profiler.c \ + test/core/util/memory_counters.c \ test/core/util/mock_endpoint.c \ test/core/util/parse_hexstring.c \ test/core/util/port_posix.c \ diff --git a/build.yaml b/build.yaml index cbbc3d22464..ce9bc286a29 100644 --- a/build.yaml +++ b/build.yaml @@ -567,6 +567,7 @@ filegroups: - test/core/end2end/fixtures/proxy.h - test/core/iomgr/endpoint_tests.h - test/core/util/grpc_profiler.h + - test/core/util/memory_counters.h - test/core/util/mock_endpoint.h - test/core/util/parse_hexstring.h - test/core/util/port.h @@ -577,6 +578,7 @@ filegroups: - test/core/end2end/fixtures/proxy.c - test/core/iomgr/endpoint_tests.c - test/core/util/grpc_profiler.c + - test/core/util/memory_counters.c - test/core/util/mock_endpoint.c - test/core/util/parse_hexstring.c - test/core/util/port_posix.c diff --git a/test/core/json/fuzzer.c b/test/core/json/fuzzer.c index 044db973ab1..e94b41ca999 100644 --- a/test/core/json/fuzzer.c +++ b/test/core/json/fuzzer.c @@ -38,42 +38,12 @@ #include #include "src/core/lib/json/json.h" - -static size_t g_total_size = 0; -static gpr_allocation_functions g_old_allocs; - -void *guard_malloc(size_t size) { - size_t *ptr; - g_total_size += size; - ptr = g_old_allocs.malloc_fn(size + sizeof(size)); - *ptr++ = size; - return ptr; -} - -void *guard_realloc(void *vptr, size_t size) { - size_t *ptr = vptr; - --ptr; - g_total_size -= *ptr; - ptr = g_old_allocs.realloc_fn(ptr, size + sizeof(size)); - g_total_size += size; - *ptr++ = size; - return ptr; -} - -void guard_free(void *vptr) { - size_t *ptr = vptr; - --ptr; - g_total_size -= *ptr; - g_old_allocs.free_fn(ptr); -} - -struct gpr_allocation_functions g_guard_allocs = {guard_malloc, guard_realloc, - guard_free}; +#include "test/core/util/memory_counters.h" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { char *s; - g_old_allocs = gpr_get_allocation_functions(); - gpr_set_allocation_functions(g_guard_allocs); + struct grpc_memory_counters counters; + grpc_memory_counters_init(); s = gpr_malloc(size); memcpy(s, data, size); grpc_json *x; @@ -81,7 +51,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { grpc_json_destroy(x); } gpr_free(s); - gpr_set_allocation_functions(g_old_allocs); - GPR_ASSERT(g_total_size == 0); + counters = grpc_memory_counters_snapshot(); + grpc_memory_counters_destroy(); + GPR_ASSERT(counters.total_size_relative == 0); return 0; } diff --git a/test/core/util/memory_counters.c b/test/core/util/memory_counters.c new file mode 100644 index 00000000000..85e85237a2f --- /dev/null +++ b/test/core/util/memory_counters.c @@ -0,0 +1,104 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#include +#include + +#include +#include + +#include "test/core/util/memory_counters.h" + +static gpr_mu g_memory_mutex; +static struct grpc_memory_counters g_memory_counters; +static gpr_allocation_functions g_old_allocs; + +static void *guard_malloc(size_t size) { + size_t *ptr; + gpr_mu_lock(&g_memory_mutex); + g_memory_counters.total_size_absolute += size; + g_memory_counters.total_size_relative += size; + g_memory_counters.total_allocs_absolute++; + g_memory_counters.total_allocs_relative++; + gpr_mu_unlock(&g_memory_mutex); + ptr = g_old_allocs.malloc_fn(size + sizeof(size)); + *ptr++ = size; + return ptr; +} + +static void *guard_realloc(void *vptr, size_t size) { + size_t *ptr = vptr; + --ptr; + gpr_mu_lock(&g_memory_mutex); + g_memory_counters.total_size_absolute += size; + g_memory_counters.total_size_relative -= *ptr; + g_memory_counters.total_size_relative += size; + g_memory_counters.total_allocs_absolute++; + gpr_mu_unlock(&g_memory_mutex); + ptr = g_old_allocs.realloc_fn(ptr, size + sizeof(size)); + *ptr++ = size; + return ptr; +} + +static void guard_free(void *vptr) { + size_t *ptr = vptr; + --ptr; + gpr_mu_lock(&g_memory_mutex); + g_memory_counters.total_size_relative -= *ptr; + g_memory_counters.total_allocs_relative--; + gpr_mu_unlock(&g_memory_mutex); + g_old_allocs.free_fn(ptr); +} + +struct gpr_allocation_functions g_guard_allocs = {guard_malloc, guard_realloc, + guard_free}; + +void grpc_memory_counters_init() { + memset(&g_memory_counters, 0, sizeof(g_memory_counters)); + gpr_mu_init(&g_memory_mutex); + g_old_allocs = gpr_get_allocation_functions(); + gpr_set_allocation_functions(g_guard_allocs); +} + +void grpc_memory_counters_destroy() { + gpr_set_allocation_functions(g_old_allocs); + gpr_mu_destroy(&g_memory_mutex); +} + +struct grpc_memory_counters grpc_memory_counters_snapshot() { + struct grpc_memory_counters counters; + gpr_mu_lock(&g_memory_mutex); + counters = g_memory_counters; + gpr_mu_unlock(&g_memory_mutex); + return counters; +} diff --git a/test/core/util/memory_counters.h b/test/core/util/memory_counters.h new file mode 100644 index 00000000000..f3328165014 --- /dev/null +++ b/test/core/util/memory_counters.h @@ -0,0 +1,48 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#ifndef GRPC_TEST_CORE_UTIL_MEMORY_COUNTERS_H +#define GRPC_TEST_CORE_UTIL_MEMORY_COUNTERS_H + +struct grpc_memory_counters { + size_t total_size_relative; + size_t total_size_absolute; + size_t total_allocs_relative; + size_t total_allocs_absolute; +}; + +void grpc_memory_counters_init(); +void grpc_memory_counters_destroy(); +struct grpc_memory_counters grpc_memory_counters_snapshot(); + +#endif diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index b2f2e1eb524..4a7a3c8c86d 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -6164,6 +6164,7 @@ "test/core/end2end/fixtures/proxy.h", "test/core/iomgr/endpoint_tests.h", "test/core/util/grpc_profiler.h", + "test/core/util/memory_counters.h", "test/core/util/mock_endpoint.h", "test/core/util/parse_hexstring.h", "test/core/util/port.h", @@ -6181,6 +6182,8 @@ "test/core/iomgr/endpoint_tests.h", "test/core/util/grpc_profiler.c", "test/core/util/grpc_profiler.h", + "test/core/util/memory_counters.c", + "test/core/util/memory_counters.h", "test/core/util/mock_endpoint.c", "test/core/util/mock_endpoint.h", "test/core/util/parse_hexstring.c", diff --git a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj index cb033a5aa4a..d1f67ee44e3 100644 --- a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj +++ b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj @@ -153,6 +153,7 @@ + @@ -176,6 +177,8 @@ + + diff --git a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters index 81b2a810530..2fee6aab62b 100644 --- a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters @@ -25,6 +25,9 @@ test\core\util + + test\core\util + test\core\util @@ -63,6 +66,9 @@ test\core\util + + test\core\util + test\core\util diff --git a/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj b/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj index bb93b2c6f35..336825353c7 100644 --- a/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj +++ b/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj @@ -151,6 +151,7 @@ + @@ -166,6 +167,8 @@ + + diff --git a/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj.filters b/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj.filters index 4c4620a288d..5c2b961b676 100644 --- a/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj.filters @@ -13,6 +13,9 @@ test\core\util + + test\core\util + test\core\util @@ -45,6 +48,9 @@ test\core\util + + test\core\util + test\core\util From 839cdb51d5521b2792d04a351d864d9c35ab7511 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Tue, 12 Apr 2016 02:11:10 +0200 Subject: [PATCH 07/14] Adding memory checks for server fuzzer. --- test/core/end2end/fuzzers/server_fuzzer.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/core/end2end/fuzzers/server_fuzzer.c b/test/core/end2end/fuzzers/server_fuzzer.c index 393d33033ba..c82df42ebfc 100644 --- a/test/core/end2end/fuzzers/server_fuzzer.c +++ b/test/core/end2end/fuzzers/server_fuzzer.c @@ -35,6 +35,7 @@ #include "src/core/ext/transport/chttp2/transport/chttp2_transport.h" #include "src/core/lib/surface/server.h" +#include "test/core/util/memory_counters.h" #include "test/core/util/mock_endpoint.h" static const bool squelch = true; @@ -48,8 +49,10 @@ static void dont_log(gpr_log_func_args *args) {} int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { grpc_test_only_set_metadata_hash_seed(0); + struct grpc_memory_counters counters; if (squelch) gpr_set_log_function(dont_log); grpc_init(); + grpc_memory_counters_init(); grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; grpc_endpoint *mock_endpoint = grpc_mock_endpoint_create(discard_write); @@ -116,6 +119,9 @@ done: } grpc_server_destroy(server); grpc_completion_queue_destroy(cq); + counters = grpc_memory_counters_snapshot(); + grpc_memory_counters_destroy(); grpc_shutdown(); + GPR_ASSERT(counters.total_size_relative == 0); return 0; } From 2e1a5ac748ca21e87a012eb2c0d86ec273350389 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Tue, 12 Apr 2016 08:42:34 +0200 Subject: [PATCH 08/14] Initializing counters before grpc_init, and destroying them after grpc_shutdown. --- test/core/end2end/fuzzers/server_fuzzer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/core/end2end/fuzzers/server_fuzzer.c b/test/core/end2end/fuzzers/server_fuzzer.c index c82df42ebfc..06dc5b8bf34 100644 --- a/test/core/end2end/fuzzers/server_fuzzer.c +++ b/test/core/end2end/fuzzers/server_fuzzer.c @@ -51,8 +51,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { grpc_test_only_set_metadata_hash_seed(0); struct grpc_memory_counters counters; if (squelch) gpr_set_log_function(dont_log); - grpc_init(); grpc_memory_counters_init(); + grpc_init(); grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; grpc_endpoint *mock_endpoint = grpc_mock_endpoint_create(discard_write); @@ -120,8 +120,8 @@ done: grpc_server_destroy(server); grpc_completion_queue_destroy(cq); counters = grpc_memory_counters_snapshot(); - grpc_memory_counters_destroy(); grpc_shutdown(); + grpc_memory_counters_destroy(); GPR_ASSERT(counters.total_size_relative == 0); return 0; } From 7c37a687eb884c78d119d4b7903f173cca351255 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Tue, 12 Apr 2016 19:08:00 +0200 Subject: [PATCH 09/14] Preventing crashes for known realloc behaviors. --- test/core/util/memory_counters.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/core/util/memory_counters.c b/test/core/util/memory_counters.c index 85e85237a2f..bebe94e5826 100644 --- a/test/core/util/memory_counters.c +++ b/test/core/util/memory_counters.c @@ -43,8 +43,13 @@ static gpr_mu g_memory_mutex; static struct grpc_memory_counters g_memory_counters; static gpr_allocation_functions g_old_allocs; +static void *guard_malloc(size_t size); +static void *guard_realloc(void *vptr, size_t size); +static void guard_free(void *vptr); + static void *guard_malloc(size_t size) { size_t *ptr; + if (!size) return NULL; gpr_mu_lock(&g_memory_mutex); g_memory_counters.total_size_absolute += size; g_memory_counters.total_size_relative += size; @@ -58,6 +63,13 @@ static void *guard_malloc(size_t size) { static void *guard_realloc(void *vptr, size_t size) { size_t *ptr = vptr; + if (vptr == NULL) { + return guard_malloc(size); + } + if (size == 0) { + guard_free(vptr); + return NULL; + } --ptr; gpr_mu_lock(&g_memory_mutex); g_memory_counters.total_size_absolute += size; @@ -72,6 +84,7 @@ static void *guard_realloc(void *vptr, size_t size) { static void guard_free(void *vptr) { size_t *ptr = vptr; + if (!vptr) return; --ptr; gpr_mu_lock(&g_memory_mutex); g_memory_counters.total_size_relative -= *ptr; From 5b304753965d4179f374b5cded063e79d3d17b0a Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Tue, 12 Apr 2016 19:29:22 +0200 Subject: [PATCH 10/14] Fixing a couple of issues within core. --- src/core/lib/channel/channel_args.c | 3 ++- src/core/lib/surface/server.c | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/core/lib/channel/channel_args.c b/src/core/lib/channel/channel_args.c index 28d2d78d00f..97c40e84810 100644 --- a/src/core/lib/channel/channel_args.c +++ b/src/core/lib/channel/channel_args.c @@ -74,7 +74,8 @@ grpc_channel_args *grpc_channel_args_copy_and_add(const grpc_channel_args *src, return dst; } dst->num_args = src_num_args + num_to_add; - dst->args = gpr_malloc(sizeof(grpc_arg) * dst->num_args); + dst->args = + dst->num_args ? gpr_malloc(sizeof(grpc_arg) * dst->num_args) : NULL; for (i = 0; i < src_num_args; i++) { dst->args[i] = copy_arg(&src->args[i]); } diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index ad8ee8c7a99..e3b54ac012e 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -251,7 +251,9 @@ static void channel_broadcaster_init(grpc_server *s, channel_broadcaster *cb) { count++; } cb->num_channels = count; - cb->channels = gpr_malloc(sizeof(*cb->channels) * cb->num_channels); + cb->channels = cb->num_channels + ? gpr_malloc(sizeof(*cb->channels) * cb->num_channels) + : NULL; count = 0; for (c = s->root_channel_data.next; c != &s->root_channel_data; c = c->next) { cb->channels[count++] = c->channel; From 14b5dfeafb1f5f351ea3e114131afd7c63adc4b9 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Tue, 12 Apr 2016 19:53:07 +0200 Subject: [PATCH 11/14] Properly grabbing counters at the proper time. --- test/core/end2end/fuzzers/server_fuzzer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/end2end/fuzzers/server_fuzzer.c b/test/core/end2end/fuzzers/server_fuzzer.c index 06dc5b8bf34..40273711ab7 100644 --- a/test/core/end2end/fuzzers/server_fuzzer.c +++ b/test/core/end2end/fuzzers/server_fuzzer.c @@ -119,8 +119,8 @@ done: } grpc_server_destroy(server); grpc_completion_queue_destroy(cq); - counters = grpc_memory_counters_snapshot(); grpc_shutdown(); + counters = grpc_memory_counters_snapshot(); grpc_memory_counters_destroy(); GPR_ASSERT(counters.total_size_relative == 0); return 0; From a592ffa7451e67d453e42e4f79bc5e7cb498e2b3 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Tue, 12 Apr 2016 19:56:57 +0200 Subject: [PATCH 12/14] Memory checking the client fuzzer. --- test/core/end2end/fuzzers/client_fuzzer.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/core/end2end/fuzzers/client_fuzzer.c b/test/core/end2end/fuzzers/client_fuzzer.c index 4e22e268f04..afcf7638f72 100644 --- a/test/core/end2end/fuzzers/client_fuzzer.c +++ b/test/core/end2end/fuzzers/client_fuzzer.c @@ -36,6 +36,7 @@ #include "src/core/ext/transport/chttp2/transport/chttp2_transport.h" #include "src/core/lib/surface/channel.h" +#include "test/core/util/memory_counters.h" #include "test/core/util/mock_endpoint.h" static const bool squelch = true; @@ -48,7 +49,9 @@ static void dont_log(gpr_log_func_args *args) {} int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { grpc_test_only_set_metadata_hash_seed(0); + struct grpc_memory_counters counters; if (squelch) gpr_set_log_function(dont_log); + grpc_memory_counters_init(); grpc_init(); grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; @@ -150,5 +153,8 @@ done: grpc_byte_buffer_destroy(response_payload_recv); } grpc_shutdown(); + counters = grpc_memory_counters_snapshot(); + grpc_memory_counters_destroy(); + GPR_ASSERT(counters.total_size_relative == 0); return 0; } From b71bf8eb214894d11db77eae78a9abb8c4cebf44 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Wed, 13 Apr 2016 01:30:13 +0200 Subject: [PATCH 13/14] Revert "Fixing a couple of issues within core." This reverts commit 5b304753965d4179f374b5cded063e79d3d17b0a. --- src/core/lib/channel/channel_args.c | 3 +-- src/core/lib/surface/server.c | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/core/lib/channel/channel_args.c b/src/core/lib/channel/channel_args.c index 97c40e84810..28d2d78d00f 100644 --- a/src/core/lib/channel/channel_args.c +++ b/src/core/lib/channel/channel_args.c @@ -74,8 +74,7 @@ grpc_channel_args *grpc_channel_args_copy_and_add(const grpc_channel_args *src, return dst; } dst->num_args = src_num_args + num_to_add; - dst->args = - dst->num_args ? gpr_malloc(sizeof(grpc_arg) * dst->num_args) : NULL; + dst->args = gpr_malloc(sizeof(grpc_arg) * dst->num_args); for (i = 0; i < src_num_args; i++) { dst->args[i] = copy_arg(&src->args[i]); } diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index e3b54ac012e..ad8ee8c7a99 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -251,9 +251,7 @@ static void channel_broadcaster_init(grpc_server *s, channel_broadcaster *cb) { count++; } cb->num_channels = count; - cb->channels = cb->num_channels - ? gpr_malloc(sizeof(*cb->channels) * cb->num_channels) - : NULL; + cb->channels = gpr_malloc(sizeof(*cb->channels) * cb->num_channels); count = 0; for (c = s->root_channel_data.next; c != &s->root_channel_data; c = c->next) { cb->channels[count++] = c->channel; From 23e4baefc2669c271f78402f5e9523d9f60a4cfe Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Wed, 13 Apr 2016 01:35:42 +0200 Subject: [PATCH 14/14] Enforcing undefined behavior of malloc. --- src/core/lib/support/alloc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/lib/support/alloc.c b/src/core/lib/support/alloc.c index 020917f79c8..618c3f6acd8 100644 --- a/src/core/lib/support/alloc.c +++ b/src/core/lib/support/alloc.c @@ -53,6 +53,7 @@ void gpr_set_allocation_functions(gpr_allocation_functions functions) { void *gpr_malloc(size_t size) { void *p; + if (size == 0) return NULL; GPR_TIMER_BEGIN("gpr_malloc", 0); p = g_alloc_functions.malloc_fn(size); if (!p) { @@ -69,6 +70,7 @@ void gpr_free(void *p) { } void *gpr_realloc(void *p, size_t size) { + if ((size == 0) && (p == NULL)) return NULL; GPR_TIMER_BEGIN("gpr_realloc", 0); p = g_alloc_functions.realloc_fn(p, size); if (!p) {