From: Arran Cudbard-Bell Date: Wed, 17 Mar 2021 13:21:02 +0000 (+0000) Subject: Add more verification calls for value box lists X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f8f6ae12541fa284a16b4df23eb9b9681a826bf9;p=thirdparty%2Ffreeradius-server.git Add more verification calls for value box lists --- diff --git a/src/lib/unlang/map.c b/src/lib/unlang/map.c index b1199745617..6c3ff9ff645 100644 --- a/src/lib/unlang/map.c +++ b/src/lib/unlang/map.c @@ -297,13 +297,9 @@ static unlang_action_t map_proc_apply(rlm_rcode_t *p_result, request_t *request, /* * FIXME - We don't yet support async LHS/RHS expansions for map procs */ -#ifndef NDEBUG - if (!fr_dlist_empty(&map_proc_state->src_result)) fr_dlist_verify(&map_proc_state->src_result); -#endif + FR_DLIST_VERIFY(&map_proc_state->src_result); *p_result = map_proc(request, gext->proc_inst, &map_proc_state->src_result); -#ifndef NDEBUG - if (!fr_dlist_empty(&map_proc_state->src_result)) fr_dlist_verify(&map_proc_state->src_result); -#endif + FR_DLIST_VERIFY(&map_proc_state->src_result); return *p_result == RLM_MODULE_YIELD ? UNLANG_ACTION_YIELD : UNLANG_ACTION_CALCULATE_RESULT; } diff --git a/src/lib/unlang/xlat_eval.c b/src/lib/unlang/xlat_eval.c index 05ec78fd517..41f569ca6cf 100644 --- a/src/lib/unlang/xlat_eval.c +++ b/src/lib/unlang/xlat_eval.c @@ -923,9 +923,9 @@ xlat_action_t xlat_frame_eval_resume(TALLOC_CTX *ctx, fr_dcursor_t *out, * to debug problems if they free or change elements * and don't remove them from the list. */ - fr_dlist_verify(result); + VALUE_BOX_LIST_VERIFY(result); xa = resume(ctx, out, request, exp->call.inst, thread_inst->data, result, rctx); - fr_dlist_verify(result); + VALUE_BOX_LIST_VERIFY(result); RDEBUG2("EXPAND %%{%s:...}", exp->call.func->name); switch (xa) { @@ -984,7 +984,7 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out, ssize_t slen; if (!fr_dlist_empty(result)) { - fr_dlist_verify(result); + VALUE_BOX_LIST_VERIFY(result); result_str = fr_value_box_list_aprint(NULL, result, NULL, NULL); if (!result_str) return XLAT_ACTION_FAIL; } else { @@ -1024,6 +1024,7 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out, fr_value_box_bstrdup_buffer_shallow(NULL, value, NULL, str, false); } + VALUE_BOX_VERIFY(value); fr_dcursor_append(out, value); /* Append the result of the expansion */ talloc_free(result_str); xlat_debug_log_result(request, value); @@ -1049,8 +1050,7 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out, */ if (RDEBUG_ENABLED2) fr_value_box_list_acopy(NULL, &result_copy, result); - fr_dlist_verify(result); - + VALUE_BOX_LIST_VERIFY(result); xa = xlat_process_args(ctx, result, request, node->call.func->input_type, node->call.func->args); if (xa == XLAT_ACTION_FAIL) { if (RDEBUG_ENABLED2) fr_dlist_talloc_free(&result_copy); @@ -1059,7 +1059,7 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out, xa = node->call.func->func.async(ctx, out, request, node->call.inst->data, thread_inst->data, result); - fr_dlist_verify(result); + VALUE_BOX_LIST_VERIFY(result); if (RDEBUG_ENABLED2) { xlat_debug_log_expansion(request, *in, &result_copy); @@ -1123,7 +1123,7 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out, xlat_debug_log_expansion(request, *in, NULL); xlat_debug_log_list_result(request, result); - fr_dlist_verify(result); + VALUE_BOX_LIST_VERIFY(result); fr_dcursor_init(&from, result); fr_dcursor_merge(out, &from); fr_assert(fr_dlist_empty(result)); @@ -1138,7 +1138,8 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out, node->fmt); fr_assert(!fr_dlist_empty(result)); - fr_dlist_verify(result); + + VALUE_BOX_LIST_VERIFY(result); /* * If the input is a series of xlat expansions, @@ -1152,7 +1153,6 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out, MEM(value = fr_value_box_alloc_null(ctx)); if (!fr_dlist_empty(result)) { - fr_dlist_verify(result); str = fr_value_box_list_aprint(value, result, NULL, NULL); if (!str) return XLAT_ACTION_FAIL; } else { @@ -1163,6 +1163,7 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out, fr_dlist_talloc_free(result); fr_dlist_insert_head(result, value); } + VALUE_BOX_LIST_VERIFY(result); MEM(value = fr_value_box_alloc(ctx, FR_TYPE_GROUP, NULL, false)); fr_dlist_move(&value->vb_group, result); diff --git a/src/lib/util/dlist.h b/src/lib/util/dlist.h index 966b595dd05..bb1889e4e71 100644 --- a/src/lib/util/dlist.h +++ b/src/lib/util/dlist.h @@ -565,22 +565,26 @@ static inline void *fr_dlist_replace(fr_dlist_head_t *list_head, void *item, voi * Does nothing if the list was not initialised with #fr_dlist_talloc_init. */ #ifndef TALLOC_GET_TYPE_ABORT_NOOP -static inline CC_HINT(nonnull) void fr_dlist_verify(fr_dlist_head_t *list_head) +static inline CC_HINT(nonnull) void fr_dlist_verify(char const *file, int line, fr_dlist_head_t *list_head) { void *item; if (!list_head->type) return; + fr_assert_msg(fr_dlist_initialised(list_head), "CONSISTENCY CHECK FAILED %s[%i]: dlist not initialised", + file, line); + for (item = fr_dlist_head(list_head); item; item = fr_dlist_next(list_head, item)) { item = _talloc_get_type_abort(item, list_head->type, __location__); } } +# define FR_DLIST_VERIFY(_head) fr_dlist_verify(__FILE__, __LINE__, _head) #elif !defined(NDEBUG) -# define fr_dlist_verify(_head) fr_assert(_head) +# define FR_DLIST_VERIFY(_head) fr_assert(_head) #else -# define fr_dlist_verify(_head) +# define FR_DLIST_VERIFY(_head) #endif /** Merge two lists, inserting the tail of one into the other diff --git a/src/lib/util/value.c b/src/lib/util/value.c index 5488a2888bb..6a0a325ac60 100644 --- a/src/lib/util/value.c +++ b/src/lib/util/value.c @@ -4804,8 +4804,6 @@ ssize_t fr_value_box_print(fr_sbuff_t *out, fr_value_box_t const *data, fr_sbuff fr_value_box_t vb; ssize_t slen; - memset(&vb, 0, sizeof(vb)); - /* * Be lazy by just converting it to a string, and then printing the string. */ @@ -5176,3 +5174,48 @@ bool fr_value_box_list_tainted(fr_value_box_list_t const *head) return false; } + +/** Validation function to check that a fr_value_box_t is correctly initialised + * + * @note Do not add talloc checks here. fr_value_box_t may be allocated on the stack. + */ +void value_box_verify(char const *file, int line, fr_value_box_t const *vb) +{ + fr_fatal_assert_msg(vb, "CONSISTENCY CHECK FAILED %s[%i]: fr_value_box_t pointer was NULL", file, line); + + switch (vb->type) { + case FR_TYPE_STRING: + fr_fatal_assert_msg(vb->vb_strvalue, "CONSISTENCY CHECK FAILED %s[%i]: fr_value_box_t strvalue field " + "was NULL", file, line); + fr_fatal_assert_msg(vb->vb_strvalue[vb->vb_length] == '\0', + "CONSISTENCY CHECK FAILED %s[%i]: fr_value_box_t strvalue field " + "not null terminated", file, line); + break; + + case FR_TYPE_OCTETS: + fr_fatal_assert_msg(vb->vb_octets, "CONSISTENCY CHECK FAILED %s[%i]: fr_value_box_t octets field " + "was NULL", file, line); + break; + + case FR_TYPE_VOID: + fr_fatal_assert_msg(vb->vb_void, "CONSISTENCY CHECK FAILED %s[%i]: fr_value_box_t ptr field " + "was NULL", file, line); + break; + + case FR_TYPE_GROUP: + value_box_list_verify(file, line, &vb->vb_group); + break; + + default: + break; + } +} + +void value_box_list_verify(char const *file, int line, fr_value_box_list_t const *list) +{ + fr_value_box_t const *vb; + + FR_DLIST_VERIFY(list); + + while ((vb = fr_dlist_next(list, vb))) value_box_verify(file, line, list); +} diff --git a/src/lib/util/value.h b/src/lib/util/value.h index dbb25500c23..466fce1d4b8 100644 --- a/src/lib/util/value.h +++ b/src/lib/util/value.h @@ -165,6 +165,7 @@ struct value_box_s { */ #define vb_strvalue datum.strvalue #define vb_octets datum.octets +#define vb_void datum.ptr #define vb_group datum.children #define vb_ip datum.ip @@ -735,8 +736,9 @@ bool fr_value_box_list_tainted(fr_value_box_list_t const *head); int fr_value_box_list_flatten_argv(TALLOC_CTX *ctx, char ***argv_p, fr_value_box_list_t const *in); /** @} */ -/* - * Printing +/** @name Print the value of a value box as a string + * + * @{ */ ssize_t fr_value_box_print(fr_sbuff_t *out, fr_value_box_t const *data, fr_sbuff_escape_rules_t const *e_rules) CC_HINT(nonnull(1,2)); @@ -753,6 +755,7 @@ static inline size_t fr_value_box_aprint_quoted(TALLOC_CTX *ctx, char **out, { SBUFF_OUT_TALLOC_FUNC_NO_LEN_DEF(fr_value_box_print_quoted, data, quote) } +/** @} */ /** @name Hashing * * @{ @@ -760,6 +763,22 @@ static inline size_t fr_value_box_aprint_quoted(TALLOC_CTX *ctx, char **out, uint32_t fr_value_box_hash_update(fr_value_box_t const *vb, uint32_t hash); /** @} */ +void value_box_verify(char const *file, int line, fr_value_box_t const *vb); +void value_box_list_verify(char const *file, int line, fr_value_box_list_t const *list); + +#ifdef WITH_VERIFY_PTR +# define VALUE_BOX_VERIFY(_x) value_box_verify(__FILE__, __LINE__, _x) +# define VALUE_BOX_LIST_VERIFY(_x) value_box_list_verify(__FILE__, __LINE__, _x) +#else +/* + * Even if were building without WITH_VERIFY_PTR + * the pointer must not be NULL when these various macros are used + * so we can add some sneaky asserts. + */ +# define VALUE_BOX_VERIFY(_x) fr_assert(_x) +# define VALUE_BOX_LIST_VERIFY(_x) fr_assert(_x) +#endif + #undef _CONST #ifdef __cplusplus