From 4aa541683b1341cd9d258a2010a5c6c9fd495540 Mon Sep 17 00:00:00 2001 From: George Joseph Date: Tue, 3 May 2022 06:57:58 -0600 Subject: [PATCH] GCC12: Fixes for 16+ Most issues were in stringfields and had to do with comparing a pointer to an constant/interned string with NULL. Since the string was a constant, a pointer to it could never be NULL so the comparison was always "true". gcc now complains about that. There were also a few issues where determining if there was enough space for a memcpy or s(n)printf which were fixed by defining some of the involved variables as "volatile". There were also a few other miscellaneous fixes. ASTERISK-30044 Change-Id: Ia081ca1bcfb329df6487c4660aaf1944309eb570 --- addons/Makefile | 4 ++ apps/app_festival.c | 2 +- channels/chan_sip.c | 4 +- channels/sig_analog.c | 4 ++ funcs/func_scramble.c | 2 +- include/asterisk/stringfields.h | 65 +++++++++++++++++---------------- include/asterisk/strings.h | 10 +++-- main/pbx.c | 15 ++++++-- main/stun.c | 8 +++- res/res_config_pgsql.c | 2 +- res/res_tonedetect.c | 2 +- tests/test_vector.c | 2 +- 12 files changed, 74 insertions(+), 46 deletions(-) diff --git a/addons/Makefile b/addons/Makefile index ae97ffb1c2..06360dbfa5 100644 --- a/addons/Makefile +++ b/addons/Makefile @@ -62,6 +62,10 @@ chan_ooh323.so: _ASTCFLAGS+=$(H323CFLAGS) $(call MOD_ADD_C,chan_ooh323,$(H323SOURCE)) ifneq ($(wildcard mp3/Makefile),) +# At the current time, the fate of mp3 is in flux so it didn't make sense to +# add configure/makeopts processing for array-bounds since this is the only +# source file that needs that warning suppressed. +mp3/layer3.o: _ASTCFLAGS+=-Wno-array-bounds $(call MOD_ADD_C,format_mp3,mp3/common.c mp3/dct64_i386.c mp3/decode_ntom.c mp3/layer3.c mp3/tabinit.c mp3/interface.c) .PHONY: check_mp3 diff --git a/apps/app_festival.c b/apps/app_festival.c index c18725dbb6..5348bc79da 100644 --- a/apps/app_festival.c +++ b/apps/app_festival.c @@ -433,7 +433,7 @@ static int festival_exec(struct ast_channel *chan, const char *vdata) } readcache = 0; writecache = 0; - if (strlen(cachedir) + strlen(MD5Hex) + 1 <= MAXFESTLEN && (usecache == -1)) { + if (strlen(cachedir) + sizeof(MD5Hex) + 1 <= MAXFESTLEN && (usecache == -1)) { snprintf(cachefile, sizeof(cachefile), "%s/%s", cachedir, MD5Hex); fdesc = open(cachefile, O_RDWR); if (fdesc == -1) { diff --git a/channels/chan_sip.c b/channels/chan_sip.c index affd73fbd5..bf03c5ff26 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -35420,8 +35420,8 @@ AST_TEST_DEFINE(get_in_brackets_const_test) ast_test_status_update(test, "Unexpected result: %d != %d\n", expected_res, res); \ return AST_TEST_FAIL; \ } \ - if ((expected_start) != start) { \ - const char *e = expected_start ? expected_start : "(null)"; \ + if ((void *)(expected_start) != (void *)start) { \ + const char *e = ((void *)expected_start != (void *)NULL) ? expected_start : "(null)"; \ const char *a = start ? start : "(null)"; \ ast_test_status_update(test, "Unexpected start: %s != %s\n", e, a); \ return AST_TEST_FAIL; \ diff --git a/channels/sig_analog.c b/channels/sig_analog.c index 64f1114fde..ea507fe078 100644 --- a/channels/sig_analog.c +++ b/channels/sig_analog.c @@ -1033,6 +1033,10 @@ int analog_call(struct analog_pvt *p, struct ast_channel *ast, const char *rdest ast_log(LOG_WARNING, "Number '%s' is shorter than stripmsd (%d)\n", c, p->stripmsd); c = NULL; } + if (c && (strlen(c) > sizeof(p->dop.dialstr) - 3 /* "Tw\0" */)) { + ast_log(LOG_WARNING, "Number '%s' is longer than %d bytes\n", c, (int)sizeof(p->dop.dialstr) - 2); + c = NULL; + } if (c) { p->dop.op = ANALOG_DIAL_OP_REPLACE; snprintf(p->dop.dialstr, sizeof(p->dop.dialstr), "Tw%s", c); diff --git a/funcs/func_scramble.c b/funcs/func_scramble.c index a90d7a2774..5fe194cd15 100644 --- a/funcs/func_scramble.c +++ b/funcs/func_scramble.c @@ -126,7 +126,7 @@ static int scramble_callback(struct ast_audiohook *audiohook, struct ast_channel if (frame->frametype == AST_FRAME_VOICE) { /* only invert voice frequencies */ /* Based on direction of frame, and confirm it is applicable */ - if (!(direction == AST_AUDIOHOOK_DIRECTION_READ ? &ni->rx : &ni->tx)) { + if (!(direction == AST_AUDIOHOOK_DIRECTION_READ ? ni->rx : ni->tx)) { return 0; } /* Scramble the sample now */ diff --git a/include/asterisk/stringfields.h b/include/asterisk/stringfields.h index edf479d75b..94ba756e98 100644 --- a/include/asterisk/stringfields.h +++ b/include/asterisk/stringfields.h @@ -355,10 +355,11 @@ enum ast_stringfield_cleanup_type { \retval zero on success \retval non-zero on failure */ + #define ast_string_field_init(x, size) \ ({ \ int __res__ = -1; \ - if (((void *)(x)) != NULL) { \ + if (((void *)(x)) != (void *)NULL) { \ __res__ = __ast_string_field_init(&(x)->__field_mgr, &(x)->__field_mgr_pool, size, __FILE__, __LINE__, __PRETTY_FUNCTION__); \ } \ __res__ ; \ @@ -373,7 +374,7 @@ enum ast_stringfield_cleanup_type { #define ast_string_field_free_memory(x) \ ({ \ int __res__ = -1; \ - if (((void *)(x)) != NULL) { \ + if (((void *)(x)) != (void *)NULL) { \ __res__ = __ast_string_field_free_memory(&(x)->__field_mgr, &(x)->__field_mgr_pool, \ AST_STRINGFIELD_DESTROY, __FILE__, __LINE__, __PRETTY_FUNCTION__); \ } \ @@ -400,7 +401,7 @@ int __ast_string_field_free_memory(struct ast_string_field_mgr *mgr, #define ast_string_field_init_extended(x, field) \ ({ \ int __res__ = -1; \ - if (((void *)(x)) != NULL) { \ + if (((void *)(x)) != (void *)NULL) { \ ast_string_field *non_const = (ast_string_field *)&(x)->field; \ *non_const = __ast_string_field_empty; \ __res__ = AST_VECTOR_APPEND(&(x)->__field_mgr.string_fields, non_const); \ @@ -474,7 +475,7 @@ void __ast_string_field_release_active(struct ast_string_field_pool *pool_head, #define ast_string_field_ptr_set(x, ptr, data) \ ({ \ int __res__ = -1; \ - if (((void *)(x)) != NULL) { \ + if (((void *)(x)) != (void *)NULL) { \ __res__ = ast_string_field_ptr_set_by_fields((x)->__field_mgr_pool, (x)->__field_mgr, ptr, data); \ } \ __res__; \ @@ -482,26 +483,28 @@ void __ast_string_field_release_active(struct ast_string_field_pool *pool_head, #define __ast_string_field_ptr_set_by_fields(field_mgr_pool, field_mgr, ptr, data, file, lineno, func) \ ({ \ - int __res__ = 0; \ - const char *__d__ = (data); \ - size_t __dlen__ = (__d__) ? strlen(__d__) + 1 : 1; \ - ast_string_field *__p__ = (ast_string_field *) (ptr); \ - ast_string_field target = *__p__; \ - if (__dlen__ == 1) { \ - __ast_string_field_release_active(field_mgr_pool, *__p__); \ - *__p__ = __ast_string_field_empty; \ - } else if ((__dlen__ <= AST_STRING_FIELD_ALLOCATION(*__p__)) || \ - (!__ast_string_field_ptr_grow(&field_mgr, &field_mgr_pool, __dlen__, __p__)) || \ - (target = __ast_string_field_alloc_space(&field_mgr, &field_mgr_pool, __dlen__, file, lineno, func))) { \ - if (target != *__p__) { \ - __ast_string_field_release_active(field_mgr_pool, *__p__); \ - *__p__ = target; \ - } \ - memcpy(* (void **) __p__, __d__, __dlen__); \ - } else { \ - __res__ = -1; \ - } \ - __res__; \ + int __res__ = 0; \ + const char *__d__ = (data); \ + ast_string_field *__p__ = (ast_string_field *) (ptr); \ + ast_string_field target = *__p__; \ + if (__d__ == NULL || *__d__ == '\0') { \ + __ast_string_field_release_active(field_mgr_pool, *__p__); \ + *__p__ = __ast_string_field_empty; \ + } else { \ + size_t __dlen__ = strlen(__d__) + 1; \ + if ((__dlen__ <= AST_STRING_FIELD_ALLOCATION(*__p__)) || \ + (!__ast_string_field_ptr_grow(&field_mgr, &field_mgr_pool, __dlen__, __p__)) || \ + (target = __ast_string_field_alloc_space(&field_mgr, &field_mgr_pool, __dlen__, file, lineno, func))) { \ + if (target != *__p__) { \ + __ast_string_field_release_active(field_mgr_pool, *__p__); \ + *__p__ = target; \ + } \ + memcpy(* (void **) __p__, __d__, __dlen__); \ + } else { \ + __res__ = -1; \ + } \ + } \ + __res__; \ }) #define ast_string_field_ptr_set_by_fields(field_mgr_pool, field_mgr, ptr, data) \ @@ -518,7 +521,7 @@ void __ast_string_field_release_active(struct ast_string_field_pool *pool_head, #define ast_string_field_set(x, field, data) \ ({ \ int __res__ = -1; \ - if (((void *)(x)) != NULL) { \ + if (((void *)(x)) != (void *)NULL) { \ __res__ = ast_string_field_ptr_set(x, &(x)->field, data); \ } \ __res__; \ @@ -534,7 +537,7 @@ void __ast_string_field_release_active(struct ast_string_field_pool *pool_head, #define ast_string_field_ptr_build(x, ptr, fmt, args...) \ ({ \ int __res__ = -1; \ - if (((void *)(x)) != NULL) { \ + if (((void *)(x)) != (void *)NULL) { \ __ast_string_field_ptr_build(__FILE__, __LINE__, __PRETTY_FUNCTION__, \ &(x)->__field_mgr, &(x)->__field_mgr_pool, (ast_string_field *) ptr, fmt, args); \ __res__ = 0; \ @@ -552,7 +555,7 @@ void __ast_string_field_release_active(struct ast_string_field_pool *pool_head, #define ast_string_field_build(x, field, fmt, args...) \ ({ \ int __res__ = -1; \ - if (((void *)(x)) != NULL) { \ + if (((void *)(x)) != (void *)NULL) { \ __ast_string_field_ptr_build(__FILE__, __LINE__, __PRETTY_FUNCTION__, \ &(x)->__field_mgr, &(x)->__field_mgr_pool, (ast_string_field *) &(x)->field, fmt, args); \ __res__ = 0; \ @@ -570,7 +573,7 @@ void __ast_string_field_release_active(struct ast_string_field_pool *pool_head, #define ast_string_field_ptr_build_va(x, ptr, fmt, args) \ ({ \ int __res__ = -1; \ - if (((void *)(x)) != NULL) { \ + if (((void *)(x)) != (void *)NULL) { \ __ast_string_field_ptr_build_va(&(x)->__field_mgr, &(x)->__field_mgr_pool, (ast_string_field *) ptr, fmt, args, \ __FILE__, __LINE__, __PRETTY_FUNCTION__); \ __res__ = 0; \ @@ -588,7 +591,7 @@ void __ast_string_field_release_active(struct ast_string_field_pool *pool_head, #define ast_string_field_build_va(x, field, fmt, args) \ ({ \ int __res__ = -1; \ - if (((void *)(x)) != NULL) { \ + if (((void *)(x)) != (void *)NULL) { \ __ast_string_field_ptr_build_va(&(x)->__field_mgr, &(x)->__field_mgr_pool, (ast_string_field *) &(x)->field, fmt, args, \ __FILE__, __LINE__, __PRETTY_FUNCTION__); \ __res__ = 0; \ @@ -607,7 +610,7 @@ void __ast_string_field_release_active(struct ast_string_field_pool *pool_head, #define ast_string_fields_cmp(instance1, instance2) \ ({ \ int __res__ = -1; \ - if (((void *)(instance1)) != NULL && ((void *)(instance2)) != NULL) { \ + if (((void *)(instance1)) != (void *)NULL && ((void *)(instance2)) != (void *)NULL) { \ __res__ = __ast_string_fields_cmp(&(instance1)->__field_mgr.string_fields, \ &(instance2)->__field_mgr.string_fields); \ } \ @@ -627,7 +630,7 @@ int __ast_string_fields_cmp(struct ast_string_field_vector *left, struct ast_str #define ast_string_fields_copy(copy, orig) \ ({ \ int __res__ = -1; \ - if (((void *)(copy)) != NULL && ((void *)(orig)) != NULL) { \ + if (((void *)(copy)) != (void *)NULL && ((void *)(orig)) != (void *)NULL) { \ __res__ = __ast_string_fields_copy(((copy)->__field_mgr_pool), \ (struct ast_string_field_mgr *)&((copy)->__field_mgr), \ (struct ast_string_field_mgr *)&((orig)->__field_mgr), \ diff --git a/include/asterisk/strings.h b/include/asterisk/strings.h index ab46273b60..93983c3e2b 100644 --- a/include/asterisk/strings.h +++ b/include/asterisk/strings.h @@ -393,11 +393,13 @@ char *ast_escape_c_alloc(const char *s); AST_INLINE_API( void ast_copy_string(char *dst, const char *src, size_t size), { - while (*src && size) { - *dst++ = *src++; - size--; + volatile size_t sz = size; + volatile char *sp = (char *)src; + while (*sp && sz) { + *dst++ = *sp++; + sz--; } - if (__builtin_expect(!size, 0)) + if (__builtin_expect(!sz, 0)) dst--; *dst = '\0'; } diff --git a/main/pbx.c b/main/pbx.c index 1e08f233dc..3a701aa058 100644 --- a/main/pbx.c +++ b/main/pbx.c @@ -1652,6 +1652,7 @@ static const char *get_pattern_node(struct pattern_node *node, const char *src, #undef INC_DST_OVERFLOW_CHECK } +#define MAX_EXTENBUF_SIZE 512 static struct match_char *add_exten_to_pattern_tree(struct ast_context *con, struct ast_exten *e1, int findonly) { struct match_char *m1 = NULL; @@ -1662,11 +1663,13 @@ static struct match_char *add_exten_to_pattern_tree(struct ast_context *con, str int pattern = 0; int idx_cur; int idx_next; - char extenbuf[512]; + char extenbuf[MAX_EXTENBUF_SIZE]; + volatile size_t required_space = strlen(e1->exten) + 1; struct pattern_node pat_node[2]; if (e1->matchcid) { - if (sizeof(extenbuf) < strlen(e1->exten) + strlen(e1->cidmatch) + 2) { + required_space += (strlen(e1->cidmatch) + 2 /* '/' + NULL */); + if (required_space > MAX_EXTENBUF_SIZE) { ast_log(LOG_ERROR, "The pattern %s/%s is too big to deal with: it will be ignored! Disaster!\n", e1->exten, e1->cidmatch); @@ -1674,7 +1677,13 @@ static struct match_char *add_exten_to_pattern_tree(struct ast_context *con, str } sprintf(extenbuf, "%s/%s", e1->exten, e1->cidmatch);/* Safe. We just checked. */ } else { - ast_copy_string(extenbuf, e1->exten, sizeof(extenbuf)); + if (required_space > MAX_EXTENBUF_SIZE) { + ast_log(LOG_ERROR, + "The pattern %s/%s is too big to deal with: it will be ignored! Disaster!\n", + e1->exten, e1->cidmatch); + return NULL; + } + ast_copy_string(extenbuf, e1->exten, required_space); } #ifdef NEED_DEBUG diff --git a/main/stun.c b/main/stun.c index 8007f3a3ed..06e6d9ff29 100644 --- a/main/stun.c +++ b/main/stun.c @@ -370,7 +370,13 @@ int ast_stun_handle_packet(int s, struct sockaddr_in *src, unsigned char *data, st.username ? st.username : ""); if (st.username) { append_attr_string(&attr, STUN_USERNAME, st.username, &resplen, &respleft); - snprintf(combined, sizeof(combined), "%16s%16s", st.username + 16, st.username); + /* + * For Google Voice, the stun username is made up of the local + * and remote usernames, each being fixed at 16 bytes. We have + * to swap the two at this point. + */ + snprintf(combined, 17, "%16s", st.username + 16); + snprintf(combined + 16, 17, "%16s", st.username); } else { combined[0] = '\0'; } diff --git a/res/res_config_pgsql.c b/res/res_config_pgsql.c index dbcd9e77b6..62a3565e02 100644 --- a/res/res_config_pgsql.c +++ b/res/res_config_pgsql.c @@ -1287,7 +1287,7 @@ static int require_pgsql(const char *database, const char *tablename, va_list ap res = -1; } else { struct ast_str *sql = ast_str_create(100); - char fieldtype[10]; + char fieldtype[20]; PGresult *result; if (requirements == RQ_CREATECHAR || type == RQ_CHAR) { diff --git a/res/res_tonedetect.c b/res/res_tonedetect.c index 45afe5b084..055142bbef 100644 --- a/res/res_tonedetect.c +++ b/res/res_tonedetect.c @@ -367,7 +367,7 @@ static int detect_callback(struct ast_audiohook *audiohook, struct ast_channel * return 0; } - if (!(direction == AST_AUDIOHOOK_DIRECTION_READ ? &di->rx : &di->tx)) { + if (!(direction == AST_AUDIOHOOK_DIRECTION_READ ? di->rx : di->tx)) { return 0; } diff --git a/tests/test_vector.c b/tests/test_vector.c index 2dfcc60a8c..5f6f0a54b3 100644 --- a/tests/test_vector.c +++ b/tests/test_vector.c @@ -48,7 +48,7 @@ static void cleanup(char *element) } #define STRING_CMP(a, b) ({ \ - ((a) == NULL || (b) == NULL) ? -1 : (strcmp((a), (b)) == 0); \ + ((void *)(a) == (void *)NULL || (void *)(b) == (void *)NULL) ? -1 : (strcmp((a), (b)) == 0); \ }) AST_TEST_DEFINE(basic_ops)