From: Arran Cudbard-Bell Date: Tue, 26 May 2020 03:34:50 +0000 (-0500) Subject: pair/value: Remove final calls to memsteal and bstrsteal X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6b1fde655c997d6dcf376f3787f86bd9adaaa9cf;p=thirdparty%2Ffreeradius-server.git pair/value: Remove final calls to memsteal and bstrsteal --- diff --git a/src/lib/eap/tls.c b/src/lib/eap/tls.c index 39f1b7171b8..21fd86acc55 100644 --- a/src/lib/eap/tls.c +++ b/src/lib/eap/tls.c @@ -317,12 +317,14 @@ int eap_tls_success(REQUEST *request, eap_session_t *eap_session, uint8_t *session_id; VALUE_PAIR *vp; - if (eap_crypto_tls_session_id(request->parent->reply, request, tls_session->ssl, - &session_id, eap_session->type, - sessid_prf_label, sessid_prf_label_len) < 0) return -1; - MEM(pair_add_reply(&vp, attr_eap_session_id) >= 0); - fr_pair_value_memsteal(vp, session_id, true); + if (eap_crypto_tls_session_id(vp, request, tls_session->ssl, + &session_id, eap_session->type, + sessid_prf_label, sessid_prf_label_len) < 0) { + pair_delete_reply(vp); + return -1; + } + fr_pair_value_memdup_buffer_shallow(vp, session_id, false); RINDENT(); RDEBUG2("&reply:%pP", vp); diff --git a/src/lib/server/pair.h b/src/lib/server/pair.h index 3eade2849e3..8f2579948da 100644 --- a/src/lib/server/pair.h +++ b/src/lib/server/pair.h @@ -109,40 +109,52 @@ RCSIDH(server_pair_h, "$Id$") */ #define pair_update_session_state(_attr, _da) fr_pair_update_by_da(request->state_ctx, _attr, &request->state, _da) +/** Delete a VALUE_PAIR in a list + * + * @param[in] _list to delete the pair from. + * @param[in] _pair May be a VALUE_PAIR or fr_dict_attr_t. + */ +#define pair_delete(_list, _pair) \ + _Generic((_pair), \ + fr_dict_attr_t const * : fr_pair_delete_by_da(_list, (fr_dict_attr_t const *)_pair), \ + fr_dict_attr_t * : fr_pair_delete_by_da(_list, (fr_dict_attr_t const *)_pair), \ + VALUE_PAIR * : fr_pair_delete(_list, (VALUE_PAIR const *)_pair) \ + ) -/** Return or allocate a VALUE_PAIR in the request list +/** Delete a VALUE_PAIR in the request list * - * @param[in] _da #fr_dict_attr_t of the pair(s) to be deleted. + * @param[in] _pair #fr_dict_attr_t of the pair(s) to be deleted. * @return * - >0 the number of pairs deleted. * - 0 if no pairs were deleted. */ -#define pair_delete_request(_da) fr_pair_delete_by_da(&request->packet->vps, _da) +#define pair_delete_request(_pair) pair_delete(&request->packet->vps, _pair) + /** Delete a VALUE_PAIR in the reply list * - * @param[in] _da #fr_dict_attr_t of the pair(s) to be deleted. + * @param[in] _pair #fr_dict_attr_t of the pair(s) to be deleted. * @return * - >0 the number of pairs deleted. * - 0 if no pairs were deleted. */ -#define pair_delete_reply(_da) fr_pair_delete_by_da(&request->reply->vps, _da) +#define pair_delete_reply(_pair) pair_delete(&request->reply->vps, _pair) /** Delete a VALUE_PAIR in the control list * - * @param[in] _da #fr_dict_attr_t of the pair(s) to be deleted. + * @param[in] _pair #fr_dict_attr_t of the pair(s) to be deleted. * @return * - >0 the number of pairs deleted. * - 0 if no pairs were deleted. */ -#define pair_delete_control(_da) fr_pair_delete_by_da(&request->control, _da) +#define pair_delete_control(_pair) pair_delete(&request->control, _pair) /** Delete a VALUE_PAIR in the session_state list * - * @param[in] _da #fr_dict_attr_t of the pair(s) to be deleted. + * @param[in] _pair #fr_dict_attr_t of the pair(s) to be deleted. * @return * - >0 the number of pairs deleted. * - 0 if no pairs were deleted. */ -#define pair_delete_session_state(_da) fr_pair_delete_by_da(&request->state, _da) +#define pair_delete_session_state(_pair) pair_delete(&request->state, _pair) diff --git a/src/lib/tls/ocsp.c b/src/lib/tls/ocsp.c index 722f4ede25d..b39845eb68e 100644 --- a/src/lib/tls/ocsp.c +++ b/src/lib/tls/ocsp.c @@ -137,7 +137,7 @@ static int ocsp_staple_to_pair(VALUE_PAIR **out, REQUEST *request, OCSP_RESPONSE { VALUE_PAIR *vp; size_t len; - uint8_t *buff, *p; + uint8_t *p; if (!resp) { REDEBUG("No OCSP response available"); @@ -150,14 +150,14 @@ static int ocsp_staple_to_pair(VALUE_PAIR **out, REQUEST *request, OCSP_RESPONSE return -1; } - MEM(p = buff = talloc_array(request, uint8_t, len)); /* Alloc in context of request - steal later */ + MEM(pair_update_request(&vp, attr_tls_ocsp_response) >= 0); + MEM(fr_pair_value_mem_alloc(vp, &p, len, true)); len = i2d_OCSP_RESPONSE(resp, &p); if (len <= 0) { REDEBUG("Failed serialising OCSP response"); + pair_delete_request(vp); return -1; } - MEM(pair_update_request(&vp, attr_tls_ocsp_response) >= 0); - fr_pair_value_memsteal(vp, buff, true); RDEBUG2("Serializing OCSP response"); RINDENT(); diff --git a/src/lib/unlang/xlat_eval.c b/src/lib/unlang/xlat_eval.c index 88f5908ae2b..da544bcbaae 100644 --- a/src/lib/unlang/xlat_eval.c +++ b/src/lib/unlang/xlat_eval.c @@ -1745,7 +1745,8 @@ int xlat_eval_pair(REQUEST *request, VALUE_PAIR *vp) * then we just want to copy the new value in unmolested. */ if ((vp->op == T_OP_REG_EQ) || (vp->op == T_OP_REG_NE)) { - fr_pair_value_bstrsteal(vp, expanded); + fr_pair_value_bstrdup_buffer(vp, expanded, vp->vp_tainted); + talloc_free(expanded); return 0; } diff --git a/src/lib/util/pair.c b/src/lib/util/pair.c index 58451d0ff62..77d28b9e01e 100644 --- a/src/lib/util/pair.c +++ b/src/lib/util/pair.c @@ -757,6 +757,26 @@ int fr_pair_delete_by_da(VALUE_PAIR **list, fr_dict_attr_t const *da) return cnt; } +/** Remove VALUE_PAIR from a list + * + * @param[in] list of value pairs to remove VP from. + * @param[in] vp to remove + */ +void fr_pair_delete(VALUE_PAIR **list, VALUE_PAIR const *vp) +{ + fr_cursor_t cursor; + VALUE_PAIR *cvp; + + for (cvp = fr_cursor_init(&cursor, list); + cvp; + cvp = fr_cursor_next(&cursor)) { + if (cvp == vp) { + fr_cursor_free_item(&cursor); + return; + } + } +} + /** Order attributes by their da, and tag * * Useful where attributes need to be aggregated, but not necessarily @@ -1836,48 +1856,6 @@ int fr_pair_value_bstrn_append(VALUE_PAIR *vp, char const *src, size_t len, bool return ret; } -/** Append a talloced buffer to an existing "string" type value pair - * - * @param[in,out] vp to update. - * @param[in] src data to copy. - * @param[in] tainted Whether the value came from a trusted source. - * @return - * - 0 on success. - * - -1 on failure. - */ -int fr_pair_value_bstr_append_buffer(VALUE_PAIR *vp, char const *src, bool tainted) -{ - int ret; - - if (!fr_cond_assert(vp->da->type == FR_TYPE_STRING)) return -1; - - ret = fr_value_box_bstr_append_buffer(vp, &vp->data, src, tainted); - if (ret == 0) { - vp->type = VT_DATA; - VP_VERIFY(vp); - } - - return ret; -} - -/** Reparent an allocated char buffer to a VALUE_PAIR - * - * @param[in,out] vp to update - * @param[in] src buffer to steal. - */ -int fr_pair_value_bstrsteal(VALUE_PAIR *vp, char *src) -{ - if (!fr_cond_assert(vp->da->type == FR_TYPE_STRING)) return -1; - - fr_value_box_clear(&vp->data); /* Free any existing buffers */ - fr_value_box_bstrsteal(vp, &vp->data, vp->da, src, false); - vp->type = VT_DATA; - - VP_VERIFY(vp); - - return 0; -} - /** Pre-allocate a memory buffer for a "octets" type value pair * * @note Will clear existing values (including buffers). @@ -2080,27 +2058,6 @@ int fr_pair_value_mem_append_buffer(VALUE_PAIR *vp, uint8_t *src, bool tainted) return ret; } -/** Reparent an allocated octet buffer to a VALUE_PAIR - * - * @note Will clear existing values (including buffers). - * - * @param[in,out] vp to update - * @param[in] src buffer to steal. - * @param[in] tainted Whether the value came from a trusted source. - */ -int fr_pair_value_memsteal(VALUE_PAIR *vp, uint8_t const *src, bool tainted) -{ - if (!fr_cond_assert(vp->da->type == FR_TYPE_OCTETS)) return -1; - - fr_value_box_clear(&vp->data); /* Free any existing buffers */ - fr_value_box_memsteal(vp, &vp->data, vp->da, src, tainted); - vp->type = VT_DATA; - - VP_VERIFY(vp); - - return 0; -} - /** Print the value of an attribute to a string * * @param[out] out Where to write the string. diff --git a/src/lib/util/pair.h b/src/lib/util/pair.h index 69c6f031f24..7592f2d0026 100644 --- a/src/lib/util/pair.h +++ b/src/lib/util/pair.h @@ -277,6 +277,8 @@ int fr_pair_update_by_da(TALLOC_CTX *ctx, VALUE_PAIR **out, VALUE_PAIR **list, int fr_pair_delete_by_da(VALUE_PAIR **head, fr_dict_attr_t const *da); +void fr_pair_delete(VALUE_PAIR **list, VALUE_PAIR const *vp); + /* functions for FR_TYPE_GROUP */ fr_pair_list_t *fr_pair_group_get_sublist(VALUE_PAIR *head); @@ -371,8 +373,6 @@ int fr_pair_value_bstrdup_buffer_shallow(VALUE_PAIR *vp, char const *src, bool int fr_pair_value_bstrn_append(VALUE_PAIR *vp, char const *src, size_t len, bool tainted); int fr_pair_value_bstr_append_buffer(VALUE_PAIR *vp, char const *src, bool tainted); - -int fr_pair_value_bstrsteal(VALUE_PAIR *vp, char *src); /** @} */ /** @name Assign and manipulate octets strings @@ -394,8 +394,6 @@ int fr_pair_value_memdup_buffer_shallow(VALUE_PAIR *vp, uint8_t const *src, boo int fr_pair_value_mem_append(VALUE_PAIR *vp, uint8_t *src, size_t len, bool tainted); int fr_pair_value_mem_append_buffer(VALUE_PAIR *vp, uint8_t *src, bool tainted); - -int fr_pair_value_memsteal(VALUE_PAIR *vp, uint8_t const *src, bool tainted); /** @} */ /* Printing functions */ diff --git a/src/lib/util/value.c b/src/lib/util/value.c index 16d723be3c4..0723b3187c0 100644 --- a/src/lib/util/value.c +++ b/src/lib/util/value.c @@ -3846,47 +3846,6 @@ int fr_value_box_bstr_append_buffer(TALLOC_CTX *ctx, fr_value_box_t *dst, char c return fr_value_box_bstrn_append(ctx, dst, src, len - 1, tainted); } -/** Steal a nul terminated talloced buffer into a specified ctx, and assign to a #fr_value_box_t - * - * Steal a talloced nul terminated buffer, setting fields in the dst value box appropriately. - * - * The buffer must be \0 terminated, or an error will be returned. - * - * @param[in] ctx to allocate any new buffers in. - * @param[in] dst to assign new buffer to. - * @param[in] enumv Aliases for values. - * @param[in] src a talloced nul terminated buffer. - * @param[in] tainted Whether the value came from a trusted source. - * @return - * - 0 on success. - * - -1 on failure. - */ -int fr_value_box_bstrsteal(TALLOC_CTX *ctx, fr_value_box_t *dst, fr_dict_attr_t const *enumv, - char *src, bool tainted) -{ - size_t len; - char *str; - - len = talloc_array_length(src); - if ((len == 0) || (src[len - 1] != '\0')) { - fr_strerror_printf("Input buffer empty or not \\0 terminated"); - return -1; - } - - str = talloc_steal(ctx, src); - if (!str) { - fr_strerror_printf("Failed stealing string buffer"); - return -1; - } - talloc_set_type(str, char); - - fr_value_box_init(dst, FR_TYPE_STRING, enumv, tainted); - dst->vb_strvalue = str; - dst->vb_length = len - 1; - - return 0; -} - /** Pre-allocate an octets buffer for filling by the caller * * @note Buffer will not be zeroed, as it's assumed the caller will be filling it. @@ -4135,29 +4094,6 @@ int fr_value_box_mem_append_buffer(TALLOC_CTX *ctx, fr_value_box_t *dst, uint8_t return fr_value_box_mem_append(ctx, dst, src, talloc_array_length(src), tainted); } -/** Steal a talloced buffer into a specified ctx, and assign to a #fr_value_box_t - * - * Steal a talloced buffer, setting fields in the dst value box appropriately. - * - * @param[in] ctx to allocate any new buffers in. - * @param[in] dst to assign new buffer to. - * @param[in] enumv Aliases for values. - * @param[in] src a talloced nul terminated buffer. - * @param[in] tainted Whether the value came from a trusted source. - */ -void fr_value_box_memsteal(TALLOC_CTX *ctx, fr_value_box_t *dst, fr_dict_attr_t const *enumv, - uint8_t const *src, bool tainted) -{ - (void) talloc_get_type_abort_const(src, uint8_t); - - src = talloc_steal(ctx, src); /* steal can never fail according to talloc docs */ - talloc_set_type(src, uint8_t); - - fr_value_box_init(dst, FR_TYPE_OCTETS, enumv, tainted); - dst->vb_octets = src; - dst->datum.length = talloc_array_length(src); -} - /** Increment a boxed value * * Implements safe integer overflow. diff --git a/src/lib/util/value.h b/src/lib/util/value.h index 48941ebe00f..a525e884522 100644 --- a/src/lib/util/value.h +++ b/src/lib/util/value.h @@ -642,9 +642,6 @@ int fr_value_box_bstrdup_buffer_shallow(TALLOC_CTX *ctx, fr_value_box_t *dst, f int fr_value_box_bstrn_append(TALLOC_CTX *ctx, fr_value_box_t *dst, char const *src, size_t len, bool tainted); int fr_value_box_bstr_append_buffer(TALLOC_CTX *ctx, fr_value_box_t *dst, char const *src, bool tainted); - -int fr_value_box_bstrsteal(TALLOC_CTX *ctx, fr_value_box_t *dst, fr_dict_attr_t const *enumv, - char *src, bool tainted); /** @} */ /** @name Assign and manipulate octets strings @@ -672,10 +669,6 @@ int fr_value_box_mem_append(TALLOC_CTX *ctx, fr_value_box_t *dst, uint8_t const *src, size_t len, bool tainted); int fr_value_box_mem_append_buffer(TALLOC_CTX *ctx, fr_value_box_t *dst, uint8_t const *src, bool tainted); - - -void fr_value_box_memsteal(TALLOC_CTX *ctx, fr_value_box_t *dst, fr_dict_attr_t const *enumv, - uint8_t const *src, bool tainted); /** @} */ void fr_value_box_increment(fr_value_box_t *vb); diff --git a/src/modules/rlm_sql/rlm_sql.c b/src/modules/rlm_sql/rlm_sql.c index 6f31248d504..2b13e35f6bc 100644 --- a/src/modules/rlm_sql/rlm_sql.c +++ b/src/modules/rlm_sql/rlm_sql.c @@ -652,16 +652,18 @@ int sql_set_user(rlm_sql_t const *inst, REQUEST *request, char const *username) return 0; } - len = xlat_aeval(request, &expanded, request, sqluser, NULL, NULL); + MEM(pair_update_request(&vp, inst->sql_user) >= 0); + len = xlat_aeval(vp, &expanded, request, sqluser, NULL, NULL); if (len < 0) { + pair_delete_request(vp); return -1; } /* * Replace any existing SQL-User-Name with outs */ - MEM(pair_update_request(&vp, inst->sql_user) >= 0); - fr_pair_value_bstrsteal(vp, expanded); + fr_pair_value_bstrdup_buffer_shallow(vp, expanded, true); + MEM(fr_pair_value_bstr_realloc(vp, NULL, len) == 0); RDEBUG2("SQL-User-Name set to '%pV'", &vp->data); return 0; diff --git a/src/modules/rlm_sqlippool/rlm_sqlippool.c b/src/modules/rlm_sqlippool/rlm_sqlippool.c index a065737eb6f..2ca5a34450f 100644 --- a/src/modules/rlm_sqlippool/rlm_sqlippool.c +++ b/src/modules/rlm_sqlippool/rlm_sqlippool.c @@ -467,10 +467,12 @@ static int do_logging(UNUSED rlm_sqlippool_t *inst, REQUEST *request, char const if (!str || !*str) return rcode; - if (xlat_aeval(request, &expanded, request, str, NULL, NULL) < 0) return rcode; - MEM(pair_add_request(&vp, attr_module_success_message) == 0); - fr_pair_value_bstrsteal(vp, expanded); + if (xlat_aeval(request, &expanded, request, str, NULL, NULL) < 0) { + pair_delete_request(vp); + return rcode; + } + fr_pair_value_bstrdup_buffer_shallow(vp, expanded, true); return rcode; }