From f2213dbd1b51838ec6b59073d9d071a75def7858 Mon Sep 17 00:00:00 2001 From: Alex Colomar Date: Sat, 10 Sep 2022 18:00:27 +0200 Subject: Added missing error checking in the C API. pthread_mutex_init(3) may fail for several reasons, and failing to check will cause Undefined Behavior when those errors happen. Add missing checks, and correctly deinitialize previously created stuff before exiting from the API. Signed-off-by: Alejandro Colomar Reviewed-by: Andrew Clayton Reviewed-by: Zhidao HONG --- src/nxt_unit.c | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) (limited to 'src/nxt_unit.c') diff --git a/src/nxt_unit.c b/src/nxt_unit.c index e5cb0b58..3932b2ab 100644 --- a/src/nxt_unit.c +++ b/src/nxt_unit.c @@ -116,7 +116,7 @@ static int nxt_unit_incoming_mmap(nxt_unit_ctx_t *ctx, pid_t pid, int fd); static void nxt_unit_awake_ctx(nxt_unit_ctx_t *ctx, nxt_unit_ctx_impl_t *ctx_impl); -static void nxt_unit_mmaps_init(nxt_unit_mmaps_t *mmaps); +static int nxt_unit_mmaps_init(nxt_unit_mmaps_t *mmaps); nxt_inline void nxt_unit_process_use(nxt_unit_process_t *process); nxt_inline void nxt_unit_process_release(nxt_unit_process_t *process); static void nxt_unit_mmaps_destroy(nxt_unit_mmaps_t *mmaps); @@ -606,7 +606,7 @@ nxt_unit_create(nxt_unit_init_t *init) if (nxt_slow_path(rc != 0)) { nxt_unit_alert(NULL, "failed to initialize mutex (%d)", rc); - goto fail; + goto out_unit_free; } lib->unit.data = init->data; @@ -631,17 +631,35 @@ nxt_unit_create(nxt_unit_init_t *init) rc = nxt_unit_ctx_init(lib, &lib->main_ctx, init->ctx_data); if (nxt_slow_path(rc != NXT_UNIT_OK)) { - pthread_mutex_destroy(&lib->mutex); - goto fail; + goto out_mutex_destroy; + } + + rc = nxt_unit_mmaps_init(&lib->incoming); + if (nxt_slow_path(rc != 0)) { + nxt_unit_alert(NULL, "failed to initialize mutex (%d)", rc); + + goto out_ctx_free; } - nxt_unit_mmaps_init(&lib->incoming); - nxt_unit_mmaps_init(&lib->outgoing); + rc = nxt_unit_mmaps_init(&lib->outgoing); + if (nxt_slow_path(rc != 0)) { + nxt_unit_alert(NULL, "failed to initialize mutex (%d)", rc); + + goto out_mmaps_destroy; + } return lib; -fail: +out_mmaps_destroy: + nxt_unit_mmaps_destroy(&lib->incoming); + +out_ctx_free: + nxt_unit_ctx_free(&lib->main_ctx); + +out_mutex_destroy: + pthread_mutex_destroy(&lib->mutex); +out_unit_free: nxt_unit_free(NULL, lib); return NULL; @@ -4093,15 +4111,15 @@ nxt_unit_awake_ctx(nxt_unit_ctx_t *ctx, nxt_unit_ctx_impl_t *ctx_impl) } -static void +static int nxt_unit_mmaps_init(nxt_unit_mmaps_t *mmaps) { - pthread_mutex_init(&mmaps->mutex, NULL); - mmaps->size = 0; mmaps->cap = 0; mmaps->elts = NULL; mmaps->allocated_chunks = 0; + + return pthread_mutex_init(&mmaps->mutex, NULL); } -- cgit From e70653c76606293e3687b05bbd8b5c06d700fb8b Mon Sep 17 00:00:00 2001 From: Andrew Clayton Date: Tue, 6 Dec 2022 20:57:16 +0000 Subject: Fix compilation with GCC and -O0. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Andrei reported an issue with building unit when using '-O0' with GCC producing the following compiler errors cc -c -pipe -fPIC -fvisibility=hidden -O -W -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wmissing-prototypes -Werror -g -O0 -I src -I build \ \ \ -o build/src/nxt_unit.o \ -MMD -MF build/src/nxt_unit.dep -MT build/src/nxt_unit.o \ src/nxt_unit.c src/nxt_unit.c: In function ‘nxt_unit_log’: src/nxt_unit.c:6601:9: error: ‘msg’ may be used uninitialized [-Werror=maybe-uninitialized] 6601 | p = nxt_unit_snprint_prefix(p, end, pid, level); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ src/nxt_unit.c:6682:1: note: by argument 2 of type ‘const char *’ to ‘nxt_unit_snprint_prefix’ declared here 6682 | nxt_unit_snprint_prefix(char *p, const char *end, pid_t pid, int level) | ^~~~~~~~~~~~~~~~~~~~~~~ src/nxt_unit.c:6582:22: note: ‘msg’ declared here 6582 | char msg[NXT_MAX_ERROR_STR], *p, *end; | ^~~ src/nxt_unit.c: In function ‘nxt_unit_req_log’: src/nxt_unit.c:6645:9: error: ‘msg’ may be used uninitialized [-Werror=maybe-uninitialized] 6645 | p = nxt_unit_snprint_prefix(p, end, pid, level); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ src/nxt_unit.c:6682:1: note: by argument 2 of type ‘const char *’ to ‘nxt_unit_snprint_prefix’ declared here 6682 | nxt_unit_snprint_prefix(char *p, const char *end, pid_t pid, int level) | ^~~~~~~~~~~~~~~~~~~~~~~ src/nxt_unit.c:6625:35: note: ‘msg’ declared here 6625 | char msg[NXT_MAX_ERROR_STR], *p, *end; | ^~~ cc1: all warnings being treated as errors The above was reproduced with $ ./configure --cc-opt=-O0 && ./configure python && make -j4 This warning doesn't happen on clang (15.0.4) or GCC (8.3) and seems to have been introduced in GCC 11. The above is from GCC (12.2.1, Fedora 37). The trigger of this GCC issue is actually part of a commit I introduced a few months back to constify some function parameters and it seems the consensus for how to resolve this problem is to simply remove the const qualifier from the *end parameter to nxt_unit_snprint_prefix(). Reported-by: Andrei Zeliankou Link: Link: Link: Fixes: 4418f99 ("Constified numerous function parameters.") Signed-off-by: Andrew Clayton --- src/nxt_unit.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/nxt_unit.c') diff --git a/src/nxt_unit.c b/src/nxt_unit.c index 3932b2ab..e1b1897a 100644 --- a/src/nxt_unit.c +++ b/src/nxt_unit.c @@ -196,7 +196,7 @@ static int nxt_unit_request_hash_add(nxt_unit_ctx_t *ctx, static nxt_unit_request_info_t *nxt_unit_request_hash_find( nxt_unit_ctx_t *ctx, uint32_t stream, int remove); -static char * nxt_unit_snprint_prefix(char *p, const char *end, pid_t pid, +static char * nxt_unit_snprint_prefix(char *p, char *end, pid_t pid, int level); static void *nxt_unit_lvlhsh_alloc(void *data, size_t size); static void nxt_unit_lvlhsh_free(void *data, void *p); @@ -6679,7 +6679,7 @@ static const char * nxt_unit_log_levels[] = { static char * -nxt_unit_snprint_prefix(char *p, const char *end, pid_t pid, int level) +nxt_unit_snprint_prefix(char *p, char *end, pid_t pid, int level) { struct tm tm; struct timespec ts; -- cgit