From 2bb3cb15cc26b18e11e28c6d7ec1a7753afdbe06 Mon Sep 17 00:00:00 2001 From: Arran Cudbard-Bell Date: Mon, 25 Jan 2021 14:33:50 +0000 Subject: [PATCH] Add function for explicitly clearing outstanding messages --- src/bin/fuzzer.c | 5 ++-- src/bin/radclient.c | 4 +-- src/bin/radiusd.c | 4 +-- src/bin/radsniff.c | 4 +-- src/bin/radsnmp.c | 6 ++-- src/bin/radwho.c | 2 +- src/bin/unit_test_attribute.c | 8 +++--- src/lib/redis/redis.c | 2 +- src/lib/server/dl_module.c | 2 +- src/lib/server/tmpl_tokenize.c | 2 +- src/lib/unlang/xlat_tokenize.c | 4 +-- src/lib/util/dict_tokenize.c | 2 +- src/lib/util/dl.c | 4 +-- src/lib/util/socket.c | 2 +- src/lib/util/strerror.c | 36 +++++++++++++----------- src/lib/util/strerror.h | 29 ++++++++++--------- src/lib/util/strerror_tests.c | 8 +++--- src/modules/proto_dhcpv4/dhcpclient.c | 4 +-- src/modules/proto_ldap_sync/sync_touch.c | 2 +- 19 files changed, 67 insertions(+), 63 deletions(-) diff --git a/src/bin/fuzzer.c b/src/bin/fuzzer.c index fd7ef7c673..8505da966f 100644 --- a/src/bin/fuzzer.c +++ b/src/bin/fuzzer.c @@ -78,7 +78,7 @@ int LLVMFuzzerInitialize(int *argc, char ***argv) * is generated. */ fr_strerror_const("fuzz"); /* allocate the pools */ - fr_strerror(); /* clears the message, leaves the pools */ + fr_strerror_clear(); /* clears the message, leaves the pools */ /* * Setup our own internal atexit handler @@ -191,14 +191,13 @@ int LLVMFuzzerTestOneInput(const uint8_t *buf, size_t len) tp->func(ctx, &vps, buf, len, decode_ctx); talloc_free(ctx); - fr_strerror(); /* Clear any undrained errors */ /* * Clear error messages from the run. Clearing these * keeps malloc/free balanced, which helps to avoid the * fuzzers leak heuristics from firing. */ - fr_strerror_const(NULL); + fr_strerror_clear(); return 0; } diff --git a/src/bin/radclient.c b/src/bin/radclient.c index 9573296a62..c5cba43e9a 100644 --- a/src/bin/radclient.c +++ b/src/bin/radclient.c @@ -1074,7 +1074,7 @@ static int recv_one_packet(fr_time_t wait_time) stats.rejected++; } - fr_strerror(); /* Clear strerror buffer */ + fr_strerror_clear(); /* Clear strerror buffer */ /* * If we had an expected response code, check to see if the @@ -1373,7 +1373,7 @@ int main(int argc, char **argv) "Failed to initialize the dictionaries"); return 1; } - fr_strerror(); /* Clear the error buffer */ + fr_strerror_clear(); /* Clear the error buffer */ /* * Get the request type diff --git a/src/bin/radiusd.c b/src/bin/radiusd.c index e15c5bc0d0..7890778ce1 100644 --- a/src/bin/radiusd.c +++ b/src/bin/radiusd.c @@ -901,7 +901,7 @@ int main(int argc, char *argv[]) /* * Clear the libfreeradius error buffer. */ - fr_strerror(); + fr_strerror_clear(); /* * Prevent anything from modifying the dictionaries @@ -1066,7 +1066,7 @@ cleanup: * we don't inteferere with the onexit() handler. */ if (!rad_suid_is_down_permanent() && (fr_get_lsan_state() == 1)) rad_suid_up(); - fr_strerror(); /* clear error buffer */ + fr_strerror_clear(); /* clear error buffer */ return ret; } diff --git a/src/bin/radsniff.c b/src/bin/radsniff.c index 579afc5775..c8fee2bf7e 100644 --- a/src/bin/radsniff.c +++ b/src/bin/radsniff.c @@ -2648,7 +2648,7 @@ int main(int argc, char *argv[]) return 1; } - fr_strerror(); /* Clear out any non-fatal errors */ + fr_strerror_clear(); /* Clear out any non-fatal errors */ if (conf->list_attributes) { conf->list_da_num = rs_build_dict_list(conf->list_da, NUM_ELEMENTS(conf->list_da), @@ -2900,7 +2900,7 @@ int main(int argc, char *argv[]) } /* Clear any irrelevant errors */ - fr_strerror(); + fr_strerror_clear(); } /* diff --git a/src/bin/radsnmp.c b/src/bin/radsnmp.c index 48b93b993c..fd4846cc0f 100644 --- a/src/bin/radsnmp.c +++ b/src/bin/radsnmp.c @@ -293,7 +293,7 @@ static ssize_t radsnmp_pair_from_oid(TALLOC_CTX *ctx, radsnmp_conf_t *conf, fr_d return slen; } - fr_strerror(); /* Clear pending errors */ + fr_strerror_clear(); /* Clear pending errors */ /* * SNMP requests the leaf under the OID with .0. @@ -622,7 +622,7 @@ static int radsnmp_set_response(int fd, fr_dict_attr_t const *error, fr_pair_lis static int radsnmp_send_recv(radsnmp_conf_t *conf, int fd) { - fr_strerror(); + fr_strerror_clear(); #define NEXT_LINE(_line, _buffer) \ { \ @@ -1059,7 +1059,7 @@ int main(int argc, char **argv) fr_perror("radsnmp"); fr_exit_now(EXIT_FAILURE); } - fr_strerror(); /* Clear the error buffer */ + fr_strerror_clear(); /* Clear the error buffer */ /* * Get the request type diff --git a/src/bin/radwho.c b/src/bin/radwho.c index 731a845bda..513507b215 100644 --- a/src/bin/radwho.c +++ b/src/bin/radwho.c @@ -320,7 +320,7 @@ int main(int argc, char **argv) PERROR("Failed to initialize the dictionaries"); return 1; } - fr_strerror(); /* Clear the error buffer */ + fr_strerror_clear(); /* Clear the error buffer */ /* * Be safe. diff --git a/src/bin/unit_test_attribute.c b/src/bin/unit_test_attribute.c index 11e36168b4..4242581049 100644 --- a/src/bin/unit_test_attribute.c +++ b/src/bin/unit_test_attribute.c @@ -1327,7 +1327,7 @@ static size_t command_decode_pair(command_result_t *result, command_file_ctx_t * /* * Clear any spurious errors */ - fr_strerror(); + fr_strerror_clear(); ASAN_UNPOISON_MEMORY_REGION(to_dec_end, COMMAND_OUTPUT_MAX - slen); /* @@ -1434,7 +1434,7 @@ static size_t command_decode_proto(command_result_t *result, command_file_ctx_t /* * Clear any spurious errors */ - fr_strerror(); + fr_strerror_clear(); ASAN_UNPOISON_MEMORY_REGION(to_dec_end, COMMAND_OUTPUT_MAX - slen); /* @@ -1726,7 +1726,7 @@ static size_t command_encode_pair(command_result_t *result, command_file_ctx_t * /* * Clear any spurious errors */ - fr_strerror(); + fr_strerror_clear(); fr_pair_list_free(&head); @@ -1803,7 +1803,7 @@ static size_t command_encode_proto(command_result_t *result, command_file_ctx_t /* * Clear any spurious errors */ - fr_strerror(); + fr_strerror_clear(); CLEAR_TEST_POINT(cc); diff --git a/src/lib/redis/redis.c b/src/lib/redis/redis.c index bac252077a..aa83f45c18 100644 --- a/src/lib/redis/redis.c +++ b/src/lib/redis/redis.c @@ -437,7 +437,7 @@ fr_redis_rcode_t fr_redis_pipeline_result(unsigned int *pipelined, fr_redis_rcod fr_assert(out_len >= (size_t)*pipelined); - fr_strerror(); /* Clear any outstanding errors */ + fr_strerror_clear(); /* Clear any outstanding errors */ if ((size_t) *pipelined > out_len) { for (i = 0; i < (size_t)*pipelined; i++) { diff --git a/src/lib/server/dl_module.c b/src/lib/server/dl_module.c index 30e9da5745..7a14f5b516 100644 --- a/src/lib/server/dl_module.c +++ b/src/lib/server/dl_module.c @@ -105,7 +105,7 @@ static int dl_module_onload_func(dl_t const *dl, UNUSED void *symbol, UNUSED voi /* * Clear pre-existing errors. */ - fr_strerror(); + fr_strerror_clear(); if (dl_module->common->onload) { int ret; diff --git a/src/lib/server/tmpl_tokenize.c b/src/lib/server/tmpl_tokenize.c index e5cfc71745..1f1ce1a4d5 100644 --- a/src/lib/server/tmpl_tokenize.c +++ b/src/lib/server/tmpl_tokenize.c @@ -1538,7 +1538,7 @@ static inline int tmpl_attr_afrom_attr_substr(TALLOC_CTX *ctx, tmpl_attr_error_t if (fr_sbuff_out(NULL, &oid, name) > 0) { fr_dict_attr_t *da_unknown; - fr_strerror(); /* Clear out any existing errors */ + fr_strerror_clear(); /* Clear out any existing errors */ /* * If it's numeric and not a known attribute diff --git a/src/lib/unlang/xlat_tokenize.c b/src/lib/unlang/xlat_tokenize.c index 87649d2018..55e132b938 100644 --- a/src/lib/unlang/xlat_tokenize.c +++ b/src/lib/unlang/xlat_tokenize.c @@ -1227,7 +1227,7 @@ ssize_t xlat_tokenize_ephemeral(TALLOC_CTX *ctx, xlat_exp_t **head, xlat_flags_t *head = NULL; - fr_strerror(); /* Clear error buffer */ + fr_strerror_clear(); /* Clear error buffer */ if (xlat_tokenize_literal(ctx, head, flags, &our_in, false, p_rules, t_rules) < 0) return -fr_sbuff_used(&our_in); @@ -1445,7 +1445,7 @@ ssize_t xlat_tokenize(TALLOC_CTX *ctx, xlat_exp_t **head, xlat_flags_t *flags, f if (!flags) flags = &tmp_flags; *head = NULL; - fr_strerror(); /* Clear error buffer */ + fr_strerror_clear(); /* Clear error buffer */ if (xlat_tokenize_literal(ctx, head, flags, &our_in, false, p_rules, t_rules) < 0) return -fr_sbuff_used(&our_in); diff --git a/src/lib/util/dict_tokenize.c b/src/lib/util/dict_tokenize.c index 1be89825a2..4f126f192b 100644 --- a/src/lib/util/dict_tokenize.c +++ b/src/lib/util/dict_tokenize.c @@ -1842,7 +1842,7 @@ static int _dict_from_file(dict_tokenize_ctx_t *ctx, ret = _dict_from_file(ctx, dir, argv[1], fn, line); if ((ret == -2) && (argv[0][8] == '-')) { - fr_strerror_printf(NULL); /* delete all errors */ + fr_strerror_clear(); /* delete all errors */ ret = 0; } diff --git a/src/lib/util/dl.c b/src/lib/util/dl.c index ae754079f2..adb8d827b5 100644 --- a/src/lib/util/dl.c +++ b/src/lib/util/dl.c @@ -477,7 +477,7 @@ dl_t *dl_by_name(dl_loader_t *dl_loader, char const *name, void *uctx, bool uctx */ #if defined(RTLD_DEEPBIND) && !defined(__SANITIZE_ADDRESS__) flags |= RTLD_DEEPBIND; - fr_strerror(); /* clear error buffer */ + fr_strerror_clear(); /* clear error buffer */ #endif /* @@ -494,7 +494,7 @@ dl_t *dl_by_name(dl_loader_t *dl_loader, char const *name, void *uctx, bool uctx char *ctx, *paths, *path; char *p; - fr_strerror(); + fr_strerror_clear(); ctx = paths = talloc_typed_strdup(NULL, search_path); while ((path = strsep(&paths, ":")) != NULL) { diff --git a/src/lib/util/socket.c b/src/lib/util/socket.c index 58f7c2cc55..749c4f6dda 100644 --- a/src/lib/util/socket.c +++ b/src/lib/util/socket.c @@ -1042,7 +1042,7 @@ done: * Clear any errors we may have produced in the * capabilities check. */ - fr_strerror(); + fr_strerror_clear(); #endif return 0; } diff --git a/src/lib/util/strerror.c b/src/lib/util/strerror.c index 28b69c34b2..9746ee92fb 100644 --- a/src/lib/util/strerror.c +++ b/src/lib/util/strerror.c @@ -155,9 +155,7 @@ static fr_log_entry_t *strerror_vprintf(char const *fmt, va_list ap) * Clear any existing log messages */ if (!fmt) { - talloc_free_children(buffer->pool_a); - talloc_free_children(buffer->pool_b); - fr_dlist_clear(&buffer->entries); + fr_strerror_clear(); return NULL; } @@ -382,7 +380,7 @@ void fr_strerror_marker_printf_push_head(char const *subject, size_t offset, cha * * @hidecallergraph */ -static inline CC_HINT(always_inline) fr_log_entry_t *strerror_const(char const *msg) +static inline CC_HINT(always_inline) CC_HINT(nonnull) fr_log_entry_t *strerror_const(char const *msg) { fr_log_entry_t *entry; fr_log_buffer_t *buffer; @@ -390,16 +388,6 @@ static inline CC_HINT(always_inline) fr_log_entry_t *strerror_const(char const * buffer = fr_strerror_init(); if (unlikely(!buffer)) return NULL; - /* - * Clear any existing log messages - */ - if (!msg) { - fr_dlist_clear(&buffer->entries); - talloc_free_children(buffer->pool_a); - talloc_free_children(buffer->pool_b); - return NULL; - } - entry = talloc(pool_alt(buffer), fr_log_entry_t); if (unlikely(!entry)) { fr_perror("Failed allocating memory for libradius error buffer"); @@ -442,12 +430,11 @@ void fr_strerror_const(char const *msg) * * @hidecallergraph */ -static fr_log_entry_t *strerror_const_push(fr_log_buffer_t *buffer, char const *msg) +static CC_HINT(always_inline) CC_HINT(nonnull) +fr_log_entry_t *strerror_const_push(fr_log_buffer_t *buffer, char const *msg) { fr_log_entry_t *entry; - if (!msg) return NULL; - /* * Address pathological case where we could leak memory * if only a combination of fr_strerror and @@ -546,6 +533,21 @@ char const *fr_strerror(void) return entry->msg; } +/** Clears all pending messages from the talloc pools + * + */ +void fr_strerror_clear(void) +{ + fr_log_buffer_t *buffer; + + if (unlikely(!fr_strerror_buffer)) return; + + buffer = fr_strerror_init(); + fr_dlist_clear(&buffer->entries); + talloc_free_children(buffer->pool_a); + talloc_free_children(buffer->pool_b); +} + /** Get the last library error marker * * @param[out] subject The subject string the error relates to. diff --git a/src/lib/util/strerror.h b/src/lib/util/strerror.h index c4329e8b2a..cc1032d34b 100644 --- a/src/lib/util/strerror.h +++ b/src/lib/util/strerror.h @@ -41,13 +41,13 @@ extern "C" { * @{ */ /** @hidecallergraph */ -void fr_strerror_printf(char const *fmt, ...) CC_HINT(format (printf, 1, 2)); +void fr_strerror_printf(char const *fmt, ...) CC_HINT(nonnull) CC_HINT(format (printf, 1, 2)); /** @hidecallergraph */ -void fr_strerror_printf_push(char const *fmt, ...) CC_HINT(format (printf, 1, 2)); +void fr_strerror_printf_push(char const *fmt, ...) CC_HINT(nonnull) CC_HINT(format (printf, 1, 2)); /** @hidecallergraph */ -void fr_strerror_printf_push_head(char const *fmt, ...) CC_HINT(format (printf, 1, 2)); +void fr_strerror_printf_push_head(char const *fmt, ...) CC_HINT(nonnull) CC_HINT(format (printf, 1, 2)); /** @} */ /** @name Add an error string with marker to the thread local error stack @@ -59,13 +59,13 @@ void fr_strerror_printf_push_head(char const *fmt, ...) CC_HINT(format (printf * @{ */ /** @hidecallergraph */ -void fr_strerror_marker_printf(char const *subject, size_t offset, char const *fmt, ...) CC_HINT(format (printf, 3, 4)); +void fr_strerror_marker_printf(char const *subject, size_t offset, char const *fmt, ...) CC_HINT(nonnull) CC_HINT(format (printf, 3, 4)); /** @hidecallergraph */ -void fr_strerror_marker_printf_push(char const *subject, size_t offset, char const *fmt, ...) CC_HINT(format (printf, 3, 4)); +void fr_strerror_marker_printf_push(char const *subject, size_t offset, char const *fmt, ...) CC_HINT(nonnull) CC_HINT(format (printf, 3, 4)); /** @hidecallergraph */ -void fr_strerror_marker_printf_push_head(char const *subject, size_t offset, char const *fmt, ...) CC_HINT(format (printf, 3, 4)); +void fr_strerror_marker_printf_push_head(char const *subject, size_t offset, char const *fmt, ...) CC_HINT(nonnull) CC_HINT(format (printf, 3, 4)); /** @} */ /** @name Add a const error string to the thread local error stack @@ -75,13 +75,13 @@ void fr_strerror_marker_printf_push_head(char const *subject, size_t offset, ch * @{ */ /** @hidecallergraph */ -void fr_strerror_const(char const *msg); +void fr_strerror_const(char const *msg) CC_HINT(nonnull); /** @hidecallergraph */ -void fr_strerror_const_push(char const *msg); +void fr_strerror_const_push(char const *msg) CC_HINT(nonnull); /** @hidecallergraph */ -void fr_strerror_const_push_head(char const *msg); +void fr_strerror_const_push_head(char const *msg) CC_HINT(nonnull); /** @} */ /** @name Retrieve errors from the thread local error stack @@ -89,22 +89,25 @@ void fr_strerror_const_push_head(char const *msg); * @{ */ /** @hidecallergraph */ -char const *fr_strerror(void); +char const *fr_strerror(void) CC_HINT(warn_unused_result); /** @hidecallergraph */ -char const *fr_strerror_marker(char const **subject, size_t *offset); +void fr_strerror_clear(void); + +/** @hidecallergraph */ +char const *fr_strerror_marker(char const **subject, size_t *offset) CC_HINT(nonnull); /** @hidecallergraph */ char const *fr_strerror_peek(void); /** @hidecallergraph */ -char const *fr_strerror_marker_peek(char const **subject, size_t *offset); +char const *fr_strerror_marker_peek(char const **subject, size_t *offset) CC_HINT(nonnull); /** @hidecallergraph */ char const *fr_strerror_pop(void); /** @hidecallergraph */ -char const *fr_strerror_marker_pop(char const **subject, size_t *offset); +char const *fr_strerror_marker_pop(char const **subject, size_t *offset) CC_HINT(nonnull); /** @hidecallergraph */ void fr_perror(char const *, ...) CC_HINT(format (printf, 1, 2)); diff --git a/src/lib/util/strerror_tests.c b/src/lib/util/strerror_tests.c index 45a9931278..dd190e723c 100644 --- a/src/lib/util/strerror_tests.c +++ b/src/lib/util/strerror_tests.c @@ -186,12 +186,12 @@ static void strerror_printf_benchmark(void) uint64_t rate; fr_strerror_const("pre-allocate buffers"); - fr_strerror(); + fr_strerror_clear(); start = fr_time(); for (i = 0; i < 100000; i++) { fr_strerror_printf("I am a test %u string %u %s", i, i, "benchmark"); - fr_strerror(); /* Clear */ + fr_strerror_clear(); /* Clear */ } stop = fr_time(); @@ -209,12 +209,12 @@ static void strerror_const_benchmark(void) uint64_t rate; fr_strerror_const("pre-allocate buffers"); - fr_strerror(); + fr_strerror_clear(); start = fr_time(); for (i = 0; i < 100000; i++) { fr_strerror_const("I am a test string"); - fr_strerror(); /* Clear */ + fr_strerror_clear(); /* Clear */ } stop = fr_time(); diff --git a/src/modules/proto_dhcpv4/dhcpclient.c b/src/modules/proto_dhcpv4/dhcpclient.c index 183bd86b07..e07d83ba43 100644 --- a/src/modules/proto_dhcpv4/dhcpclient.c +++ b/src/modules/proto_dhcpv4/dhcpclient.c @@ -430,7 +430,7 @@ static int send_with_socket(fr_radius_packet_t **reply, fr_radius_packet_t *requ *reply = fr_dhcpv4_udp_packet_recv(sockfd); if (!*reply) { if (errno == EAGAIN) { - fr_strerror(); /* clear error */ + fr_strerror_clear(); /* clear error */ ERROR("Timed out waiting for reply"); } else { ERROR("Error receiving reply"); @@ -647,7 +647,7 @@ int main(int argc, char **argv) fr_perror("dhcpclient"); fr_exit(EXIT_FAILURE); } - fr_strerror(); /* Clear the error buffer */ + fr_strerror_clear(); /* Clear the error buffer */ /* * Initialise the DHCPv4 library diff --git a/src/modules/proto_ldap_sync/sync_touch.c b/src/modules/proto_ldap_sync/sync_touch.c index 74b5d3dc5b..8079242a1f 100644 --- a/src/modules/proto_ldap_sync/sync_touch.c +++ b/src/modules/proto_ldap_sync/sync_touch.c @@ -147,7 +147,7 @@ int main(int argc, char **argv) fr_perror("sync_touch"); fr_exit_now(EXIT_FAILURE); } - fr_strerror(); /* Clear the error buffer */ + fr_strerror_clear(); /* Clear the error buffer */ fr_set_signal(SIGPIPE, rs_signal_stop); fr_set_signal(SIGINT, rs_signal_stop); -- 2.47.2