Merge pull request #6423 from ctiller/signal_handler
Fix some ubsan issues
diff --git a/build.yaml b/build.yaml
index 09ab527..659c861 100644
--- a/build.yaml
+++ b/build.yaml
@@ -3295,7 +3295,7 @@
LDXX: clang++
compile_the_world: true
test_environ:
- UBSAN_OPTIONS: print_stacktrace=1
+ UBSAN_OPTIONS: halt_on_error=1:print_stacktrace=1
timeout_multiplier: 1.5
defaults:
boringssl:
diff --git a/src/core/ext/client_config/subchannel.c b/src/core/ext/client_config/subchannel.c
index bd45d38..cfd39e7 100644
--- a/src/core/ext/client_config/subchannel.c
+++ b/src/core/ext/client_config/subchannel.c
@@ -320,7 +320,7 @@
c->filters = NULL;
}
c->addr = gpr_malloc(args->addr_len);
- memcpy(c->addr, args->addr, args->addr_len);
+ if (args->addr_len) memcpy(c->addr, args->addr, args->addr_len);
c->pollset_set = grpc_pollset_set_create();
c->addr_len = args->addr_len;
grpc_set_initial_connect_string(&c->addr, &c->addr_len,
diff --git a/src/core/ext/client_config/subchannel_index.c b/src/core/ext/client_config/subchannel_index.c
index ab8d9bd..690cb16 100644
--- a/src/core/ext/client_config/subchannel_index.c
+++ b/src/core/ext/client_config/subchannel_index.c
@@ -77,12 +77,19 @@
grpc_subchannel_key *k = gpr_malloc(sizeof(*k));
k->connector = grpc_connector_ref(connector);
k->args.filter_count = args->filter_count;
- k->args.filters = gpr_malloc(sizeof(*k->args.filters) * k->args.filter_count);
- memcpy((grpc_channel_filter *)k->args.filters, args->filters,
- sizeof(*k->args.filters) * k->args.filter_count);
+ if (k->args.filter_count > 0) {
+ k->args.filters =
+ gpr_malloc(sizeof(*k->args.filters) * k->args.filter_count);
+ memcpy((grpc_channel_filter *)k->args.filters, args->filters,
+ sizeof(*k->args.filters) * k->args.filter_count);
+ } else {
+ k->args.filters = NULL;
+ }
k->args.addr_len = args->addr_len;
k->args.addr = gpr_malloc(args->addr_len);
- memcpy(k->args.addr, args->addr, k->args.addr_len);
+ if (k->args.addr_len > 0) {
+ memcpy(k->args.addr, args->addr, k->args.addr_len);
+ }
k->args.args = copy_channel_args(args->args);
return k;
}
@@ -104,11 +111,15 @@
if (c != 0) return c;
c = GPR_ICMP(a->args.filter_count, b->args.filter_count);
if (c != 0) return c;
- c = memcmp(a->args.addr, b->args.addr, a->args.addr_len);
- if (c != 0) return c;
- c = memcmp(a->args.filters, b->args.filters,
- a->args.filter_count * sizeof(*a->args.filters));
- if (c != 0) return c;
+ if (a->args.addr_len) {
+ c = memcmp(a->args.addr, b->args.addr, a->args.addr_len);
+ if (c != 0) return c;
+ }
+ if (a->args.filter_count > 0) {
+ c = memcmp(a->args.filters, b->args.filters,
+ a->args.filter_count * sizeof(*a->args.filters));
+ if (c != 0) return c;
+ }
return grpc_channel_args_compare(a->args.args, b->args.args);
}
diff --git a/src/core/ext/transport/chttp2/transport/frame_goaway.c b/src/core/ext/transport/chttp2/transport/frame_goaway.c
index 69accb7..827e7a6 100644
--- a/src/core/ext/transport/chttp2/transport/frame_goaway.c
+++ b/src/core/ext/transport/chttp2/transport/frame_goaway.c
@@ -137,7 +137,8 @@
++cur;
/* fallthrough */
case GRPC_CHTTP2_GOAWAY_DEBUG:
- memcpy(p->debug_data + p->debug_pos, cur, (size_t)(end - cur));
+ if (end != cur)
+ memcpy(p->debug_data + p->debug_pos, cur, (size_t)(end - cur));
GPR_ASSERT((size_t)(end - cur) < UINT32_MAX - p->debug_pos);
p->debug_pos += (uint32_t)(end - cur);
p->state = GRPC_CHTTP2_GOAWAY_DEBUG;
diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.c b/src/core/ext/transport/chttp2/transport/hpack_parser.c
index 687936b..ed45bc9 100644
--- a/src/core/ext/transport/chttp2/transport/hpack_parser.c
+++ b/src/core/ext/transport/chttp2/transport/hpack_parser.c
@@ -1138,6 +1138,7 @@
/* append some bytes to a string */
static void append_bytes(grpc_chttp2_hpack_parser_string *str,
const uint8_t *data, size_t length) {
+ if (length == 0) return;
if (length + str->length > str->capacity) {
GPR_ASSERT(str->length + length <= UINT32_MAX);
str->capacity = (uint32_t)(str->length + length);
@@ -1445,6 +1446,11 @@
stream id on a header */
if (stream_parsing != NULL) {
if (parser->is_boundary) {
+ if (stream_parsing->header_frames_received ==
+ GPR_ARRAY_SIZE(stream_parsing->got_metadata_on_parse)) {
+ gpr_log(GPR_ERROR, "too many trailer frames");
+ return GRPC_CHTTP2_CONNECTION_ERROR;
+ }
stream_parsing
->got_metadata_on_parse[stream_parsing->header_frames_received] = 1;
stream_parsing->header_frames_received++;
diff --git a/src/core/lib/channel/channel_args.c b/src/core/lib/channel/channel_args.c
index 893cf07..569be4d 100644
--- a/src/core/lib/channel/channel_args.c
+++ b/src/core/lib/channel/channel_args.c
@@ -132,7 +132,8 @@
for (size_t i = 0; i < a->num_args; i++) {
args[i] = &a->args[i];
}
- qsort(args, a->num_args, sizeof(grpc_arg *), cmp_key_stable);
+ if (a->num_args > 1)
+ qsort(args, a->num_args, sizeof(grpc_arg *), cmp_key_stable);
grpc_channel_args *b = gpr_malloc(sizeof(grpc_channel_args));
b->num_args = a->num_args;
diff --git a/src/core/lib/compression/compression_algorithm.c b/src/core/lib/compression/compression_algorithm.c
index 7039364..820871d 100644
--- a/src/core/lib/compression/compression_algorithm.c
+++ b/src/core/lib/compression/compression_algorithm.c
@@ -199,5 +199,6 @@
int grpc_compression_options_is_algorithm_enabled(
const grpc_compression_options *opts,
grpc_compression_algorithm algorithm) {
+ if (algorithm >= GRPC_COMPRESS_ALGORITHMS_COUNT) return 0;
return GPR_BITGET(opts->enabled_algorithms_bitset, algorithm);
}
diff --git a/src/core/lib/support/murmur_hash.c b/src/core/lib/support/murmur_hash.c
index 5711fff..7137c1f 100644
--- a/src/core/lib/support/murmur_hash.c
+++ b/src/core/lib/support/murmur_hash.c
@@ -33,6 +33,8 @@
#include "src/core/lib/support/murmur_hash.h"
+#include <string.h>
+
#define ROTL32(x, r) ((x) << (r)) | ((x) >> (32 - (r)))
#define FMIX32(h) \
@@ -42,10 +44,6 @@
(h) *= 0xc2b2ae35; \
(h) ^= (h) >> 16;
-/* Block read - if your platform needs to do endian-swapping or can only
- handle aligned reads, do the conversion here */
-#define GETBLOCK32(p, i) (p)[(i)]
-
uint32_t gpr_murmur_hash3(const void *key, size_t len, uint32_t seed) {
const uint8_t *data = (const uint8_t *)key;
const size_t nblocks = len / 4;
@@ -62,7 +60,7 @@
/* body */
for (i = -(int)nblocks; i; i++) {
- k1 = GETBLOCK32(blocks, i);
+ memcpy(&k1, blocks + i, sizeof(uint32_t));
k1 *= c1;
k1 = ROTL32(k1, 15);
diff --git a/src/core/lib/transport/metadata.c b/src/core/lib/transport/metadata.c
index 5847ec9..82c8e23 100644
--- a/src/core/lib/transport/metadata.c
+++ b/src/core/lib/transport/metadata.c
@@ -373,7 +373,8 @@
ss = g_static_strtab[idx];
if (ss == NULL) break;
if (ss->hash == hash && GPR_SLICE_LENGTH(ss->slice) == length &&
- 0 == memcmp(buf, GPR_SLICE_START_PTR(ss->slice), length)) {
+ (length == 0 ||
+ 0 == memcmp(buf, GPR_SLICE_START_PTR(ss->slice), length))) {
GPR_TIMER_END("grpc_mdstr_from_buffer", 0);
return ss;
}
diff --git a/test/core/end2end/fuzzers/api_fuzzer.c b/test/core/end2end/fuzzers/api_fuzzer.c
index cdfa960..a8b1232 100644
--- a/test/core/end2end/fuzzers/api_fuzzer.c
+++ b/test/core/end2end/fuzzers/api_fuzzer.c
@@ -424,15 +424,19 @@
static void read_metadata(input_stream *inp, size_t *count,
grpc_metadata **metadata, call_state *cs) {
*count = next_byte(inp);
- *metadata = gpr_malloc(*count * sizeof(**metadata));
- memset(*metadata, 0, *count * sizeof(**metadata));
- for (size_t i = 0; i < *count; i++) {
- (*metadata)[i].key = read_string(inp);
- read_buffer(inp, (char **)&(*metadata)[i].value,
- &(*metadata)[i].value_length);
- (*metadata)[i].flags = read_uint32(inp);
- add_to_free(cs, (void *)(*metadata)[i].key);
- add_to_free(cs, (void *)(*metadata)[i].value);
+ if (*count) {
+ *metadata = gpr_malloc(*count * sizeof(**metadata));
+ memset(*metadata, 0, *count * sizeof(**metadata));
+ for (size_t i = 0; i < *count; i++) {
+ (*metadata)[i].key = read_string(inp);
+ read_buffer(inp, (char **)&(*metadata)[i].value,
+ &(*metadata)[i].value_length);
+ (*metadata)[i].flags = read_uint32(inp);
+ add_to_free(cs, (void *)(*metadata)[i].key);
+ add_to_free(cs, (void *)(*metadata)[i].value);
+ }
+ } else {
+ *metadata = gpr_malloc(1);
}
add_to_free(cs, *metadata);
}
diff --git a/tools/run_tests/configs.json b/tools/run_tests/configs.json
index bcc4118..b0839ef 100644
--- a/tools/run_tests/configs.json
+++ b/tools/run_tests/configs.json
@@ -57,7 +57,7 @@
{
"config": "ubsan",
"environ": {
- "UBSAN_OPTIONS": "print_stacktrace=1"
+ "UBSAN_OPTIONS": "halt_on_error=1:print_stacktrace=1"
},
"timeout_multiplier": 1.5
},