From: Arran Cudbard-Bell Date: Fri, 15 Jan 2021 20:46:34 +0000 (+0000) Subject: Swap out the list heads for pairs X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=ee445822d3ef8a2d5d24ce058904ba4d87495c5b;p=thirdparty%2Ffreeradius-server.git Swap out the list heads for pairs Rework the support for child session state to make it simpler Fix a ton of pair parenting issues --- diff --git a/share/dictionary/freeradius/dictionary.freeradius.internal b/share/dictionary/freeradius/dictionary.freeradius.internal index 9e9cd1f2ed3..f9e8b2d12dc 100644 --- a/share/dictionary/freeradius/dictionary.freeradius.internal +++ b/share/dictionary/freeradius/dictionary.freeradius.internal @@ -44,10 +44,12 @@ ATTRIBUTE Relax-Filter 501 bool # Range: 1 - 10 # Attribute lists # -ATTRIBUTE Request 1 group -ATTRIBUTE Reply 2 group -ATTRIBUTE Session-State 3 group +ATTRIBUTE Root 1 group +ATTRIBUTE Request 2 group +ATTRIBUTE Reply 3 group ATTRIBUTE Control 4 group +ATTRIBUTE Session-State 5 group + # # Range: 11 - 19 diff --git a/src/bin/radclient.c b/src/bin/radclient.c index d8f968c6404..2ad75467bc8 100644 --- a/src/bin/radclient.c +++ b/src/bin/radclient.c @@ -46,7 +46,9 @@ typedef struct request_s request_t; /* to shut up warnings about mschap.h */ #include "radclient.h" -#define pair_update_request(_attr, _da) fr_pair_update_by_da(request->packet, _attr, &request->request_pairs, _da) +#define pair_update_request(_attr, _da) fr_pair_update_by_da(request, _attr, &request->request_pairs, _da) +#define request_pairs request_list +#define reply_pairs reply_list static int retries = 3; static fr_time_delta_t timeout = ((fr_time_delta_t) 5) * NSEC; @@ -391,7 +393,7 @@ static int radclient_init(TALLOC_CTX *ctx, rc_file_pair_t *files) /* * Read the request VP's. */ - if (fr_pair_list_afrom_file(request->packet, dict_radius, + if (fr_pair_list_afrom_file(request, dict_radius, &request->request_pairs, packets, &packets_done) < 0) { char const *input; diff --git a/src/bin/unit_test_module.c b/src/bin/unit_test_module.c index e13586fe7fa..2b6c9e5da00 100644 --- a/src/bin/unit_test_module.c +++ b/src/bin/unit_test_module.c @@ -143,7 +143,7 @@ static RADCLIENT *client_alloc(TALLOC_CTX *ctx, char const *ip, char const *name static request_t *request_from_file(TALLOC_CTX *ctx, FILE *fp, fr_event_list_t *el, RADCLIENT *client) { fr_pair_t *vp; - request_t *request; + request_t *request; fr_cursor_t cursor; static int number = 0; @@ -151,8 +151,7 @@ static request_t *request_from_file(TALLOC_CTX *ctx, FILE *fp, fr_event_list_t * /* * Create and initialize the new request. */ - request = request_local_alloc(ctx); - + request = request_local_alloc(ctx, NULL); /* * FIXME - Should be less RADIUS centric, but everything * else assumes RADIUS at the moment so we can fix this later. @@ -193,7 +192,7 @@ static request_t *request_from_file(TALLOC_CTX *ctx, FILE *fp, fr_event_list_t * /* * Read packet from fp */ - if (fr_pair_list_afrom_file(request->packet, dict_protocol, &request->request_pairs, fp, &filedone) < 0) { + if (fr_pair_list_afrom_file(request->request_ctx, dict_protocol, &request->request_pairs, fp, &filedone) < 0) { fr_perror("%s", main_config->name); talloc_free(request); return NULL; @@ -343,7 +342,7 @@ static bool do_xlats(char const *filename, FILE *fp) /* * Create and initialize the new request. */ - request = request_alloc(NULL); + request = request_alloc(NULL, NULL); request->log.dst = talloc_zero(request, log_dst_t); request->log.dst->func = vlog_request; @@ -543,14 +542,14 @@ static request_t *request_clone(request_t *old) { request_t *request; - request = request_alloc(NULL); + request = request_alloc(NULL, NULL); if (!request) return NULL; if (!request->packet) request->packet = fr_radius_packet_alloc(request, false); if (!request->reply) request->reply = fr_radius_packet_alloc(request, false); memcpy(request->packet, old->packet, sizeof(*request->packet)); - (void) fr_pair_list_copy(request->packet, &request->request_pairs, &old->request_pairs); + (void) fr_pair_list_copy(request->request_ctx, &request->request_pairs, &old->request_pairs); request->packet->timestamp = fr_time(); request->number = old->number++; @@ -938,7 +937,7 @@ int main(int argc, char *argv[]) } } - if (fr_pair_list_afrom_file(request, dict_protocol, &filter_vps, fp, &filedone) < 0) { + if (fr_pair_list_afrom_file(request->request_ctx, dict_protocol, &filter_vps, fp, &filedone) < 0) { fr_perror("Failed reading attributes from %s", filter_file); EXIT_WITH_FAILURE; } diff --git a/src/lib/curl/base.c b/src/lib/curl/base.c index f0487947e79..92bb33783d7 100644 --- a/src/lib/curl/base.c +++ b/src/lib/curl/base.c @@ -208,7 +208,7 @@ int fr_curl_response_certinfo(request_t *request, fr_curl_io_request_t *randle) RDEBUG3("If this value is required, define attribute \"%s\"", buffer); continue; } - MEM(vp = fr_pair_afrom_da(request->packet, da)); + MEM(vp = fr_pair_afrom_da(request->request_ctx, da)); fr_pair_value_from_str(vp, q + 1, -1, '\0', true); fr_cursor_append(&cursor, vp); diff --git a/src/lib/eap/chbind.c b/src/lib/eap/chbind.c index 8e3084eafcd..b6942fbe626 100644 --- a/src/lib/eap/chbind.c +++ b/src/lib/eap/chbind.c @@ -184,13 +184,13 @@ FR_CODE chbind_process(request_t *request, CHBIND_REQ *chbind) (chbind->response == NULL)); /* Set-up the fake request */ - fake = request_alloc_fake(request, NULL); - MEM(fr_pair_add_by_da(fake->packet, &vp, &fake->request_pairs, attr_freeradius_proxied_to) >= 0); + fake = request_alloc(request, &(request_init_args_t){ .parent = request }); + MEM(fr_pair_add_by_da(fake->request_ctx, &vp, &fake->request_pairs, attr_freeradius_proxied_to) >= 0); fr_pair_value_from_str(vp, "127.0.0.1", sizeof("127.0.0.1"), '\0', false); /* Add the username to the fake request */ if (chbind->username) { - vp = fr_pair_copy(fake->packet, chbind->username); + vp = fr_pair_copy(fake->request_ctx, chbind->username); fr_pair_add(&fake->request_pairs, vp); } @@ -209,7 +209,7 @@ FR_CODE chbind_process(request_t *request, CHBIND_REQ *chbind) while (data_len > 0) { ssize_t attr_len; - attr_len = fr_radius_decode_pair(fake->packet, &cursor, dict_radius, + attr_len = fr_radius_decode_pair(fake->request_ctx, &cursor, dict_radius, attr_data, data_len, NULL); if (attr_len <= 0) { /* @@ -303,13 +303,13 @@ chbind_packet_t *eap_chbind_vp2packet(TALLOC_CTX *ctx, fr_pair_list_t *vps) return packet; } -fr_pair_t *eap_chbind_packet2vp(fr_radius_packet_t *packet, chbind_packet_t *chbind) +fr_pair_t *eap_chbind_packet2vp(TALLOC_CTX *ctx, chbind_packet_t *chbind) { fr_pair_t *vp; if (!chbind) return NULL; /* don't produce garbage */ - MEM(vp = fr_pair_afrom_da(packet, attr_eap_channel_binding_message)); + MEM(vp = fr_pair_afrom_da(ctx, attr_eap_channel_binding_message)); fr_pair_value_memdup(vp, (uint8_t *) chbind, talloc_array_length((uint8_t *)chbind), false); return vp; diff --git a/src/lib/eap/chbind.h b/src/lib/eap/chbind.h index b95782ffe2a..e1a958c1957 100644 --- a/src/lib/eap/chbind.h +++ b/src/lib/eap/chbind.h @@ -59,5 +59,5 @@ typedef struct { /* Channel binding function prototypes */ FR_CODE chbind_process(request_t *request, CHBIND_REQ *chbind_req); -fr_pair_t *eap_chbind_packet2vp(fr_radius_packet_t *packet, chbind_packet_t *chbind); +fr_pair_t *eap_chbind_packet2vp(TALLOC_CTX *ctx, chbind_packet_t *chbind); chbind_packet_t *eap_chbind_vp2packet(TALLOC_CTX *ctx, fr_pair_list_t *vps); diff --git a/src/lib/eap_aka_sim/decode.c b/src/lib/eap_aka_sim/decode.c index d80ad921816..1e7a570b22c 100644 --- a/src/lib/eap_aka_sim/decode.c +++ b/src/lib/eap_aka_sim/decode.c @@ -996,7 +996,7 @@ int fr_aka_sim_decode(request_t *request, fr_cursor_t *decoded, fr_dict_t const * in the SIM/AKA/AKA' dict. */ while (p < end) { - ret = fr_aka_sim_decode_pair(request->packet, decoded, dict, p, end - p, decoder_ctx); + ret = fr_aka_sim_decode_pair(request->request_ctx, decoded, dict, p, end - p, decoder_ctx); if (ret <= 0) { RPEDEBUG("Failed decoding AT"); error: @@ -1015,7 +1015,7 @@ int fr_aka_sim_decode(request_t *request, fr_cursor_t *decoded, fr_dict_t const { fr_pair_t *vp; - vp = fr_pair_afrom_child_num(request->packet, fr_dict_root(dict), FR_SUBTYPE); + vp = fr_pair_afrom_child_num(request->request_ctx, fr_dict_root(dict), FR_SUBTYPE); if (!vp) { fr_strerror_const("Failed allocating subtype attribute"); goto error; diff --git a/src/lib/eap_aka_sim/state_machine.c b/src/lib/eap_aka_sim/state_machine.c index f759cf482a6..4db33f1a391 100644 --- a/src/lib/eap_aka_sim/state_machine.c +++ b/src/lib/eap_aka_sim/state_machine.c @@ -373,8 +373,8 @@ static int identity_to_permanent_identity(request_t *request, fr_pair_t *in, eap * fancy, just copy Identity -> Permanent-Identity. */ if (!strip_hint) { - MEM(fr_pair_update_by_da(request->state_ctx, &vp, - &request->state_pairs, attr_eap_aka_sim_permanent_identity) >= 0); + MEM(fr_pair_update_by_da(request->session_state_ctx, &vp, + &request->session_state_pairs, attr_eap_aka_sim_permanent_identity) >= 0); fr_pair_value_bstrndup(vp, in->vp_strvalue, in->vp_length, true); return 0; } @@ -409,8 +409,8 @@ static int identity_to_permanent_identity(request_t *request, fr_pair_t *in, eap */ if ((fr_aka_sim_id_type(&our_type, &our_method, in->vp_strvalue, in->vp_length) < 0) || (our_type != AKA_SIM_ID_TYPE_PERMANENT)) { - MEM(fr_pair_update_by_da(request->state_ctx, &vp, - &request->state_pairs, attr_eap_aka_sim_permanent_identity) >= 0); + MEM(fr_pair_update_by_da(request->session_state_ctx, &vp, + &request->session_state_pairs, attr_eap_aka_sim_permanent_identity) >= 0); fr_pair_value_bstrndup(vp, in->vp_strvalue, in->vp_length, true); RDEBUG2("%s has incorrect hint byte, expected '%c', got '%c'. " @@ -429,8 +429,8 @@ static int identity_to_permanent_identity(request_t *request, fr_pair_t *in, eap * Strip off the hint byte, and then add the permanent * identity to the output list. */ - MEM(fr_pair_update_by_da(request->state_ctx, &vp, - &request->state_pairs, attr_eap_aka_sim_permanent_identity) >= 0); + MEM(fr_pair_update_by_da(request->session_state_ctx, &vp, + &request->session_state_pairs, attr_eap_aka_sim_permanent_identity) >= 0); fr_pair_value_bstrndup(vp, in->vp_strvalue + 1, in->vp_length - 1, true); RDEBUG2("Stripping 'hint' byte from %s", attr_eap_aka_sim_permanent_identity->name); @@ -590,7 +590,7 @@ static unlang_action_t pseudonym_store_resume(rlm_rcode_t *p_result, module_ctx_ * state increment by 1, otherwise, add the * attribute and set to zero. */ - vp = fr_pair_find_by_da(&request->state_pairs, attr_eap_aka_sim_counter); + vp = fr_pair_find_by_da(&request->session_state_pairs, attr_eap_aka_sim_counter); if (vp) { vp->vp_uint16++; /* @@ -1160,7 +1160,7 @@ static unlang_action_t common_reauthentication_request_compose(rlm_rcode_t *p_re */ case FR_EAP_METHOD_SIM: case FR_EAP_METHOD_AKA: - if (fr_aka_sim_vector_gsm_umts_kdf_0_reauth_from_attrs(request, &request->state_pairs, + if (fr_aka_sim_vector_gsm_umts_kdf_0_reauth_from_attrs(request, &request->session_state_pairs, &eap_aka_sim_session->keys) != 0) { request_new_id: switch (eap_aka_sim_session->last_id_req) { @@ -1192,7 +1192,7 @@ static unlang_action_t common_reauthentication_request_compose(rlm_rcode_t *p_re case FR_EAP_METHOD_AKA_PRIME: switch (eap_aka_sim_session->kdf) { case FR_KDF_VALUE_PRIME_WITH_CK_PRIME_IK_PRIME: - if (fr_aka_sim_vector_umts_kdf_1_reauth_from_attrs(request, &request->state_pairs, + if (fr_aka_sim_vector_umts_kdf_1_reauth_from_attrs(request, &request->session_state_pairs, &eap_aka_sim_session->keys) != 0) { goto request_new_id; } diff --git a/src/lib/io/worker.c b/src/lib/io/worker.c index 7c150f3b52b..b68c137536f 100644 --- a/src/lib/io/worker.c +++ b/src/lib/io/worker.c @@ -706,7 +706,7 @@ static void worker_request_bootstrap(fr_worker_t *worker, fr_channel_data_t *cd, if (fr_heap_num_elements(worker->time_order) >= (uint32_t) worker->config.max_requests) goto nak; - ctx = request = request_alloc(NULL); + ctx = request = request_alloc(NULL, NULL); if (!request) goto nak; worker_request_init(worker, request, now); diff --git a/src/lib/server/base.c b/src/lib/server/base.c index 03b3fcca83d..63ed54ad847 100644 --- a/src/lib/server/base.c +++ b/src/lib/server/base.c @@ -38,6 +38,12 @@ RCSID("$Id$") */ int server_init(CONF_SECTION *cs) { + /* + * Load dictionary attributes used + * for requests. + */ + if (request_global_init() < 0) return -1; + /* * Initialise the trigger rate limiting tree */ @@ -92,10 +98,6 @@ int server_init(CONF_SECTION *cs) */ void server_free(void) { - /* - * Free password dictionaries - */ - password_free(); /* * Free xlat instance data, and call any detach methods @@ -117,6 +119,11 @@ void server_free(void) */ xlat_free(); + /* + * Free password dictionaries + */ + password_free(); + /* * The only maps remaining are the ones registered by the server core. */ @@ -133,4 +140,8 @@ void server_free(void) */ trigger_exec_free(); + /* + * Free the internal dictionaries the request uses + */ + request_global_free(); } diff --git a/src/lib/server/cond_eval.c b/src/lib/server/cond_eval.c index 81a36bf799b..555aa4e6a99 100644 --- a/src/lib/server/cond_eval.c +++ b/src/lib/server/cond_eval.c @@ -565,7 +565,7 @@ static int cond_compare_virtual(request_t *request, map_t const *map) /* * Create the virtual check item. */ - MEM(virt = fr_pair_afrom_da(request, tmpl_da(map->lhs))); + MEM(virt = fr_pair_afrom_da(request->request_ctx, tmpl_da(map->lhs))); virt->op = map->op; fr_value_box_copy(virt, &virt->data, rhs); @@ -711,7 +711,7 @@ int cond_eval_map(request_t *request, UNUSED int depth, fr_cond_t const *c) return -1; } - MEM(vp = fr_pair_afrom_da(request, tmpl_da(map->lhs))); + MEM(vp = fr_pair_afrom_da(request->request_ctx, tmpl_da(map->lhs))); vp->op = c->data.map->op; fr_value_box_copy(vp, &vp->data, rhs); diff --git a/src/lib/server/exfile.c b/src/lib/server/exfile.c index 919c830cc6c..6493bbdffb1 100644 --- a/src/lib/server/exfile.c +++ b/src/lib/server/exfile.c @@ -85,7 +85,7 @@ static inline void exfile_trigger_exec(exfile_t *ef, request_t *request, exfile_ return; } - fr_pair_list_copy(request, &args, &ef->trigger_args); + fr_pair_list_copy(request ? request->request_ctx : NULL, &args, &ef->trigger_args); fr_cursor_init(&cursor, &args); MEM(vp = fr_pair_afrom_da(NULL, da)); diff --git a/src/lib/server/pair.h b/src/lib/server/pair.h index 2ae47fdb84a..d2127b47079 100644 --- a/src/lib/server/pair.h +++ b/src/lib/server/pair.h @@ -33,7 +33,7 @@ RCSIDH(server_pair_h, "$Id$") * - 0 on success. * - -1 on failure. */ -#define pair_add_request(_attr, _da) fr_pair_add_by_da(request->packet, _attr, &request->request_pairs, _da) +#define pair_add_request(_attr, _da) fr_pair_add_by_da(request->request_ctx, _attr, &request->request_pairs, _da) /** Allocate a fr_pair_t in the reply list * @@ -43,7 +43,7 @@ RCSIDH(server_pair_h, "$Id$") * - 0 on success. * - -1 on failure. */ -#define pair_add_reply(_attr, _da) fr_pair_add_by_da(request->reply, _attr, &request->reply_pairs, _da) +#define pair_add_reply(_attr, _da) fr_pair_add_by_da(request->reply_ctx, _attr, &request->reply_pairs, _da) /** Allocate a fr_pair_t in the control list * @@ -53,7 +53,7 @@ RCSIDH(server_pair_h, "$Id$") * - 0 on success. * - -1 on failure. */ -#define pair_add_control(_attr, _da) fr_pair_add_by_da(request, _attr, &request->control_pairs, _da) +#define pair_add_control(_attr, _da) fr_pair_add_by_da(request->control_ctx, _attr, &request->control_pairs, _da) /** Allocate a fr_pair_t in the session-state list * @@ -63,7 +63,7 @@ RCSIDH(server_pair_h, "$Id$") * - 0 on success. * - -1 on failure. */ -#define pair_add_session_state(_attr, _da) fr_pair_add_by_da(request->state_ctx, _attr, &request->state_pairs, _da) +#define pair_add_session_state(_attr, _da) fr_pair_add_by_da(request->session_state_ctx, _attr, &request->session_state_pairs, _da) /** Return or allocate a fr_pair_t in the request list * @@ -74,7 +74,7 @@ RCSIDH(server_pair_h, "$Id$") * - 0 if we allocated a new attribute. * - -1 on failure. */ -#define pair_update_request(_attr, _da) fr_pair_update_by_da(request->packet, _attr, &request->request_pairs, _da) +#define pair_update_request(_attr, _da) fr_pair_update_by_da(request->request_ctx, _attr, &request->request_pairs, _da) /** Return or allocate a fr_pair_t in the reply list * @@ -85,7 +85,7 @@ RCSIDH(server_pair_h, "$Id$") * - 0 if we allocated a new attribute. * - -1 on failure. */ -#define pair_update_reply(_attr, _da) fr_pair_update_by_da(request->reply, _attr, &request->reply_pairs, _da) +#define pair_update_reply(_attr, _da) fr_pair_update_by_da(request->reply_ctx, _attr, &request->reply_pairs, _da) /** Return or allocate a fr_pair_t in the control list * @@ -96,7 +96,7 @@ RCSIDH(server_pair_h, "$Id$") * - 0 if we allocated a new attribute. * - -1 on failure. */ -#define pair_update_control(_attr, _da) fr_pair_update_by_da(request, _attr, &request->control_pairs, _da) +#define pair_update_control(_attr, _da) fr_pair_update_by_da(request->control_ctx, _attr, &request->control_pairs, _da) /** Return or allocate a fr_pair_t in the session_state list * @@ -107,7 +107,7 @@ RCSIDH(server_pair_h, "$Id$") * - 0 if we allocated a new attribute. * - -1 on failure. */ -#define pair_update_session_state(_attr, _da) fr_pair_update_by_da(request->state_ctx, _attr, &request->state_pairs, _da) +#define pair_update_session_state(_attr, _da) fr_pair_update_by_da(request->session_state_ctx, _attr, &request->session_state_pairs, _da) /** Delete a fr_pair_t in a list * @@ -156,5 +156,5 @@ RCSIDH(server_pair_h, "$Id$") * - >0 the number of pairs deleted. * - 0 if no pairs were deleted. */ -#define pair_delete_session_state(_pair) pair_delete(&request->state_pairs, _pair) +#define pair_delete_session_state(_pair) pair_delete(&request->session_state_pairs, _pair) diff --git a/src/lib/server/pair_server_tests.c b/src/lib/server/pair_server_tests.c index 4e841704ece..42a312d8cc2 100644 --- a/src/lib/server/pair_server_tests.c +++ b/src/lib/server/pair_server_tests.c @@ -40,6 +40,7 @@ static void pair_tests_init(void); #endif #include +#include #include #include @@ -172,6 +173,11 @@ static void pair_tests_init(void) if (!fr_dict_global_ctx_init(autofree, dict_dir)) goto error; + /* + * Initialise attributes that go in requests + */ + if (request_global_init() < 0) goto error; + /* * Set the root name of the dictionary */ @@ -182,6 +188,8 @@ static void pair_tests_init(void) /* Initialize the "sample_pairs" list */ if (load_attr_pairs(&sample_pairs) < 0) goto error; + + } static request_t *request_fake_alloc(void) @@ -191,7 +199,7 @@ static request_t *request_fake_alloc(void) /* * Create and initialize the new request. */ - request = request_local_alloc(autofree); + request = request_local_alloc(autofree, NULL); request->packet = fr_radius_packet_alloc(request, false); TEST_CHECK(request->packet != NULL); @@ -207,15 +215,13 @@ static request_t *request_fake_alloc(void) */ static void test_pair_add_request(void) { - fr_pair_t *local_pairs; + fr_pair_t *local_pair; fr_cursor_t cursor; fr_pair_t *vp; request_t *request = request_fake_alloc(); - fr_pair_list_init(&local_pairs); - TEST_CASE("Add 'Test-Integer' in 'request_pairs' using pair_add_request()"); - TEST_CHECK(pair_add_request(&local_pairs, attr_test_integer) == 0); + TEST_CHECK(pair_add_request(&local_pair, attr_test_integer) == 0); TEST_CASE("Validating VP_VERIFY()"); TEST_CHECK((vp = fr_cursor_init(&cursor, &request->request_pairs)) != NULL); @@ -227,20 +233,18 @@ static void test_pair_add_request(void) TEST_CHECK(vp->vp_uint32 == 12345); TEST_MSG("Expected %s == 12345", attr_test_integer->name); - fr_pair_list_free(&local_pairs); + TEST_CHECK_RET(talloc_free(request), 0); } static void test_pair_add_reply(void) { - fr_pair_t *local_pairs; + fr_pair_t *local_vp; fr_cursor_t cursor; fr_pair_t *vp; request_t *request = request_fake_alloc(); - fr_pair_list_init(&local_pairs); - TEST_CASE("Add 'Test-Integer' in 'reply_pairs' using pair_add_reply()"); - TEST_CHECK(pair_add_reply(&local_pairs, attr_test_integer) == 0); + TEST_CHECK(pair_add_reply(&local_vp, attr_test_integer) == 0); TEST_CASE("Validating VP_VERIFY()"); TEST_CHECK((vp = fr_cursor_init(&cursor, &request->reply_pairs)) != NULL); @@ -252,20 +256,18 @@ static void test_pair_add_reply(void) TEST_CHECK(vp->vp_uint32 == 12345); TEST_MSG("Expected %s == 12345", attr_test_integer->name); - fr_pair_list_free(&local_pairs); + TEST_CHECK_RET(talloc_free(request), 0); } static void test_pair_add_control(void) { - fr_pair_t *local_pairs; + fr_pair_t *local_vp; fr_cursor_t cursor; fr_pair_t *vp; request_t *request = request_fake_alloc(); - fr_pair_list_init(&local_pairs); - TEST_CASE("Add 'Test-Integer' in 'control_pairs' using pair_add_control()"); - TEST_CHECK(pair_add_control(&local_pairs, attr_test_integer) == 0); + TEST_CHECK(pair_add_control(&local_vp, attr_test_integer) == 0); TEST_CASE("Validating VP_VERIFY()"); TEST_CHECK((vp = fr_cursor_init(&cursor, &request->control_pairs)) != NULL); @@ -277,23 +279,21 @@ static void test_pair_add_control(void) TEST_CHECK(vp->vp_uint32 == 12345); TEST_MSG("Expected %s == 12345", attr_test_integer->name); - fr_pair_list_free(&local_pairs); + TEST_CHECK_RET(talloc_free(request), 0); } static void test_pair_add_session_state(void) { - fr_pair_t *local_pairs; + fr_pair_t *local_vp; fr_cursor_t cursor; fr_pair_t *vp; request_t *request = request_fake_alloc(); - fr_pair_list_init(&local_pairs); - TEST_CASE("Add 'Test-Integer' in 'control_pairs' using pair_add_session_state()"); - TEST_CHECK(pair_add_session_state(&local_pairs, attr_test_integer) == 0); + TEST_CHECK(pair_add_session_state(&local_vp, attr_test_integer) == 0); TEST_CASE("Validating VP_VERIFY()"); - TEST_CHECK((vp = fr_cursor_init(&cursor, &request->state_pairs)) != NULL); + TEST_CHECK((vp = fr_cursor_init(&cursor, &request->session_state_pairs)) != NULL); VP_VERIFY(vp); TEST_MSG("Set vp = 12345"); @@ -302,7 +302,7 @@ static void test_pair_add_session_state(void) TEST_CHECK(vp->vp_uint32 == 12345); TEST_MSG("Expected %s == 12345", attr_test_integer->name); - fr_pair_list_free(&local_pairs); + TEST_CHECK_RET(talloc_free(request), 0); } static void test_pair_update_request(void) @@ -330,6 +330,8 @@ static void test_pair_update_request(void) * Such 'vp != NULL' just to mute clang "warning: Dereference of null pointer" */ TEST_CHECK(vp && vp->vp_uint32 == 112233); + + TEST_CHECK_RET(talloc_free(request), 0); } static void test_pair_update_reply(void) @@ -353,6 +355,8 @@ static void test_pair_update_reply(void) VP_VERIFY(vp); TEST_CHECK(vp && vp->vp_uint32 == 3333); + + TEST_CHECK_RET(talloc_free(request), 0); } static void test_pair_update_control(void) @@ -376,6 +380,8 @@ static void test_pair_update_control(void) VP_VERIFY(vp); TEST_CHECK(vp && vp->vp_uint32 == 44444); + + TEST_CHECK_RET(talloc_free(request), 0); } static void test_pair_update_session_state(void) @@ -393,12 +399,14 @@ static void test_pair_update_session_state(void) vp->vp_uint32 = 7890; TEST_CASE("Expected attr_test_integer (vp->vp_uint32 == 7890)"); - TEST_CHECK((vp = fr_pair_find_by_da(&request->state_pairs, attr_test_integer)) != NULL); + TEST_CHECK((vp = fr_pair_find_by_da(&request->session_state_pairs, attr_test_integer)) != NULL); TEST_CASE("Validating VP_VERIFY()"); VP_VERIFY(vp); TEST_CHECK(vp && vp->vp_uint32 == 7890); + + TEST_CHECK_RET(talloc_free(request), 0); } static void test_pair_delete_request(void) @@ -413,6 +421,8 @@ static void test_pair_delete_request(void) TEST_CASE("The 'Test-Integer' shouldn't exist in 'request->request_pairs'"); TEST_CHECK(fr_pair_find_by_da(&request->request_pairs, attr_test_integer) == NULL); + + TEST_CHECK_RET(talloc_free(request), 0); } static void test_pair_delete_reply(void) @@ -427,6 +437,8 @@ static void test_pair_delete_reply(void) TEST_CASE("The 'Test-Integer' shouldn't exist in 'request->reply_pairs'"); TEST_CHECK(fr_pair_find_by_da(&request->reply_pairs, attr_test_integer) == NULL); + + TEST_CHECK_RET(talloc_free(request), 0); } static void test_pair_delete_control(void) @@ -441,6 +453,8 @@ static void test_pair_delete_control(void) TEST_CASE("The 'Test-Integer' shouldn't exist in 'request->control_pairs'"); TEST_CHECK(fr_pair_find_by_da(&request->control_pairs, attr_test_integer) == NULL); + + TEST_CHECK_RET(talloc_free(request), 0); } static void test_pair_delete_session_state(void) @@ -448,13 +462,15 @@ static void test_pair_delete_session_state(void) request_t *request = request_fake_alloc(); TEST_CASE("Copy 'sample_pairs' into 'request->state'"); - TEST_CHECK(fr_pair_list_copy(autofree, &request->state_pairs, &sample_pairs) > 0); + TEST_CHECK(fr_pair_list_copy(autofree, &request->session_state_pairs, &sample_pairs) > 0); TEST_CASE("Delete 'Test-Integer' in 'request->state' using pair_delete_session_state()"); TEST_CHECK(pair_delete_session_state(attr_test_integer) > 0); TEST_CASE("The 'Test-Integer' shouldn't exist in 'request->state'"); - TEST_CHECK(fr_pair_find_by_da(&request->state_pairs, attr_test_integer) == NULL); + TEST_CHECK(fr_pair_find_by_da(&request->session_state_pairs, attr_test_integer) == NULL); + + TEST_CHECK_RET(talloc_free(request), 0); } TEST_LIST = { diff --git a/src/lib/server/paircmp.c b/src/lib/server/paircmp.c index 789402ba90a..161aa355562 100644 --- a/src/lib/server/paircmp.c +++ b/src/lib/server/paircmp.c @@ -135,7 +135,7 @@ static int generic_cmp(UNUSED void *instance, if (xlat_eval(value, sizeof(value), request, name, NULL, NULL) < 0) return 0; - MEM(vp = fr_pair_afrom_da(request, check_item->da)); + MEM(vp = fr_pair_afrom_da(request->request_ctx, check_item->da)); vp->op = check_item->op; fr_pair_value_from_str(vp, value, -1, '"', false); diff --git a/src/lib/server/password.c b/src/lib/server/password.c index 5612bc633b4..f34890620cd 100644 --- a/src/lib/server/password.c +++ b/src/lib/server/password.c @@ -742,7 +742,7 @@ do_header: } bad_header: - MEM(new = fr_pair_afrom_da(request, def)); + MEM(new = fr_pair_afrom_da(ctx, def)); fr_pair_value_bstrndup(new, p, end - p, true); return new; diff --git a/src/lib/server/request.c b/src/lib/server/request.c index 604e10893ec..5fff0289c5a 100644 --- a/src/lib/server/request.c +++ b/src/lib/server/request.c @@ -28,6 +28,32 @@ RCSID("$Id$") #include #include +static request_init_args_t default_args; + +static fr_dict_t const *dict_freeradius; + +extern fr_dict_autoload_t request_dict[]; +fr_dict_autoload_t request_dict[] = { + { .out = &dict_freeradius, .proto = "freeradius" }, + { NULL } +}; + +fr_dict_attr_t const *request_attr_root; +fr_dict_attr_t const *request_attr_request; +fr_dict_attr_t const *request_attr_reply; +fr_dict_attr_t const *request_attr_control; +fr_dict_attr_t const *request_attr_state; + +extern fr_dict_attr_autoload_t request_dict_attr[]; +fr_dict_attr_autoload_t request_dict_attr[] = { + { .out = &request_attr_root, .name = "root", .type = FR_TYPE_GROUP, .dict = &dict_freeradius }, + { .out = &request_attr_request, .name = "request", .type = FR_TYPE_GROUP, .dict = &dict_freeradius }, + { .out = &request_attr_reply, .name = "reply", .type = FR_TYPE_GROUP, .dict = &dict_freeradius }, + { .out = &request_attr_control, .name = "control", .type = FR_TYPE_GROUP, .dict = &dict_freeradius }, + { .out = &request_attr_state, .name = "session-state", .type = FR_TYPE_GROUP, .dict = &dict_freeradius }, + { NULL } +}; + /** The thread local free list * * Any entries remaining in the list will be freed when the thread is joined @@ -35,7 +61,7 @@ RCSID("$Id$") static _Thread_local fr_dlist_head_t *request_free_list; /* macro */ #ifndef NDEBUG -static int _state_ctx_free(TALLOC_CTX *state) +static int _state_ctx_free(fr_pair_t *state) { DEBUG4("state-ctx %p freed", state); @@ -43,20 +69,140 @@ static int _state_ctx_free(TALLOC_CTX *state) } #endif +static inline void CC_HINT(always_inline) request_log_init_orphan(request_t *request) +{ + /* + * These may be changed later by request_pre_handler + */ + request->log.lvl = fr_debug_lvl; /* Default to global debug level */ + if (!request->log.dst) { + request->log.dst = talloc_zero(request, log_dst_t); + } else { + memset(request->log.dst, 0, sizeof(*request->log.dst)); + } + request->log.dst->func = vlog_request; + request->log.dst->uctx = &default_log; +} + +static inline void CC_HINT(always_inline) request_log_init_child(request_t *child, request_t const *parent) +{ + /* + * Copy debug information. + */ + memcpy(&(child->log), &(parent->log), sizeof(child->log)); + child->log.unlang_indent = 0; /* Apart from the indent which we reset */ + child->log.module_indent = 0; /* Apart from the indent which we reset */ +} + +static inline void CC_HINT(always_inline) request_log_init_detachable(request_t *child, request_t const *parent) +{ + /* + * Ensure that we use our own version of the logging + * information, and not the original request one. + */ + child->log.dst = talloc_zero(child, log_dst_t); + memcpy(child->log.dst, parent->log.dst, sizeof(*child->log.dst)); +} + +static inline CC_HINT(always_inline) int request_detachable_init(request_t *child, request_t *parent) +{ + /* + * Associate the child with the parent, using the child's + * pointer as a unique identifier. Free it if the parent + * goes away, but don't persist it across + * challenge-response boundaries. + */ + if (request_data_talloc_add(parent, child, 0, request_t, child, true, true, false) < 0) return -1; + + return 0; +} + +static inline CC_HINT(always_inline) int request_child_init(request_t *child, request_t *parent) +{ + child->number = parent->child_number++; + child->name = talloc_typed_asprintf(child, "%s.%" PRIu64 , parent->name, child->number); + child->seq_start = 0; /* children always start with their own sequence */ + child->parent = parent; + child->dict = parent->dict; + child->config = parent->config; + child->client = parent->client; + + /* + * For new server support. + * + * FIXME: Key instead off of a "virtual server" data structure. + * + * FIXME: Permit different servers for inner && outer sessions? + */ + child->server_cs = parent->server_cs; + + child->packet = fr_radius_packet_alloc(child, true); + if (!child->packet) { + talloc_free(child); + return -1; + } + + child->reply = fr_radius_packet_alloc(child, false); + if (!child->reply) { + talloc_free(child); + return -1; + } + + /* + * Fill in the child request. + */ + child->packet->socket = parent->packet->socket; + child->packet->socket.inet.dst_port = 0; + child->packet->socket.fd = -1; + + /* + * This isn't STRICTLY required, as the child request MUST NEVER + * be put into the request list. However, it's still reasonable + * practice. + */ + child->packet->id = child->number & 0xff; + child->packet->code = parent->packet->code; + child->packet->timestamp = parent->packet->timestamp; + + /* + * Required for new identity support + */ + child->listener = parent->listener; + + /* + * Fill in the child reply, based on the child request. + */ + fr_socket_addr_swap(&child->reply->socket, &child->packet->socket); + child->reply->id = child->packet->id; + child->reply->code = 0; /* UNKNOWN code */ + child->reply->socket.fd = -1; + + return 0; +} + /** Setup logging and other fields for a request * * @param[in] file the request was allocated in. * @param[in] line the request was allocated on. * @param[in] request to (re)-initialise. */ -static void request_init(char const *file, int line, request_t *request) +static inline CC_HINT(always_inline) int request_init(char const *file, int line, + request_t *request, request_init_args_t const *args) { + + *request = (request_t){ #ifndef NDEBUG - request->magic = REQUEST_MAGIC; + .magic = REQUEST_MAGIC, #endif - - request->request_state = REQUEST_INIT; - request->master_state = REQUEST_ACTIVE; + .request_state = REQUEST_INIT, + .master_state = REQUEST_ACTIVE, + .dict = args->namespace, + .component = "", + .runnable_id = -1, + .time_order_id = -1, + .alloc_file = file, + .alloc_line = line + }; /* * Initialise the stack @@ -68,50 +214,66 @@ static void request_init(char const *file, int line, request_t *request) */ request_data_list_init(&request->data); - /* - * Initialise the state_ctx - */ - if (!request->state_ctx) { - request->state_ctx = talloc_init_const("session-state"); + { + fr_pair_t *vp = NULL, *pair_root; + + /* + * Alloc the pair root this is a + * special pair which does not + * free its children when it is + * freed. + */ + pair_root = fr_pair_root_afrom_da(request, request_attr_root); + if (unlikely(!pair_root)) return -1; + request->pair_root = pair_root; + + /* + * Copy all the pair lists over into + * the request. We then check for + * the any uninitialised lists and + * create them locally. + */ + memcpy(&request->pair_list, &args->pair_list, sizeof(request->pair_list)); + +#define list_init(_ctx, _list) \ + vp = fr_pair_afrom_da(_ctx, request_attr_##_list); \ + if (unlikely(!vp)) { \ + talloc_free(pair_root); \ + memset(&request->pair_list, 0, sizeof(request->pair_list)); \ + return -1; \ + } \ + fr_pair_add(&pair_root->children, vp); \ + request->pair_list._list = vp; + + if (!request->pair_list.request) list_init(request->pair_root, request); + if (!request->pair_list.reply) list_init(request->pair_root, reply); + if (!request->pair_list.control) list_init(request->pair_root, control); + if (!request->pair_list.state) { + list_init(NULL, state); #ifndef NDEBUG - talloc_set_destructor(request->state_ctx, _state_ctx_free); + talloc_set_destructor(request->pair_list.state, _state_ctx_free); #endif + } } /* - * These may be changed later by request_pre_handler + * Initialise packets and additional + * fields if this is going to be a + * child request. */ - request->log.lvl = fr_debug_lvl; /* Default to global debug level */ - if (!request->log.dst) { - request->log.dst = talloc_zero(request, log_dst_t); + if (args->parent) { + if (request_child_init(request, args->parent) < 0) return -1; + + if (args->detachable) { + if (request_detachable_init(request, args->parent) < 0) return -1; + request_log_init_detachable(request, args->parent); + } else { + request_log_init_child(request, args->parent); + } } else { - memset(request->log.dst, 0, sizeof(*request->log.dst)); + request_log_init_orphan(request); } - request->log.dst->func = vlog_request; - request->log.dst->uctx = &default_log; - - request->module = NULL; - request->component = ""; - - request->seq_start = 0; - request->runnable_id = -1; - request->time_order_id = -1; - - /* - * Where the request was allocated - */ - request->alloc_file = file; - request->alloc_line = line; - - fr_dlist_entry_init(&request->free_entry); /* Needs to be initialised properly, else bad things happen */ - - /* - * Initialise pair value lists - */ - fr_pair_list_init(&request->request_pairs); - fr_pair_list_init(&request->reply_pairs); - fr_pair_list_init(&request->control_pairs); - fr_pair_list_init(&request->state_pairs); + return 0; } /** Callback for freeing a request struct @@ -148,20 +310,27 @@ static int _request_free(request_t *request) * Ensure any data associated * with the state ctx is freed. */ - state_ctx = request->state_ctx; - if (request->state_ctx) { - fr_assert(!request->parent || (request->state_ctx != request->parent->state_ctx)); - talloc_free_children(request->state_ctx); + state_ctx = request->session_state_ctx; + fr_assert(talloc_parent(state_ctx) != request); /* Should never be directly parented */ + + if (request->session_state_ctx) { + fr_assert(!request->parent || (request->session_state_ctx != request->parent->session_state_ctx)); + talloc_free_children(request->session_state_ctx); + talloc_get_type_abort(request->session_state_ctx, fr_pair_t); } free_list = request_free_list; /* * Reinitialise the request */ + talloc_get_type_abort(request->session_state_ctx, fr_pair_t); + fr_assert_msg(talloc_parent(request->session_state_ctx) == NULL, "session_state_ctx %s", talloc_get_name(talloc_parent(request->session_state_ctx))); talloc_free_children(request); + talloc_get_type_abort(request->session_state_ctx, fr_pair_t); + memset(request, 0, sizeof(*request)); request->component = "free_list"; - request->state_ctx = state_ctx; /* Use the old, now cleared, state_ctx */ + request->session_state_ctx = state_ctx; /* Use the old, now cleared, state_ctx */ /* * Reinsert into the free list @@ -169,6 +338,9 @@ static int _request_free(request_t *request) fr_dlist_insert_head(free_list, request); request_free_list = free_list; + talloc_get_type_abort(request->session_state_ctx, fr_pair_t); + + return -1; /* Prevent free */ } @@ -185,14 +357,7 @@ really_free: * The reason why it's OK to do this, is if the state attributes * need to persist across requests, they will already have been * moved to a fr_state_entry_t, with the state pointers in the - * request being set to NULL, before the request is freed. - * - * Our state ctx can be the same as the parents if - * request_data_restore_to_child() was called. That - * function resets the childs state_ctx to be the same as - * the parents. Adding this check here means that we - * don't need to call request_detach() on a child which - * will be freed immediately after the detach. + * request being set to NULL, before the request is freed/ * * Note also that we do NOT call TALLOC_FREE(), which * sets state_ctx=NULL. We don't control the order in @@ -203,9 +368,10 @@ really_free: * If this assertion hits, it means that someone didn't * call fr_state_store_in_parent() */ - if (request->state_ctx) { - fr_assert(!request->parent || (request->state_ctx != request->parent->state_ctx)); - talloc_free(request->state_ctx); + if (request->session_state_ctx) { + fr_assert(!request->parent || (request->session_state_ctx != request->parent->session_state_ctx)); + + talloc_free(request->session_state_ctx); } #ifndef NDEBUG @@ -230,14 +396,51 @@ static void _request_free_list_free_on_exit(void *arg) talloc_free(list); } +static inline CC_HINT(always_inline) request_t *request_alloc_pool(TALLOC_CTX *ctx) +{ + request_t *request; + + /* + * Only allocate requests in the NULL + * ctx. There's no scenario where it's + * appropriate to allocate them in a + * pool, and using a strict talloc + * hierarchy means that child requests + * cannot be returned to a free list + * and would have to be freed. + */ + MEM(request = talloc_pooled_object(ctx, request_t, + 1 + /* Stack pool */ + UNLANG_STACK_MAX + /* Stack Frames */ + 2 + /* packets */ + 10, /* extra */ + (UNLANG_FRAME_PRE_ALLOC * UNLANG_STACK_MAX) + /* Stack memory */ + (sizeof(fr_pair_t) * 5) + /* pair lists and root*/ + (sizeof(fr_radius_packet_t) * 2) + /* packets */ + 128 /* extra */ + )); + fr_assert(ctx != request); + + return request; +} + /** Create a new request_t data structure * + * @param[in] file where the request was allocated. + * @param[in] line where the request was allocated. + * @param[in] ctx to bind the request to. + * @param[in] args Optional arguments. + * @return + * - A request on success. + * - NULL on error. */ -request_t *_request_alloc(char const *file, int line, TALLOC_CTX *ctx) +request_t *_request_alloc(char const *file, int line, TALLOC_CTX *ctx, request_init_args_t const *args) { - request_t *request; + request_t *request; fr_dlist_head_t *free_list; + if (!args) args = &default_args; + /* * Setup the free list, or return the free * list for this thread. @@ -252,24 +455,7 @@ request_t *_request_alloc(char const *file, int line, TALLOC_CTX *ctx) request = fr_dlist_head(free_list); if (!request) { - /* - * Only allocate requests in the NULL - * ctx. There's no scenario where it's - * appropriate to allocate them in a - * pool, and using a strict talloc - * hierarchy means that child requests - * cannot be returned to a free list - * and would have to be freed. - */ - MEM(request = talloc_zero_pooled_object(NULL, request_t, - 1 + /* Stack pool */ - UNLANG_STACK_MAX + /* Stack Frames */ - 2 + /* packets */ - 10, /* extra */ - (UNLANG_FRAME_PRE_ALLOC * UNLANG_STACK_MAX) + /* Stack memory */ - (sizeof(fr_radius_packet_t) * 2) + /* packets */ - 128 /* extra */ - )); + request = request_alloc_pool(ctx); talloc_set_destructor(request, _request_free); } else { /* @@ -279,7 +465,15 @@ request_t *_request_alloc(char const *file, int line, TALLOC_CTX *ctx) fr_dlist_remove(free_list, request); } - request_init(file, line, request); + if (request_init(file, line, request, args) < 0) { + talloc_free(request); + return NULL; + } + + /* + * Initialise entry in free list + */ + fr_dlist_entry_init(&request->free_entry); /* Needs to be initialised properly, else bad things happen */ /* * Bind lifetime to a parent. @@ -293,220 +487,132 @@ request_t *_request_alloc(char const *file, int line, TALLOC_CTX *ctx) return request; } -/** Allocate a request that's not in the free list - * - * This can be useful if modules need a persistent request for their own purposes - * which needs to be outside of the normal free list, so that it can be freed - * when the module requires, not when the thread destructor runs. - */ -request_t *_request_local_alloc(char const *file, int line, TALLOC_CTX *ctx) +static int _request_local_free(request_t *request) { - request_t *request; - - MEM(request = talloc_zero(ctx, request_t)); - - request_init(file, line, request); - /* - * Bind state_ctx to the request Apple - * Instruments seems to require this to not - * count the state_ctx as leaked. - * - * It's unclear if this is a genuine leak - * the previous code used a destructor - * to free the state_ctx. + * Ensure anything that might reference the request is + * freed before it is. */ - talloc_steal(request, request->state_ctx); - - return request; -} - -static request_t *request_init_fake(char const *file, int line, request_t *request, request_t *fake) -{ - fake->number = request->child_number++; - fake->name = talloc_typed_asprintf(fake, "%s.%" PRIu64 , request->name, fake->number); - - fake->seq_start = 0; /* children always start with their own sequence */ - - fake->parent = request; - fake->dict = request->dict; - fake->config = request->config; - fake->client = request->client; + talloc_free_children(request); /* - * For new server support. + * state_ctx is parented separately. * - * FIXME: Key instead off of a "virtual server" data structure. + * The reason why it's OK to do this, is if the state attributes + * need to persist across requests, they will already have been + * moved to a fr_state_entry_t, with the state pointers in the + * request being set to NULL, before the request is freed/ * - * FIXME: Permit different servers for inner && outer sessions? + * Note also that we do NOT call TALLOC_FREE(), which + * sets state_ctx=NULL. We don't control the order in + * which talloc frees the children. And the parents + * state_ctx pointer needs to stick around so that all of + * the children can check it. + * + * If this assertion hits, it means that someone didn't + * call fr_state_store_in_parent() */ - fake->server_cs = request->server_cs; - - fake->packet = fr_radius_packet_alloc(fake, true); - if (!fake->packet) { - talloc_free(fake); - return NULL; - } + if (request->session_state_ctx) { + fr_assert(!request->parent || (request->session_state_ctx != request->parent->session_state_ctx)); - fake->reply = fr_radius_packet_alloc(fake, false); - if (!fake->reply) { - talloc_free(fake); - return NULL; + talloc_free(request->session_state_ctx); } - fake->master_state = REQUEST_ACTIVE; - - /* - * Fill in the fake request. - */ - fake->packet->socket = request->packet->socket; - fake->packet->socket.inet.dst_port = 0; - fake->packet->socket.fd = -1; - - /* - * This isn't STRICTLY required, as the fake request MUST NEVER - * be put into the request list. However, it's still reasonable - * practice. - */ - fake->packet->id = fake->number & 0xff; - fake->packet->code = request->packet->code; - fake->packet->timestamp = request->packet->timestamp; - - /* - * Required for new identity support - */ - fake->listener = request->listener; - - /* - * Fill in the fake reply, based on the fake request. - */ - fr_socket_addr_swap(&fake->reply->socket, &fake->packet->socket); - fake->reply->id = fake->packet->id; - fake->reply->code = 0; /* UNKNOWN code */ - - /* - * Copy debug information. - */ - memcpy(&(fake->log), &(request->log), sizeof(fake->log)); - fake->log.unlang_indent = 0; /* Apart from the indent which we reset */ - fake->log.module_indent = 0; /* Apart from the indent which we reset */ - - /* - * Allocation information - */ - fake->alloc_file = file; - fake->alloc_line = line; - - return fake; -} - -/* - * Create a new request_t, based on an old one. - * - * This function allows modules to inject fake requests - * into the server, for tunneled protocols like TTLS & PEAP. - */ -request_t *_request_alloc_fake(char const *file, int line, request_t *request, fr_dict_t const *namespace) -{ - request_t *fake; - - fake = request_alloc(request); - if (!fake) return NULL; - - if (!request_init_fake(file, line, request, fake)) return NULL; - - if (namespace) fake->dict = namespace; +#ifndef NDEBUG + request->magic = 0x01020304; /* set the request to be nonsense */ +#endif - return fake; + return 0; } - -/** Allocate a fake request which is detachable from the parent. - * i.e. if the parent goes away, sometimes the child MAY continue to - * run. +/** Allocate a request that's not in the free list * + * This can be useful if modules need a persistent request for their own purposes + * which needs to be outside of the normal free list, so that it can be freed + * when the module requires, not when the thread destructor runs. */ -request_t *_request_alloc_detachable(char const *file, int line, request_t *request, fr_dict_t const *namespace) +request_t *_request_local_alloc(char const *file, int line, TALLOC_CTX *ctx, request_init_args_t const *args) { - request_t *fake; - - fake = _request_alloc(file, line, NULL); - if (!fake) return NULL; + request_t *request; - if (!request_init_fake(file, line, request, fake)) return NULL; + if (!args) args = &default_args; - if (namespace) fake->dict = namespace; + request = request_alloc_pool(ctx); + if (request_init(file, line, request, args) < 0) return NULL; - /* - * Ensure that we use our own version of the logging - * information, and not the original request one. - */ - fake->log.dst = talloc_zero(fake, log_dst_t); - memcpy(fake->log.dst, request->log.dst, sizeof(*fake->log.dst)); + talloc_set_destructor(request, _request_local_free); - /* - * Associate the child with the parent, using the child's - * pointer as a unique identifier. Free it if the parent - * goes away, but don't persist it across - * challenge-response boundaries. - */ - if (request_data_talloc_add(request, fake, 0, request_t, fake, true, true, false) < 0) { - talloc_free(fake); - return NULL; - } - - return fake; + return request; } /** Unlink a subrequest from its parent * * @note This should be used for requests in preparation for freeing them. * - * @param[in] fake request to unlink. + * @param[in] child request to unlink. * @param[in] will_free Caller super pinky swears to free * the request ASAP, and that it wont * touch persistable request data, - * request->state_ctx or request->state. + * request->session_state_ctx or request->state. * @return * - 0 on success. * - -1 on failure. */ -int request_detach(request_t *fake, bool will_free) +int request_detach(request_t *child, bool will_free) { - request_t *request = fake->parent; + request_t *request = child->parent; fr_assert(request != NULL); /* * Unlink the child from the parent. */ - request_data_get(request, fake, 0); + request_data_get(request, child, 0); /* * Fixup any sate or persistent * request data. */ - fr_state_detach(fake, will_free); + fr_state_detach(child, will_free); - fake->parent = NULL; + child->parent = NULL; while (!request->backlog) { fr_assert(request->parent != NULL); request = request->parent; } - fake->backlog = request->backlog; + child->backlog = request->backlog; + + return 0; +} + +int request_global_init(void) +{ + if (fr_dict_autoload(request_dict) < 0) { + PERROR("%s", __FUNCTION__); + return -1; + } + if (fr_dict_attr_autoload(request_dict_attr) < 0) { + PERROR("%s", __FUNCTION__); + fr_dict_autofree(request_dict); + return -1; + } return 0; } +void request_global_free(void) +{ + fr_dict_autofree(request_dict); +} + #ifdef WITH_VERIFY_PTR /* * Verify a packet. */ static void packet_verify(char const *file, int line, - request_t const *request, fr_radius_packet_t const *packet, - fr_pair_list_t const *list, char const *type) + request_t const *request, fr_radius_packet_t const *packet, char const *type) { TALLOC_CTX *parent; @@ -526,8 +632,6 @@ static void packet_verify(char const *file, int line, } PACKET_VERIFY(packet); - - fr_pair_list_verify(file, line, packet, list); } /* @@ -547,13 +651,23 @@ void request_verify(char const *file, int line, request_t const *request) "CONSISTENCY CHECK FAILED %s[%i]: expected request_t size of %zu bytes, got %zu bytes", file, line, sizeof(request_t), talloc_get_size(request)); - fr_pair_list_verify(file, line, request, &request->control_pairs); - fr_pair_list_verify(file, line, request->state_ctx, &request->state_pairs); + (void)talloc_get_type_abort(request->request_ctx, fr_pair_t); + fr_pair_list_verify(file, line, request->request_ctx, &request->request_pairs); + (void)talloc_get_type_abort(request->reply_ctx, fr_pair_t); + fr_pair_list_verify(file, line, request->reply_ctx, &request->reply_pairs); + (void)talloc_get_type_abort(request->control_ctx, fr_pair_t); + fr_pair_list_verify(file, line, request->control_ctx, &request->control_pairs); + (void)talloc_get_type_abort(request->session_state_ctx, fr_pair_t); + fr_assert_msg(talloc_parent(request->session_state_ctx) == NULL, + "session_state_ctx must not be parented by another chunk, but is parented by %s", + talloc_get_name(talloc_parent(request->session_state_ctx))); + + fr_pair_list_verify(file, line, request->session_state_ctx, &request->session_state_pairs); fr_assert(request->server_cs != NULL); if (request->packet) { - packet_verify(file, line, request, request->packet, &request->request_pairs, "request"); + packet_verify(file, line, request, request->packet, "request"); #if 0 /* * @todo - a multi-protocol server shouldn't have @@ -561,11 +675,11 @@ void request_verify(char const *file, int line, request_t const *request) */ if ((request->packet->code == FR_CODE_ACCESS_REQUEST) && (request->reply && !request->reply->code)) { - fr_assert(request->state_ctx != NULL); + fr_assert(request->session_state_ctx != NULL); } #endif } - if (request->reply) packet_verify(file, line, request, request->reply, &request->reply_pairs, "reply"); + if (request->reply) packet_verify(file, line, request, request->reply, "reply"); if (request->async) { (void) talloc_get_type_abort(request->async, fr_async_t); @@ -576,8 +690,8 @@ void request_verify(char const *file, int line, request_t const *request) (void) talloc_get_type_abort(rd, request_data_t); if (request_data_persistable(rd)) { - fr_assert(request->state_ctx); - fr_assert(talloc_parent(rd) == request->state_ctx); + fr_assert(request->session_state_ctx); + fr_assert(talloc_parent(rd) == request->session_state_ctx); } else { fr_assert(talloc_parent(rd) == request); } diff --git a/src/lib/server/request.h b/src/lib/server/request.h index b00c5028711..a6276682ed5 100644 --- a/src/lib/server/request.h +++ b/src/lib/server/request.h @@ -80,17 +80,112 @@ typedef enum request_state_t { typedef void (*fr_request_process_t)(request_t *, fr_state_signal_t); //!< Function handler for requests. typedef rlm_rcode_t (*RAD_REQUEST_FUNP)(request_t *); +extern fr_dict_attr_t const *request_attr_root; +extern fr_dict_attr_t const *request_attr_request; +extern fr_dict_attr_t const *request_attr_reply; +extern fr_dict_attr_t const *request_attr_control; +extern fr_dict_attr_t const *request_attr_state; + +/** Convenience macro for accessing the request list + * + * This should be used in the form `&request->request_pairs` + * to get a pointer to the head of the request list. + */ +#define request_pairs pair_list.request->children + +/** Talloc ctx for allocating request pairs under + */ +#define request_ctx pair_list.request + +/** Convenience macro for accessing the reply list + * + * This should be used in the form `&request->reply_pairs` + * to get a pointer to the head of the request list. + */ +#define reply_pairs pair_list.reply->children + +/** Talloc ctx for allocating reply pairs under + */ +#define reply_ctx pair_list.reply + +/** Convenience macro for accessing the control list + * + * This should be used in the form `&request->control_pairs` + * to get a pointer to the head of the request list. + */ +#define control_pairs pair_list.control->children + +/** Talloc ctx for allocating control pairs under + */ +#define control_ctx pair_list.control + +/** Convenience macro for accessing the state list + * + * This should be used in the form `&request->session_state_pairs` + * to get a pointer to the head of the request list. + */ +#define session_state_pairs pair_list.state->children + +/** Talloc ctx for allocating reply pairs under + */ +#define session_state_ctx pair_list.state + +/** Pair lists accessible from the request + * + */ +typedef struct { + fr_pair_t *request; //!< Pair containing the request list. + fr_pair_t *reply; //!< Pair containing the reply list. + fr_pair_t *control; //!< Pair containing the control list. + fr_pair_t *state; //!< Pair containing the state list. +} request_pair_lists_t; + struct request_s { #ifndef NDEBUG uint32_t magic; //!< Magic number used to detect memory corruption, //!< or request structs that have not been properly initialised. #endif + void *stack; //!< unlang interpreter stack. + + request_t *parent; //!< Request that generated this request. + uint64_t number; //!< Monotonically increasing request number. Reset on server restart. uint64_t child_number; //!< Monotonically increasing number for children of this request char const *name; //!< for debug printing, as (%d) is no longer sufficient + uint64_t seq_start; //!< State sequence ID. Stable identifier for a sequence of requests + //!< and responses. + + fr_pair_t *pair_root; //!< Root atribute which contains the + ///< other list attributes as children. + + /** Pair lists associated with the request + * + * @warn DO NOT allocate pairs directly beneath the root + * or in the ctx of the request. + * They MUST be allocated beneath their appropriate + * list attribute. + */ + request_pair_lists_t pair_list; //!< Structure containing all pair lists. + + /** Logging information + * + */ + struct { + log_dst_t *dst; //!< First in a list of log destinations. + + fr_log_lvl_t lvl; //!< Log messages with lvl >= to this should be logged. + + uint8_t unlang_indent; //!< By how much to indent log messages. uin8_t so it's obvious + //!< when a request has been exdented too much. + uint8_t module_indent; //!< Indentation after the module prefix name. + } log; + fr_dict_t const *dict; //!< Dictionary of the protocol that this request belongs to. + fr_radius_packet_t *packet; //!< Incoming request. + fr_radius_packet_t *reply; //!< Outgoing response. + fr_event_list_t *el; //!< thread-specific event list. fr_heap_t *backlog; //!< thread-specific backlog request_state_t request_state; //!< state for the various protocol handlers. @@ -100,21 +195,6 @@ struct request_s { rad_listen_t *listener; //!< The listener that received the request. RADCLIENT *client; //!< The client that originally sent us the request. - fr_radius_packet_t *packet; //!< Incoming request. - fr_radius_packet_t *reply; //!< Outgoing response. - - uint64_t seq_start; //!< State sequence ID. Stable identifier for a sequence of requests - //!< and responses. - - fr_pair_list_t request_list; - fr_pair_list_t reply_list; - fr_pair_list_t control_list; //!< #fr_pair_t list used to set per request parameters - //!< for modules and the server core at runtime. - TALLOC_CTX *state_ctx; //!< for request->state - fr_pair_list_t state_list; //!< #fr_pair_t list available over the lifetime of the authentication - //!< attempt. Useful where the attempt involves a sequence of - //!< many request/challenge packets, like OTP, and EAP. - rad_master_state_t master_state; //!< Set by the master thread to signal the child that's currently //!< working with the request, to do something. @@ -124,10 +204,6 @@ struct request_s { char const *component; //!< Section the request is in. char const *module; //!< Module the request is currently being processed by. - void *stack; //!< unlang interpreter stack. - - request_t *parent; - fr_event_timer_t const *ev; //!< Event in event loop tied to this request. int32_t runnable_id; //!< entry in the queue / heap of runnable packets @@ -135,16 +211,6 @@ struct request_s { main_config_t const *config; //!< Pointer to the main config hack to try and deal with hup. - struct { - log_dst_t *dst; //!< First in a list of log destinations. - - fr_log_lvl_t lvl; //!< Log messages with lvl >= to this should be logged. - - uint8_t unlang_indent; //!< By how much to indent log messages. uin8_t so it's obvious - //!< when a request has been exdented too much. - uint8_t module_indent; //!< Indentation after the module prefix name. - } log; - uint32_t options; //!< mainly for proxying EAP-MSCHAPv2. fr_async_t *async; //!< for new async listeners @@ -156,6 +222,23 @@ struct request_s { fr_dlist_t free_entry; //!< Request's entry in the free list. }; /* request_t typedef */ +/** Optional arguments for initialising requests + * + */ +typedef struct { + fr_dict_t const *namespace; //!< The namespace this request implements. + + request_t *parent; //!< If set, the request is a child request used to run + ///< policy sections and additional virtual servers. + + request_pair_lists_t pair_list; //!< Alternative pair list heads. + ///< These allow a request to expose nested attributes as + ///< request or reply lists from the parent. + + bool detachable; //!< Request should be detachable, i.e. able to run even + ///< if its parent exits. +} request_init_args_t; + #ifdef WITH_VERIFY_PTR # define REQUEST_VERIFY(_x) request_verify(__FILE__, __LINE__, _x) #else @@ -167,7 +250,6 @@ struct request_s { # define REQUEST_VERIFY(_x) fr_assert(_x) #endif - #define RAD_REQUEST_LVL_NONE (0) //!< No debug messages should be printed. #define RAD_REQUEST_LVL_DEBUG (1) #define RAD_REQUEST_LVL_DEBUG2 (2) @@ -177,19 +259,26 @@ struct request_s { #define RAD_REQUEST_OPTION_CTX (1 << 1) #define RAD_REQUEST_OPTION_DETAIL (1 << 2) -#define request_alloc(_ctx) _request_alloc( __FILE__, __LINE__, _ctx) -request_t *_request_alloc(char const *file, int line, TALLOC_CTX *ctx); - -#define request_local_alloc(_ctx) _request_local_alloc(__FILE__, __LINE__, _ctx) -request_t *_request_local_alloc(char const *file, int line, TALLOC_CTX *ctx); +/** Allocate a new request + * + * @param[in] _ctx Talloc ctx to bind the request to. + * @param[in] _args Optional arguments that control how the request is initialised. + */ +#define request_alloc(_ctx, _args) _request_alloc( __FILE__, __LINE__, (_ctx), (_args)) +request_t *_request_alloc(char const *file, int line, TALLOC_CTX *ctx, request_init_args_t const *args); -#define request_alloc_fake(_request, _namespace) _request_alloc_fake(__FILE__, __LINE__, _request, _namespace) -request_t *_request_alloc_fake(char const *file, int line, request_t *parent, fr_dict_t const *namespace); +/** Allocate a new request outside of the request pool + * + * @param[in] _ctx Talloc ctx to allocate the request in. + * @param[in] _args Optional arguments that control how the request is initialised. + */ +#define request_local_alloc(_ctx, _args) _request_local_alloc(__FILE__, __LINE__, (_ctx), (_args)) +request_t *_request_local_alloc(char const *file, int line, TALLOC_CTX *ctx, request_init_args_t const *args); -#define request_alloc_detachable(_request, _namespace) _request_alloc_detachable(__FILE__, __LINE__, _request, _namespace) -request_t *_request_alloc_detachable(char const *file, int line, request_t *request, fr_dict_t const *namespace); +int request_detach(request_t *child, bool will_free); -int request_detach(request_t *fake, bool will_free); +int request_global_init(void); +void request_global_free(void); #ifdef WITH_VERIFY_PTR void request_verify(char const *file, int line, request_t const *request); /* only for special debug builds */ diff --git a/src/lib/server/request_data.c b/src/lib/server/request_data.c index 7c58190be50..3e729c1a7ba 100644 --- a/src/lib/server/request_data.c +++ b/src/lib/server/request_data.c @@ -195,9 +195,9 @@ int _request_data_add(request_t *request, void const *unique_ptr, int unique_int * Request must have a state ctx */ fr_assert(request); - fr_assert(!persist || request->state_ctx); + fr_assert(!persist || request->session_state_ctx); fr_assert(!persist || - (talloc_parent(opaque) == request->state_ctx) || + (talloc_parent(opaque) == request->session_state_ctx) || (talloc_parent(opaque) == talloc_null_ctx())); fr_assert(!free_on_parent || (talloc_parent(opaque) != request)); @@ -243,8 +243,8 @@ int _request_data_add(request_t *request, void const *unique_ptr, int unique_int */ if (!rd) { if (persist) { - fr_assert(request->state_ctx); - rd = request_data_alloc(request->state_ctx); + fr_assert(request->session_state_ctx); + rd = request_data_alloc(request->session_state_ctx); } else { rd = request_data_alloc(request); } @@ -501,133 +501,6 @@ void request_data_dump(request_t *request) request_data_list_dump(request, &request->data); } - -/** Free any subrequest request data if the dlist head is freed - * - */ -static int _free_child_data(fr_dlist_head_t *head) -{ - request_data_t *rd = NULL, *prev; - - while ((rd = fr_dlist_next(head, rd))) { - prev = fr_dlist_remove(head, rd); - talloc_free(rd); - rd = prev; - } - - return 0; -} - -/** Store persistable data from a subrequest in its parent - * - * @note #request_data_restore_to_child should have been called - * on the request before any request data is added to it, - * so that the state_ctx is set correctly. - * - * @param[in] request The child request to retrieve state from. - * @param[in] unique_ptr A parent may have multiple subrequests spawned - * by different modules. This identifies the module - * or other facility that spawned the subrequest. - * @param[in] unique_int Further identification. - * @return - * - 0 on success. - * - <0 on failure. - */ -int request_data_store_in_parent(request_t *request, void const *unique_ptr, int unique_int) -{ - fr_dlist_head_t *child_list; - - if (!fr_cond_assert_msg((request->state_ctx == request->parent->state_ctx), - "Child's state_ctx (%p), differs from parent's (%p). " - "Unbalanced calls to request_data_restore_to_child/request_data_store_in_parent or " - "request_data_restore_to_child never called", - request->state_ctx, request->parent->state_ctx)) return -1; - - /* - * Move child request data back into its - * parent. - */ - if (request_data_by_persistance_count(request, true) > 0) { - MEM(child_list = talloc_zero(request->parent->state_ctx, fr_dlist_head_t)); - fr_dlist_talloc_init(child_list, request_data_t, list); - talloc_set_destructor(child_list, _free_child_data); - - /* - * Pull everything out of the child, - * add it to our temporary list head... - */ - request_data_by_persistance(child_list, request, true); - - /* - * ...and add the request_data from - * the child back into the parent. - */ - request_data_talloc_add(request->parent, unique_ptr, unique_int, - fr_dlist_head_t, child_list, true, false, true); - } - - /* - * Ensure request_data_restore_to_child - * can be called again if it's actually - * needed, by giving the child it's own - * unique state_ctx again. - */ - MEM(request->state_ctx = talloc_init_const("session-state")); - - return 0; -} - -/** Restore subrequest data from a parent request - * - * @param[in] request The child request to restore state to. - * @param[in] unique_ptr A parent may have multiple subrequests spawned - * by different modules. This identifies the module - * or other facility that spawned the subrequest. - * @param[in] unique_int Further identification. - * @return - * - 0 on success. - * - <0 on failure. - */ -int request_data_restore_to_child(request_t *request, void const *unique_ptr, int unique_int) -{ - fr_dlist_head_t *head; - - /* - * All requests are alloced with a state_ctx. - * In this case, nothing should be parented - * off it already, so we can just free it. - */ - if (!fr_cond_assert_msg((talloc_get_size(request->state_ctx) == 0), - "Child's state_ctx must contain no " - "allocations when restoring data from a parent")) return -1; - - /* - * If the parent and child state_ctxs are the - * same, then it means this function has been - * called before, without request_data_store_in_parent - * being called afterwards. - */ - if (!fr_cond_assert_msg((request->state_ctx != request->parent->state_ctx), - "Child's state_ctx must not be equal to its parent's state_ctx. " - "Unbalanced calls to request_data_restore_to_child/request_data_store_in_parent")) - return -1; - - /* - * Destroy the child's state_ctx and have it - * use its parent's. - */ - TALLOC_FREE(request->state_ctx); - request->state_ctx = request->parent->state_ctx; /* Use top level state ctx */ - - head = request_data_get(request->parent, unique_ptr, unique_int); - if (!head) return 0; - - request_data_restore(request, head); - talloc_free(head); - - return 0; -} - #ifdef WITH_VERIFY_PTR bool request_data_persistable(request_data_t *rd) { diff --git a/src/lib/server/request_data.h b/src/lib/server/request_data.h index ebf644fa1cd..236092b8565 100644 --- a/src/lib/server/request_data.h +++ b/src/lib/server/request_data.h @@ -109,10 +109,6 @@ void request_data_list_dump(request_t *request, fr_dlist_head_t *head); void request_data_dump(request_t *request); -int request_data_store_in_parent(request_t *request, void const *unique_ptr, int unique_int); - -int request_data_restore_to_child(request_t *request, void const *unique_ptr, int unique_int); - #ifdef WITH_VERIFY_PTR bool request_data_persistable(request_data_t *rd); diff --git a/src/lib/server/snmp.c b/src/lib/server/snmp.c index 1605b77e971..8aa882547c9 100644 --- a/src/lib/server/snmp.c +++ b/src/lib/server/snmp.c @@ -596,7 +596,7 @@ static ssize_t snmp_process_index(fr_cursor_t *out, request_t *request, return -(depth); } - MEM(vp = fr_pair_afrom_da(request->reply, da)); + MEM(vp = fr_pair_afrom_da(request->reply_ctx, da)); vp->vp_uint32 = i; fr_cursor_prepend(out, vp); @@ -805,11 +805,11 @@ static ssize_t snmp_process_leaf(fr_cursor_t *out, request_t *request, */ if (map_p->get(request->reply, &data, map_p, snmp_ctx) < 0) goto error; - MEM(vp = fr_pair_afrom_da(request->reply, map_p->da)); + MEM(vp = fr_pair_afrom_da(request->reply_ctx, map_p->da)); fr_value_box_steal(vp, &vp->data, &data); fr_cursor_append(out, vp); - MEM(vp = fr_pair_afrom_da(request->reply, attr_snmp_type)); + MEM(vp = fr_pair_afrom_da(request->reply_ctx, attr_snmp_type)); vp->vp_uint32 = map_p->type; fr_cursor_append(out, vp); } @@ -820,7 +820,7 @@ static ssize_t snmp_process_leaf(fr_cursor_t *out, request_t *request, ssize_t ret; if (!map_p->set || (map_p->type == FR_FREERADIUS_SNMP_TYPE_OBJECT)) { - MEM(vp = fr_pair_afrom_da(request->reply, attr_snmp_failure)); + MEM(vp = fr_pair_afrom_da(request->reply_ctx, attr_snmp_failure)); vp->vp_uint32 = FR_FREERADIUS_SNMP_FAILURE_VALUE_NOT_WRITABLE; fr_cursor_append(out, vp); return 0; @@ -834,7 +834,7 @@ static ssize_t snmp_process_leaf(fr_cursor_t *out, request_t *request, case FR_FREERADIUS_SNMP_FAILURE_VALUE_WRONG_LENGTH: case FR_FREERADIUS_SNMP_FAILURE_VALUE_WRONG_VALUE: case FR_FREERADIUS_SNMP_FAILURE_VALUE_INCONSISTENT_VALUE: - MEM(vp = fr_pair_afrom_da(request->reply, attr_snmp_failure)); + MEM(vp = fr_pair_afrom_da(request->reply_ctx, attr_snmp_failure)); vp->vp_uint32 = -(ret); fr_cursor_append(out, vp); break; diff --git a/src/lib/server/state.c b/src/lib/server/state.c index 1889291febc..f9a21999807 100644 --- a/src/lib/server/state.c +++ b/src/lib/server/state.c @@ -47,9 +47,10 @@ RCSID("$Id$") #include +#include #include -#include +#include #include #include #include @@ -103,15 +104,32 @@ typedef struct { int tries; - TALLOC_CTX *ctx; //!< ctx to parent any data that needs to be - //!< tied to the lifetime of the request progression. - fr_pair_t *vps; //!< session-state fr_pair_ts, parented by ctx. + fr_pair_t *ctx; //!< for all session specific data. fr_dlist_head_t data; //!< Persistable request data, also parented by ctx. - request_t *thawed; //!< The request that thawed this entry. + request_t *thawed; //!< The request that thawed this entry. } fr_state_entry_t; +/** A child of a fr_state_entry_t + * + * Children are tracked using the request data of parents. + * + * request data is added with identifiers that uniquely identify the + * subrequest it should be restored to. + * + * In this way a top level fr_state_entry_t can hold the session + * information for multiple children, and the children may hold + * state_child_entry_ts for grandchildren. + */ +typedef struct { + fr_pair_t *ctx; //!< for all session specific data. + + fr_dlist_head_t data; //!< Persistable request data, also parented by ctx. + + request_t *thawed; //!< The request that thawed this entry. +} state_child_entry_t; + struct fr_state_tree_s { uint64_t id; //!< Next ID to assign. uint64_t timed_out; //!< Number of states that were cleaned up due to @@ -260,7 +278,7 @@ static int _state_entry_free(fr_state_entry_t *entry) * by the state context. */ if (entry->ctx) { - for (vp = fr_cursor_init(&cursor, &entry->vps); + for (vp = fr_cursor_init(&cursor, &entry->ctx->children); vp; vp = fr_cursor_next(&cursor)) { fr_assert(entry->ctx == talloc_parent(vp)); @@ -388,11 +406,7 @@ static fr_state_entry_t *state_entry_create(fr_state_tree_t *state, request_t *r * Allocation doesn't need to occur inside the critical region * and would add significantly to contention. */ - entry = talloc_zero(NULL, fr_state_entry_t); - if (!entry) { - PTHREAD_MUTEX_LOCK(&state->mutex); /* Caller expects this to be locked */ - return NULL; - } + MEM(entry = talloc_zero(NULL, fr_state_entry_t)); request_data_list_init(&entry->data); talloc_set_destructor(entry, _state_entry_free); @@ -475,7 +489,7 @@ static fr_state_entry_t *state_entry_create(fr_state_tree_t *state, request_t *r */ entry->state_comp.server_id = state->server_id; - MEM(vp = fr_pair_afrom_da(request, state->da)); + MEM(vp = fr_pair_afrom_da(request->reply_ctx, state->da)); fr_pair_value_memdup(vp, entry->state, sizeof(entry->state), false); fr_pair_add(reply_list, vp); } @@ -586,10 +600,9 @@ void fr_state_discard(fr_state_tree_t *state, request_t *request) * stupid like add additional session-state attributes * in one of the later sections. */ - TALLOC_FREE(request->state_ctx); - request->state_pairs = NULL; + TALLOC_FREE(request->session_state_ctx); - MEM(request->state_ctx = talloc_init_const("session-state")); + MEM(request->session_state_ctx = fr_pair_afrom_da(NULL, request_attr_state)); RDEBUG3("%s - discarded", state->da->name); @@ -609,8 +622,6 @@ void fr_state_to_request(fr_state_tree_t *state, request_t *request) TALLOC_CTX *old_ctx = NULL; fr_pair_t *vp; - fr_assert(request->state_pairs == NULL); - /* * No State, don't do anything. */ @@ -630,24 +641,22 @@ void fr_state_to_request(fr_state_tree_t *state, request_t *request) PTHREAD_MUTEX_UNLOCK(&state->mutex); return; } - if (request->state_ctx) old_ctx = request->state_ctx; /* Store for later freeing */ + if (request->session_state_ctx) old_ctx = request->session_state_ctx; /* Store for later freeing */ fr_assert(entry->ctx); request->seq_start = entry->seq_start; - request->state_ctx = entry->ctx; - request->state_pairs = entry->vps; + request->session_state_ctx = entry->ctx; request_data_restore(request, &entry->data); entry->ctx = NULL; - entry->vps = NULL; entry->thawed = request; } PTHREAD_MUTEX_UNLOCK(&state->mutex); - if (request->state_pairs) { + if (request->session_state_pairs) { RDEBUG2("Restored &session-state"); - log_request_pair_list(L_DBG_LVL_2, request, NULL, &request->state_pairs, "&session-state."); + log_request_pair_list(L_DBG_LVL_2, request, NULL, &request->session_state_pairs, "&session-state."); } /* @@ -664,7 +673,7 @@ void fr_state_to_request(fr_state_tree_t *state, request_t *request) /** Transfer ownership of the state fr_pair_ts and ctx, back to a state entry * - * Put request->state_pairs into the State attribute. Put the State attribute + * Put request->session_state_pairs into the State attribute. Put the State attribute * into the vps list. Delete the original entry, if it exists * * Also creates a new state entry. @@ -678,11 +687,11 @@ int fr_request_to_state(fr_state_tree_t *state, request_t *request) request_data_list_init(&data); request_data_by_persistance(&data, request, true); - if (!request->state_pairs && fr_dlist_empty(&data)) return 0; + if (!request->session_state_pairs && fr_dlist_empty(&data)) return 0; - if (request->state_pairs) { + if (request->session_state_pairs) { RDEBUG2("Saving &session-state"); - log_request_pair_list(L_DBG_LVL_2, request, NULL, &request->state_pairs, "&session-state."); + log_request_pair_list(L_DBG_LVL_2, request, NULL, &request->session_state_pairs, "&session-state."); } vp = fr_pair_find_by_da(&request->request_pairs, state->da); @@ -699,15 +708,13 @@ int fr_request_to_state(fr_state_tree_t *state, request_t *request) } fr_assert(entry->ctx == NULL); - fr_assert(request->state_ctx); + fr_assert(request->session_state_ctx); entry->seq_start = request->seq_start; - entry->ctx = request->state_ctx; - entry->vps = request->state_pairs; + entry->ctx = request->session_state_ctx; fr_dlist_move(&entry->data, &data); - request->state_ctx = NULL; - request->state_pairs = NULL; + request->session_state_ctx = NULL; PTHREAD_MUTEX_UNLOCK(&state->mutex); @@ -717,75 +724,112 @@ int fr_request_to_state(fr_state_tree_t *state, request_t *request) return 0; } +/** Free any subrequest request data if the dlist head is freed + * + */ +static int _free_child_data(state_child_entry_t *child_entry) +{ + fr_dlist_talloc_free(&child_entry->data); + + return 0; +} + /** Store subrequest's session-state list and persistable request data in its parent * - * @param[in] request The child request to retrieve state from. + * @param[in] child The child request to retrieve state from. * @param[in] unique_ptr A parent may have multiple subrequests spawned * by different modules. This identifies the module * or other facility that spawned the subrequest. * @param[in] unique_int Further identification. */ -void fr_state_store_in_parent(request_t *request, void const *unique_ptr, int unique_int) +void fr_state_store_in_parent(request_t *child, void const *unique_ptr, int unique_int) { - if (!fr_cond_assert_msg(request->parent, + state_child_entry_t *child_entry; + request_t *request = child; /* Stupid logging */ + + if (!fr_cond_assert_msg(child->parent, "Child request must have request->parent set when storing state")) return; - RDEBUG3("Storing subrequest state in request %s", request->parent->name); + RDEBUG3("Storing subrequest state in request %s", child->parent->name); + + if ((request_data_by_persistance_count(request, true) > 0) || + !fr_pair_list_empty(&request->session_state_pairs)) { + MEM(child_entry = talloc_zero(request->parent->session_state_ctx, state_child_entry_t)); + request_data_list_init(&child_entry->data); + talloc_set_destructor(child_entry, _free_child_data); + + /* + * Pull everything out of the child, + * add it to our temporary list head... + * + * request_data_add allocs persistable + * request dta in the session_state_ctx + * which is why we don't need to copy or + * reparent any of this. + */ + request_data_by_persistance(&child_entry->data, request, true); - /* - * Shove this into the child to make - * it easier to store/restore the - * whole lot... - */ - if (request->state_pairs) { /* - * If parent and child share a state_ctx - * which they usually should do, then just - * add the state list into request_data_t - * and don't bother copying. + * ...and add the request_data from + * the child back into the parent. */ - request_data_talloc_add(request, (void *)fr_state_store_in_parent, 0, fr_pair_t, - request->state_pairs, true, false, true); - request->state_pairs = NULL; + request_data_talloc_add(request->parent, unique_ptr, unique_int, + state_child_entry_t, child_entry, true, false, true); } /* - * Contains asserts to protect against - * Unbalanced or out of order calls to - * fr_state_store_in_parent and fr_state_restore_to_child. + * Ensure fr_state_restore_to_child + * can be called again if it's actually + * needed, by giving the child it's own + * unique state_ctx again. */ - if (request_data_store_in_parent(request, unique_ptr, unique_int) < 0) { - request->state_pairs = request_data_get(request, (void *)fr_state_store_in_parent, 0); - return; - } + MEM(request->session_state_ctx = fr_pair_afrom_da(NULL, request_attr_state)); } /** Restore subrequest data from a parent request * - * @param[in] request The child request to restore state to. + * @param[in] child The child request to restore state to. * @param[in] unique_ptr A parent may have multiple subrequests spawned * by different modules. This identifies the module * or other facility that spawned the subrequest. * @param[in] unique_int Further identification. */ -void fr_state_restore_to_child(request_t *request, void const *unique_ptr, int unique_int) +void fr_state_restore_to_child(request_t *child, void const *unique_ptr, int unique_int) { - if (!fr_cond_assert_msg(request->parent, + state_child_entry_t *child_entry; + request_t *request = child; /* Stupid logging */ + + if (!fr_cond_assert_msg(child->parent, "Child request must have request->parent set when restoring state")) return; - RDEBUG3("Restoring subrequest state from request %s", request->parent->name); + + child_entry = request_data_get(child->parent, unique_ptr, unique_int); + if (!child_entry) { + RDEBUG3("No child state found in parent %s", child->parent->name); + return; + } /* - * Contains asserts to protect against - * Unbalanced or out of order calls to - * fr_state_store_in_parent and fr_state_restore_to_child. + * Shouldn't really be possible unless + * there's a logic bug in this API. */ - if (request_data_restore_to_child(request, unique_ptr, unique_int) < 0) return; + if (!fr_cond_assert_msg(!child_entry->thawed, + "Child state entry already thawed by %s - %p", + child_entry->thawed->name, child_entry->thawed)) return; + + RDEBUG3("Restoring subrequest state from request %s", child->parent->name); /* - * Get the state vps back + * If we can restore from the parent, do so */ - request->state_pairs = request_data_get(request, (void *)fr_state_store_in_parent, 0); + TALLOC_FREE(child->session_state_ctx); + child->session_state_ctx = child_entry->ctx; + child_entry->ctx = NULL; /* No longer owns the ctx */ + child_entry->thawed = child; + + request_data_restore(child, &child_entry->data); /* Put all the request data back */ + + talloc_free(child_entry); } /** Move all request data and session-state VPs into a new state_ctx @@ -799,50 +843,36 @@ void fr_state_restore_to_child(request_t *request, void const *unique_ptr, int u * @param[in] will_free Caller super pinky swears to free * the request ASAP, and that it wont * touch persistable request data, - * request->state_ctx or request->state. + * request->session_state_ctx or request->state. */ void fr_state_detach(request_t *request, bool will_free) { - fr_pair_list_t vps; - TALLOC_CTX *new_state_ctx; - - fr_pair_list_init(&vps); - if (unlikely(request->parent == NULL)) return; + fr_pair_t *new_state_ctx; if (will_free) { - fr_pair_list_free(&request->state_pairs); + fr_pair_list_free(&request->session_state_pairs); /* * The non-persistable stuff is * prented directly by the request */ request_data_persistable_free(request); - - /* - * Parent will take care of freeing - * honestly this should probably - * be an assert. - */ - if (request->state_ctx == request->parent->state_ctx) request->state_ctx = NULL; return; } - MEM(new_state_ctx = talloc_init_const("session-state")); + /* + * Alloc a new pairlist to use as the + * state ctx and the head of the + * state list. + */ + MEM(new_state_ctx = fr_pair_afrom_da(NULL, request_attr_state)); request_data_by_persistance_reparent(new_state_ctx, NULL, request, true); request_data_by_persistance_reparent(new_state_ctx, NULL, request, false); - (void) fr_pair_list_copy(new_state_ctx, &vps, &request->state_pairs); - fr_pair_list_free(&request->state_pairs); + (void) fr_pair_list_copy(new_state_ctx, &new_state_ctx->children, &request->session_state_pairs); + fr_pair_list_free(&request->session_state_pairs); - request->state_pairs = vps; - - /* - * ...again, should probably - * not happen and should probably - * be an assert. - */ - if (request->state_ctx != request->parent->state_ctx) talloc_free(request->state_ctx); - request->state_ctx = new_state_ctx; + request->session_state_ctx = new_state_ctx; } /** Return number of entries created diff --git a/src/lib/server/tmpl_eval.c b/src/lib/server/tmpl_eval.c index be65f5633f8..cbc83079e08 100644 --- a/src/lib/server/tmpl_eval.c +++ b/src/lib/server/tmpl_eval.c @@ -66,7 +66,7 @@ fr_pair_list_t *tmpl_list_head(request_t *request, tmpl_pair_list_t list) return &request->control_pairs; case PAIR_LIST_STATE: - return &request->state_pairs; + return &request->session_state_pairs; } RWDEBUG2("List \"%s\" is not available", @@ -96,16 +96,16 @@ TALLOC_CTX *tmpl_list_ctx(request_t *request, tmpl_pair_list_t list) switch (list) { case PAIR_LIST_REQUEST: - return request->packet; + return request->request_ctx; case PAIR_LIST_REPLY: - return request->reply; + return request->reply_ctx; case PAIR_LIST_CONTROL: - return request; + return request->control_ctx; case PAIR_LIST_STATE: - return request->state_ctx; + return request->session_state_ctx; /* Don't add default */ case PAIR_LIST_UNKNOWN: diff --git a/src/lib/server/trigger.c b/src/lib/server/trigger.c index 6cebe0dbc65..7a3c4ad74fc 100644 --- a/src/lib/server/trigger.c +++ b/src/lib/server/trigger.c @@ -205,7 +205,7 @@ static unlang_action_t trigger_process(rlm_rcode_t *p_result, module_ctx_t const /* * Bootstrap these for simpliciy. */ - (void) fr_pair_list_copy(request->packet, &request->request_pairs, &ctx->vps); + (void) fr_pair_list_copy(request->request_ctx, &request->request_pairs, &ctx->vps); if (unlang_interpret_push_instruction(request, NULL, RLM_MODULE_REJECT, UNLANG_TOP_FRAME) < 0) { @@ -376,7 +376,7 @@ int trigger_exec(request_t *request, CONF_SECTION const *cs, char const *name, b /* * radius_exec_program always needs a request. */ - fake = request_alloc(NULL); + fake = request_alloc(NULL, NULL); memcpy(&fake->server_cs, &subcs, sizeof(subcs)); /* completely wrong, but we need to use _something_ */ /* diff --git a/src/lib/tls/cache.c b/src/lib/tls/cache.c index efcacc16f45..0d16641370e 100644 --- a/src/lib/tls/cache.c +++ b/src/lib/tls/cache.c @@ -283,12 +283,12 @@ int fr_tls_cache_write(request_t *request, fr_tls_session_t *tls_session) /* * Put the SSL data into an attribute. */ - MEM(vp = fr_pair_afrom_da(request->state_ctx, attr_tls_session_data)); + MEM(vp = fr_pair_afrom_da(request->session_state_ctx, attr_tls_session_data)); fr_pair_value_memdup(vp, tls_session->session_blob, talloc_array_length(tls_session->session_blob), false); RINDENT(); RDEBUG2("&session-state.%pP", vp); REXDENT(); - fr_pair_add(&request->state_pairs, vp); + fr_pair_add(&request->session_state_pairs, vp); /* * Call the virtual server to write the session @@ -308,7 +308,7 @@ int fr_tls_cache_write(request_t *request, fr_tls_session_t *tls_session) /* * Ensure that the session data can't be used by anyone else. */ - fr_pair_delete_by_da(&request->state_pairs, attr_tls_session_data); + fr_pair_delete_by_da(&request->session_state_pairs, attr_tls_session_data); return ret; } @@ -365,7 +365,7 @@ static SSL_SESSION *fr_tls_cache_read(SSL *ssl, return NULL; } - vp = fr_pair_find_by_da(&request->state_pairs, attr_tls_session_data); + vp = fr_pair_find_by_da(&request->session_state_pairs, attr_tls_session_data); if (!vp) { RWDEBUG("No cached session found"); return NULL; @@ -411,7 +411,7 @@ static SSL_SESSION *fr_tls_cache_read(SSL *ssl, /* * Ensure that the session data can't be used by anyone else. */ - fr_pair_delete_by_da(&request->state_pairs, attr_tls_session_data); + fr_pair_delete_by_da(&request->session_state_pairs, attr_tls_session_data); return sess; } diff --git a/src/lib/tls/session.c b/src/lib/tls/session.c index 81ea6d1f212..3419596f53f 100644 --- a/src/lib/tls/session.c +++ b/src/lib/tls/session.c @@ -1092,7 +1092,7 @@ int fr_tls_session_pairs_from_x509_cert(fr_cursor_t *cursor, TALLOC_CTX *ctx, continue; } - MEM(vp = fr_pair_afrom_da(request, da)); + MEM(vp = fr_pair_afrom_da(request->request_ctx, da)); if (fr_pair_value_from_str(vp, value, -1, '\0', true) < 0) { RPWDEBUG3("Skipping: %s += '%s'", attribute, value); talloc_free(vp); @@ -1437,10 +1437,10 @@ int fr_tls_session_handshake(request_t *request, fr_tls_session_t *session) RDEBUG2("Cipher suite: %s", cipher_desc_clean); RDEBUG2("Adding TLS session information to request"); - vp = fr_pair_afrom_da(request->state_ctx, attr_tls_session_cipher_suite); + vp = fr_pair_afrom_da(request->session_state_ctx, attr_tls_session_cipher_suite); if (vp) { fr_pair_value_strdup(vp, SSL_CIPHER_get_name(cipher)); - fr_pair_add(&request->state_pairs, vp); + fr_pair_add(&request->session_state_pairs, vp); RINDENT(); RDEBUG2("&session-state.%pP", vp); REXDENT(); @@ -1453,10 +1453,10 @@ int fr_tls_session_handshake(request_t *request, fr_tls_session_t *session) version = tls_version_str[session->info.version]; } - vp = fr_pair_afrom_da(request->state_ctx, attr_tls_session_version); + vp = fr_pair_afrom_da(request->session_state_ctx, attr_tls_session_version); if (vp) { fr_pair_value_strdup(vp, version); - fr_pair_add(&request->state_pairs, vp); + fr_pair_add(&request->session_state_pairs, vp); RINDENT(); RDEBUG2("&session-state.TLS-Session-Version := \"%s\"", version); REXDENT(); @@ -1607,7 +1607,7 @@ fr_tls_session_t *fr_tls_session_init_client(TALLOC_CTX *ctx, fr_tls_conf_t *con return NULL; } - request = request_alloc(session); + request = request_alloc(session, NULL); fr_tls_session_request_bind(request, session->ssl); diff --git a/src/lib/tls/validate.c b/src/lib/tls/validate.c index 9acca4c7fd7..108f1f2c633 100644 --- a/src/lib/tls/validate.c +++ b/src/lib/tls/validate.c @@ -166,7 +166,7 @@ int fr_tls_validate_cert_cb(int ok, X509_STORE_CTX *x509_ctx) * cert_vps have a different talloc parent, so we * can't just reference them. */ - MEM(fr_pair_list_copy(request->state_ctx, &request->state_pairs, &cert_vps) >= 0); + MEM(fr_pair_list_copy(request->session_state_ctx, &request->session_state_pairs, &cert_vps) >= 0); fr_pair_list_free(&cert_vps); } } diff --git a/src/lib/unlang/io.c b/src/lib/unlang/io.c index 720ac5f14ae..dab74a45035 100644 --- a/src/lib/unlang/io.c +++ b/src/lib/unlang/io.c @@ -77,11 +77,12 @@ request_t *unlang_io_subrequest_alloc(request_t *parent, fr_dict_t const *namesp { request_t *child; - if (!detachable) { - child = request_alloc_fake(parent, namespace); - } else { - child = request_alloc_detachable(parent, namespace); - } + child = request_alloc(detachable ? NULL : parent, + (&(request_init_args_t){ + .parent = parent, + .namespace = namespace, + .detachable = detachable + })); if (!child) return NULL; /* @@ -118,5 +119,7 @@ request_t *unlang_io_subrequest_alloc(request_t *parent, fr_dict_t const *namesp */ child->async->process = unlang_io_process_interpret; + REQUEST_VERIFY(child); + return child; } diff --git a/src/lib/unlang/parallel.c b/src/lib/unlang/parallel.c index 9845bfaf52a..cea5ae0a133 100644 --- a/src/lib/unlang/parallel.c +++ b/src/lib/unlang/parallel.c @@ -67,13 +67,13 @@ static unlang_action_t unlang_parallel_child_done(UNUSED rlm_rcode_t *p_result, */ static unlang_action_t unlang_parallel_process(rlm_rcode_t *p_result, request_t *request) { - unlang_stack_t *stack = request->stack; - unlang_stack_frame_t *frame = &stack->frame[stack->depth]; - unlang_parallel_state_t *state = talloc_get_type_abort(frame->state, unlang_parallel_state_t); + unlang_stack_t *stack = request->stack; + unlang_stack_frame_t *frame = &stack->frame[stack->depth]; + unlang_parallel_state_t *state = talloc_get_type_abort(frame->state, unlang_parallel_state_t); - int i, priority; - rlm_rcode_t result; - unlang_parallel_child_state_t child_state = CHILD_DONE; /* hope that we're done */ + int i, priority; + rlm_rcode_t result; + unlang_parallel_child_state_t child_state = CHILD_DONE; /* hope that we're done */ request_t *child; /* @@ -111,13 +111,13 @@ static unlang_action_t unlang_parallel_process(rlm_rcode_t *p_result, request_t * contains state information for * the parent. */ - if ((fr_pair_list_copy(child->packet, + if ((fr_pair_list_copy(child->request_ctx, &child->request_pairs, &request->request_pairs) < 0) || - (fr_pair_list_copy(child->reply, + (fr_pair_list_copy(child->reply_ctx, &child->reply_pairs, &request->reply_pairs) < 0) || - (fr_pair_list_copy(child, + (fr_pair_list_copy(child->control_ctx, &child->control_pairs, &request->control_pairs) < 0)) { REDEBUG("failed copying lists to clone"); diff --git a/src/lib/unlang/subrequest.c b/src/lib/unlang/subrequest.c index 49e856e1dd6..6f1f8a99371 100644 --- a/src/lib/unlang/subrequest.c +++ b/src/lib/unlang/subrequest.c @@ -49,11 +49,11 @@ fr_dict_autoload_t subrequest_dict[] = { { NULL } }; -static fr_dict_attr_t const *attr_request_lifetime; +static fr_dict_attr_t const *request_attr_request_lifetime; extern fr_dict_attr_autoload_t subrequest_dict_attr[]; fr_dict_attr_autoload_t subrequest_dict_attr[] = { - { .out = &attr_request_lifetime, .name = "Request-Lifetime", .type = FR_TYPE_UINT32, .dict = &dict_freeradius }, + { .out = &request_attr_request_lifetime, .name = "Request-Lifetime", .type = FR_TYPE_UINT32, .dict = &dict_freeradius }, { NULL } }; @@ -208,6 +208,8 @@ static unlang_action_t unlang_subrequest_start(rlm_rcode_t *p_result, request_t state->session.unique_ptr, state->session.unique_int); + REQUEST_VERIFY(child); + RDEBUG2("Creating subrequest (%s)", child->name); log_request_pair_list(L_DBG_LVL_1, request, NULL, &child->request_pairs, NULL); @@ -259,7 +261,7 @@ static unlang_action_t unlang_subrequest_state_init(rlm_rcode_t *p_result, reque /* * Set the packet type. */ - MEM(vp = fr_pair_afrom_da(child->packet, gext->attr_packet_type)); + MEM(vp = fr_pair_afrom_da(child->request_ctx, gext->attr_packet_type)); if (gext->type_enum) { child->packet->code = vp->vp_uint32 = gext->type_enum->value->vb_uint32; @@ -310,12 +312,12 @@ static unlang_action_t unlang_subrequest_state_init(rlm_rcode_t *p_result, reque if (gext->src) { if (tmpl_is_list(gext->src)) { - if (tmpl_copy_pairs(child->packet, &child->request_pairs, request, gext->src) < -1) { + if (tmpl_copy_pairs(child->request_ctx, &child->request_pairs, request, gext->src) < -1) { RPEDEBUG("Failed copying source attributes into subrequest"); goto fail; } } else { - if (tmpl_copy_pair_children(child->packet, &child->request_pairs, request, gext->src) < -1) { + if (tmpl_copy_pair_children(child->request_ctx, &child->request_pairs, request, gext->src) < -1) { RPEDEBUG("Failed copying source attributes into subrequest"); goto fail; } @@ -373,7 +375,7 @@ int unlang_detached_child_init(request_t *request) /* * Set Request Lifetime */ - vp = fr_pair_find_by_da(&request->control_pairs, attr_request_lifetime); + vp = fr_pair_find_by_da(&request->control_pairs, request_attr_request_lifetime); if (!vp || (vp->vp_uint32 > 0)) { fr_time_delta_t when = 0; const fr_event_timer_t **ev_p; diff --git a/src/lib/util/acutest_helpers.h b/src/lib/util/acutest_helpers.h index 8c559a6c53a..31f3a293f31 100644 --- a/src/lib/util/acutest_helpers.h +++ b/src/lib/util/acutest_helpers.h @@ -35,6 +35,13 @@ do { \ TEST_MSG("Got length : %zu", (ssize_t)_our_got); \ } while(0) +#define TEST_CHECK_RET(_got, _exp) \ +do { \ + int _our_got = (_got); \ + TEST_CHECK(_exp == _our_got); \ + TEST_MSG("Expected ret : %"PRId64, (int64_t)_exp); \ + TEST_MSG("Got ret : %"PRId64, (int64_t)_our_got); \ +} while(0) #define TEST_CHECK_SLEN(_got, _exp) \ do { \ ssize_t _our_got = (_got); \ diff --git a/src/lib/util/pair.c b/src/lib/util/pair.c index b1e11ea2c5c..7d12e9a4b4f 100644 --- a/src/lib/util/pair.c +++ b/src/lib/util/pair.c @@ -107,7 +107,7 @@ fr_pair_t *fr_pair_alloc_null(TALLOC_CTX *ctx) fr_pair_t *vp; vp = talloc_zero(ctx, fr_pair_t); - if (!vp) { + if (unlikely(!vp)) { fr_strerror_const("Out of memory"); return NULL; } @@ -120,6 +120,58 @@ fr_pair_t *fr_pair_alloc_null(TALLOC_CTX *ctx) return vp; } +/** A special allocation function which disables child autofree + * + * This is intended to allocate root attributes for requests. + * These roots are special in that they do not necessarily own + * the child attributes and _MUST NOT_ free them when they + * themselves are freed. + * + * @param[in] ctx to allocate the pair root in. + * @param[in] da The root attribute. + * @return + * - A new root pair on success. + * - NULL on failure. + * @hidecallergraph + */ +fr_pair_t *fr_pair_root_afrom_da(TALLOC_CTX *ctx, fr_dict_attr_t const *da) +{ + fr_pair_t *vp; + + vp = talloc_zero(ctx, fr_pair_t); + if (unlikely(!vp)) { + fr_strerror_const("Out of memory"); + return NULL; + } + + vp->op = T_OP_EQ; + vp->type = VT_NONE; + + if (unlikely(da->flags.is_unknown)) { + fr_strerror_const("Root attribute cannot be unknown"); + return NULL; + } + + vp->da = da; + +#ifndef NDEBUG + switch (da->type) { + case FR_TYPE_GROUP: +#endif + fr_pair_list_init(&vp->children); + +#ifndef NDEBUG + break; + + default: + fr_strerror_const("Root must be a group type"); + return NULL; + } +#endif + + return vp; +} + /** Dynamically allocate a new attribute and assign a #fr_dict_attr_t * * @note Will duplicate any unknown attributes passed as the da. @@ -136,10 +188,7 @@ fr_pair_t *fr_pair_afrom_da(TALLOC_CTX *ctx, fr_dict_attr_t const *da) fr_pair_t *vp; vp = fr_pair_alloc_null(ctx); - if (!vp) { - fr_strerror_const("Out of memory"); - return NULL; - } + if (!vp) return NULL; /* * If we get passed an unknown da, we need to ensure that @@ -156,7 +205,16 @@ fr_pair_t *fr_pair_afrom_da(TALLOC_CTX *ctx, fr_dict_attr_t const *da) * Use the 'da' to initialize more fields. */ vp->da = da; - fr_value_box_init(&vp->data, da->type, da, false); + + switch (da->type) { + case FR_TYPE_STRUCTURAL: + fr_pair_list_init(&vp->children); + break; + + default: + fr_value_box_init(&vp->data, da->type, da, false); + break; + } return vp; } @@ -2270,17 +2328,26 @@ void fr_pair_verify(char const *file, int line, fr_pair_t const *vp) fr_table_str_by_value(fr_value_box_type_table, FR_TYPE_OCTETS, ""), fr_table_str_by_value(fr_value_box_type_table, vp->data.type, "")); } - } else if (vp->da->type != vp->data.type) { - char data_type_int[10], da_type_int[10]; + } else { + switch (vp->da->type) { + case FR_TYPE_VALUE: + if (vp->da->type != vp->data.type) { + char data_type_int[10], da_type_int[10]; - snprintf(data_type_int, sizeof(data_type_int), "%i", vp->data.type); - snprintf(da_type_int, sizeof(da_type_int), "%i", vp->da->type); + snprintf(data_type_int, sizeof(data_type_int), "%i", vp->data.type); + snprintf(da_type_int, sizeof(da_type_int), "%i", vp->da->type); - fr_fatal_assert_fail("CONSISTENCY CHECK FAILED %s[%u]: fr_pair_t attribute %p \"%s\" " - "data type (%s) does not match da type (%s)", - file, line, vp->da, vp->da->name, - fr_table_str_by_value(fr_value_box_type_table, vp->data.type, data_type_int), - fr_table_str_by_value(fr_value_box_type_table, vp->da->type, da_type_int)); + fr_fatal_assert_fail("CONSISTENCY CHECK FAILED %s[%u]: fr_pair_t attribute %p \"%s\" " + "data type (%s) does not match da type (%s)", + file, line, vp->da, vp->da->name, + fr_table_str_by_value(fr_value_box_type_table, vp->data.type, data_type_int), + fr_table_str_by_value(fr_value_box_type_table, vp->da->type, da_type_int)); + } + break; + + default: + break; + } } } diff --git a/src/lib/util/pair.h b/src/lib/util/pair.h index aeba215f379..3fbf09413c4 100644 --- a/src/lib/util/pair.h +++ b/src/lib/util/pair.h @@ -50,11 +50,6 @@ extern "C" { # define LIST_VERIFY(_x) #endif -#define request_pairs request_list -#define reply_pairs reply_list -#define control_pairs control_list -#define state_pairs state_list - /** The type of value a fr_pair_t contains * * This is used to add structure to nested fr_pair_ts and specifies what type of node it is (set, list, data). @@ -194,14 +189,12 @@ static inline bool fr_pair_list_empty(fr_pair_list_t const *list) } /* Allocation and management */ +fr_pair_list_t *fr_pair_list_alloc(TALLOC_CTX *ctx); + fr_pair_t *fr_pair_alloc_null(TALLOC_CTX *ctx); -fr_pair_list_t *fr_pair_list_alloc(TALLOC_CTX *ctx); +fr_pair_t *fr_pair_root_afrom_da(TALLOC_CTX *ctx, fr_dict_attr_t const *da) CC_HINT(nonnull(2)); -/** - * - * @hidecallergraph - */ fr_pair_t *fr_pair_afrom_da(TALLOC_CTX *ctx, fr_dict_attr_t const *da) CC_HINT(nonnull(2)); fr_pair_t *fr_pair_afrom_child_num(TALLOC_CTX *ctx, fr_dict_attr_t const *parent, unsigned int attr); diff --git a/src/modules/proto_arp/proto_arp.c b/src/modules/proto_arp/proto_arp.c index 5ac5ce63243..3bbbfa03a3d 100644 --- a/src/modules/proto_arp/proto_arp.c +++ b/src/modules/proto_arp/proto_arp.c @@ -75,7 +75,7 @@ static int mod_decode(UNUSED void const *instance, request_t *request, uint8_t * */ request->dict = dict_arp; - if (fr_arp_decode(request->packet, data, data_len, &request->request_pairs) < 0) { + if (fr_arp_decode(request->request_ctx, data, data_len, &request->request_pairs) < 0) { RPEDEBUG("Failed decoding packet"); return -1; } diff --git a/src/modules/proto_bfd/proto_bfd.c b/src/modules/proto_bfd/proto_bfd.c index c46d3c73889..049fd74de58 100644 --- a/src/modules/proto_bfd/proto_bfd.c +++ b/src/modules/proto_bfd/proto_bfd.c @@ -386,12 +386,6 @@ static const char *bfd_state[] = { static void bfd_request(bfd_state_t *session, request_t *request, fr_radius_packet_t *packet) { - memset(request, 0, sizeof(*request)); - fr_pair_list_init(&request->request_pairs); - fr_pair_list_init(&request->reply_pairs); - fr_pair_list_init(&request->control_pairs); - fr_pair_list_init(&request->state_pairs); - memset(packet, 0, sizeof(*packet)); request->packet = packet; @@ -403,16 +397,16 @@ static void bfd_request(bfd_state_t *session, request_t *request, fr_radius_pack static void bfd_trigger(bfd_state_t *session) { - fr_radius_packet_t packet; - request_t request; - char buffer[256]; + fr_radius_packet_t packet; + request_t *request = request_local_alloc(session, NULL); + char buffer[256]; snprintf(buffer, sizeof(buffer), "server.bfd.%s", bfd_state[session->session_state]); - bfd_request(session, &request, &packet); + bfd_request(session, request, &packet); - trigger_exec(&request, NULL, buffer, false, NULL); + trigger_exec(request, NULL, buffer, false, NULL); } @@ -1348,7 +1342,7 @@ static int bfd_process(bfd_state_t *session, bfd_packet_t *bfd) request_t *request; fr_radius_packet_t *packet, *reply; - request = request_alloc(session); + request = request_alloc(session, NULL); packet = fr_radius_packet_alloc(request, 0); reply = fr_radius_packet_alloc(request, 0); diff --git a/src/modules/proto_detail/proto_detail.c b/src/modules/proto_detail/proto_detail.c index 58dd957c38b..0720ebd15d4 100644 --- a/src/modules/proto_detail/proto_detail.c +++ b/src/modules/proto_detail/proto_detail.c @@ -304,7 +304,7 @@ static int mod_decode(void const *instance, request_t *request, uint8_t *const d timestamp = atoi((char const *) p); - vp = fr_pair_afrom_da(request->packet, attr_packet_original_timestamp); + vp = fr_pair_afrom_da(request->request_ctx, attr_packet_original_timestamp); if (vp) { vp->vp_date = ((fr_time_t) timestamp) * NSEC; vp->type = VT_DATA; @@ -327,7 +327,7 @@ static int mod_decode(void const *instance, request_t *request, uint8_t *const d * functions walking over the list. */ vp = NULL; - if ((fr_pair_list_afrom_str(request->packet, request->dict, (char const *) p, &vp) > 0) && vp) { + if ((fr_pair_list_afrom_str(request->request_ctx, request->dict, (char const *) p, &vp) > 0) && vp) { fr_cursor_append(&cursor, vp); } else { RWDEBUG("Ignoring line %d - :%s", lineno, p); diff --git a/src/modules/proto_dhcpv6/proto_dhcpv6_process.c b/src/modules/proto_dhcpv6/proto_dhcpv6_process.c index 60bfb7d1950..b68f5b2341b 100644 --- a/src/modules/proto_dhcpv6/proto_dhcpv6_process.c +++ b/src/modules/proto_dhcpv6/proto_dhcpv6_process.c @@ -118,27 +118,27 @@ static inline CC_HINT(always_inline) void dhcpv6_reply_initialise(request_t *req vp; vp = fr_cursor_next(&cursor)) { if (vp->da == attr_hop_count) { - fr_pair_add(&request->reply_pairs, fr_pair_copy(request->reply, vp)); + fr_pair_add(&request->reply_pairs, fr_pair_copy(request->reply_ctx, vp)); continue; } if (vp->da == attr_relay_link_address) { - fr_pair_add(&request->reply_pairs, fr_pair_copy(request->reply, vp)); + fr_pair_add(&request->reply_pairs, fr_pair_copy(request->reply_ctx, vp)); continue; } if (vp->da == attr_relay_peer_address) { - fr_pair_add(&request->reply_pairs, fr_pair_copy(request->reply, vp)); + fr_pair_add(&request->reply_pairs, fr_pair_copy(request->reply_ctx, vp)); continue; } if (vp->da == attr_interface_id) { - fr_pair_add(&request->reply_pairs, fr_pair_copy(request->reply, vp)); + fr_pair_add(&request->reply_pairs, fr_pair_copy(request->reply_ctx, vp)); continue; } } break; default: - fr_pair_list_copy_by_da(request->reply, &request->reply_pairs, &request->request_pairs, attr_transaction_id, 1); - fr_pair_list_copy_by_ancestor(request->reply, &request->reply_pairs, &request->request_pairs, attr_client_id, 0); + fr_pair_list_copy_by_da(request->reply_ctx, &request->reply_pairs, &request->request_pairs, attr_transaction_id, 1); + fr_pair_list_copy_by_ancestor(request->reply_ctx, &request->reply_pairs, &request->request_pairs, attr_client_id, 0); break; } } @@ -314,7 +314,7 @@ static unlang_action_t mod_process(rlm_rcode_t *p_result, UNUSED module_ctx_t co * Add reply->packet-type in case we're * being called via the `call {}` keyword. */ - MEM(fr_pair_update_by_da(request->reply, &reply_packet_type, + MEM(fr_pair_update_by_da(request->reply_ctx, &reply_packet_type, &request->reply_pairs, attr_packet_type) >= 0); reply_packet_type->vp_uint32 = request->reply->code; diff --git a/src/modules/proto_radius/proto_radius.c b/src/modules/proto_radius/proto_radius.c index 39b3eb53c7e..99d4798611c 100644 --- a/src/modules/proto_radius/proto_radius.c +++ b/src/modules/proto_radius/proto_radius.c @@ -221,7 +221,7 @@ static int mod_decode(void const *instance, request_t *request, uint8_t *const d * transport, via a call to fr_radius_ok(). */ fr_cursor_init(&cursor, &request->request_pairs); - if (fr_radius_decode(request->packet, request->packet->data, request->packet->data_len, + if (fr_radius_decode(request->request_ctx, request->packet->data, request->packet->data_len, NULL, client->secret, talloc_array_length(client->secret) - 1, &cursor) < 0) { RPEDEBUG("Failed decoding packet"); diff --git a/src/modules/proto_tacacs/proto_tacacs.c b/src/modules/proto_tacacs/proto_tacacs.c index 9c447f19339..131ee871db8 100644 --- a/src/modules/proto_tacacs/proto_tacacs.c +++ b/src/modules/proto_tacacs/proto_tacacs.c @@ -221,7 +221,7 @@ static int mod_decode(void const *instance, request_t *request, uint8_t *const d * transport, via a call to ??? */ fr_cursor_init(&cursor, &request->request_pairs); - if (fr_tacacs_decode(request->packet, request->packet->data, request->packet->data_len, + if (fr_tacacs_decode(request->request_ctx, request->packet->data, request->packet->data_len, NULL, client->secret, talloc_array_length(client->secret) - 1, &cursor) < 0) { RPEDEBUG("Failed decoding packet"); diff --git a/src/modules/proto_tacacs/proto_tacacs_acct.c b/src/modules/proto_tacacs/proto_tacacs_acct.c index b1c40289b24..6f0e6a97c88 100644 --- a/src/modules/proto_tacacs/proto_tacacs_acct.c +++ b/src/modules/proto_tacacs/proto_tacacs_acct.c @@ -148,7 +148,7 @@ static unlang_action_t mod_process(rlm_rcode_t *p_result, module_ctx_t const *mc memcpy(buffer, &request->async->listen, sizeof(request->async->listen)); memcpy(buffer + sizeof(request->async->listen), &pkt->session_id, sizeof(pkt->session_id)); - vp = fr_pair_afrom_da(request->packet, attr_tacacs_state); + vp = fr_pair_afrom_da(request->request_ctx, attr_tacacs_state); if (vp) { fr_pair_value_memdup(vp, buffer, sizeof(buffer), false); fr_pair_add(&request->request_pairs, vp); diff --git a/src/modules/proto_tacacs/proto_tacacs_auth.c b/src/modules/proto_tacacs/proto_tacacs_auth.c index b42d3e2f780..e472d3fe01d 100644 --- a/src/modules/proto_tacacs/proto_tacacs_auth.c +++ b/src/modules/proto_tacacs/proto_tacacs_auth.c @@ -191,7 +191,7 @@ static unlang_action_t mod_process(rlm_rcode_t *p_result, module_ctx_t const *mc memcpy(buffer, &request->async->listen, sizeof(request->async->listen)); memcpy(buffer + sizeof(request->async->listen), &pkt->session_id, sizeof(pkt->session_id)); - vp = fr_pair_afrom_da(request->packet, attr_tacacs_state); + vp = fr_pair_afrom_da(request->request_ctx, attr_tacacs_state); if (vp) { fr_pair_value_memdup(vp, buffer, sizeof(buffer), false); fr_pair_add(&request->request_pairs, vp); diff --git a/src/modules/proto_vmps/proto_vmps.c b/src/modules/proto_vmps/proto_vmps.c index 1c112fafa6b..c4f1f5a34f2 100644 --- a/src/modules/proto_vmps/proto_vmps.c +++ b/src/modules/proto_vmps/proto_vmps.c @@ -202,7 +202,7 @@ static int mod_decode(void const *instance, request_t *request, uint8_t *const d * transport, via a call to fr_vmps_ok(). */ fr_cursor_init(&cursor, &request->request_pairs); - if (fr_vmps_decode(packet, packet->data, packet->data_len, &cursor, &packet->code) < 0) { + if (fr_vmps_decode(request->request_ctx, packet->data, packet->data_len, &cursor, &packet->code) < 0) { RPEDEBUG("Failed decoding packet"); return -1; } diff --git a/src/modules/rlm_chap/rlm_chap.c b/src/modules/rlm_chap/rlm_chap.c index 65e50037fc9..1d7e9095189 100644 --- a/src/modules/rlm_chap/rlm_chap.c +++ b/src/modules/rlm_chap/rlm_chap.c @@ -144,7 +144,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authorize(rlm_rcode_t *p_result, mod if (!vp) { RDEBUG2("Creating &%s from request authenticator", attr_chap_challenge->name); - MEM(vp = fr_pair_afrom_da(request->packet, attr_chap_challenge)); + MEM(vp = fr_pair_afrom_da(request->request_ctx, attr_chap_challenge)); fr_pair_value_memdup(vp, request->packet->vector, sizeof(request->packet->vector), true); fr_pair_add(&request->request_pairs, vp); } diff --git a/src/modules/rlm_dhcpv4/rlm_dhcpv4.c b/src/modules/rlm_dhcpv4/rlm_dhcpv4.c index a67c92eb622..bbe5e865eed 100644 --- a/src/modules/rlm_dhcpv4/rlm_dhcpv4.c +++ b/src/modules/rlm_dhcpv4/rlm_dhcpv4.c @@ -100,7 +100,7 @@ static xlat_action_t dhcpv4_decode_xlat(TALLOC_CTX *ctx, fr_cursor_t *out, * Loop over all the options data */ while (p < end) { - len = fr_dhcpv4_decode_option(request->packet, &options_cursor, dict_dhcpv4, + len = fr_dhcpv4_decode_option(request->request_ctx, &options_cursor, dict_dhcpv4, p, end - p, NULL); if (len <= 0) { RPERROR("DHCP option decoding failed"); @@ -120,7 +120,7 @@ static xlat_action_t dhcpv4_decode_xlat(TALLOC_CTX *ctx, fr_cursor_t *out, decoded++; } - fr_pair_list_move(&(request->request_pairs), &head); + fr_pair_list_move(&request->request_pairs, &head); /* Free any unmoved pairs */ fr_pair_list_free(&head); diff --git a/src/modules/rlm_eap/rlm_eap.c b/src/modules/rlm_eap/rlm_eap.c index 5c6082f686a..4163f2e9452 100644 --- a/src/modules/rlm_eap/rlm_eap.c +++ b/src/modules/rlm_eap/rlm_eap.c @@ -626,7 +626,7 @@ static unlang_action_t eap_method_select(rlm_rcode_t *p_result, module_ctx_t con RETURN_MODULE_FAIL; } - if (fr_pair_list_copy(eap_session->subrequest->packet, + if (fr_pair_list_copy(eap_session->subrequest->request_ctx, &eap_session->subrequest->request_pairs, &request->request_pairs) < 0) goto list_copy_fail; } @@ -907,7 +907,7 @@ static unlang_action_t mod_post_auth(rlm_rcode_t *p_result, module_ctx_t const * */ vp = fr_pair_find_by_da(&request->reply_pairs, attr_user_name); if (!vp) { - vp = fr_pair_copy(request->reply, username); + vp = fr_pair_copy(request->reply_ctx, username); fr_pair_add(&request->reply_pairs, vp); } diff --git a/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c b/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c index 2a65a85343d..540cf06a17c 100644 --- a/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c +++ b/src/modules/rlm_eap/types/rlm_eap_fast/eap_fast.c @@ -580,7 +580,7 @@ static FR_CODE eap_fast_eap_payload(request_t *request, eap_session_t *eap_sessi /* * Allocate a fake request_t structure. */ - fake = request_alloc_fake(request, NULL); + fake = request_alloc(request, &(request_init_args_t){ .parent = request }); fr_assert(!fake->request_pairs); t = talloc_get_type_abort(tls_session->opaque, eap_fast_tunnel_t); @@ -589,7 +589,7 @@ static FR_CODE eap_fast_eap_payload(request_t *request, eap_session_t *eap_sessi * Add the tunneled attributes to the fake request. */ - MEM(fake->request_pairs = vp = fr_pair_afrom_da(fake->packet, attr_eap_message)); + MEM(fake->request_pairs = vp = fr_pair_afrom_da(fake->request_ctx, attr_eap_message)); fr_pair_value_memdup(fake->request_pairs, tlv_eap_payload->vp_octets, tlv_eap_payload->vp_length, false); RDEBUG2("Got tunneled request"); @@ -598,7 +598,7 @@ static FR_CODE eap_fast_eap_payload(request_t *request, eap_session_t *eap_sessi /* * Tell the request that it's a fake one. */ - MEM(fr_pair_add_by_da(fake->packet, &vp, &fake->request_pairs, attr_freeradius_proxied_to) >= 0); + MEM(fr_pair_add_by_da(fake->request_ctx, &vp, &fake->request_pairs, attr_freeradius_proxied_to) >= 0); fr_pair_value_from_str(vp, "127.0.0.1", sizeof("127.0.0.1"), '\0', false); /* @@ -631,7 +631,7 @@ static FR_CODE eap_fast_eap_payload(request_t *request, eap_session_t *eap_sessi } /* else there WAS a t->username */ if (t->username) { - vp = fr_pair_copy(fake->packet, t->username); + vp = fr_pair_copy(fake->request_ctx, t->username); fr_pair_add(&fake->request_pairs, vp); } @@ -680,8 +680,8 @@ static FR_CODE eap_fast_eap_payload(request_t *request, eap_session_t *eap_sessi /* * Tell the original request that it's going to be proxied. */ - fr_pair_list_copy_by_da(request, &request->control_pairs, - &fake->control, attr_proxy_to_realm, 0); + fr_pair_list_copy_by_da(request->control_ctx, &request->control_pairs, + &fake->control_pairs, attr_proxy_to_realm, 0); /* * Seed the proxy packet with the tunneled request. @@ -691,7 +691,7 @@ static FR_CODE eap_fast_eap_payload(request_t *request, eap_session_t *eap_sessi /* * FIXME: Actually proxy stuff */ - request->proxy = request_alloc_fake(request, NULL); + request->proxy = request_alloc(request, &(request_init_args_t){ .parent = request }); request->proxy->packet = talloc_steal(request->proxy, fake->packet); memset(&request->proxy->packet->src_ipaddr, 0, diff --git a/src/modules/rlm_eap/types/rlm_eap_mschapv2/rlm_eap_mschapv2.c b/src/modules/rlm_eap/types/rlm_eap_mschapv2/rlm_eap_mschapv2.c index 7c69f407cda..6c903a37ba8 100644 --- a/src/modules/rlm_eap/types/rlm_eap_mschapv2/rlm_eap_mschapv2.c +++ b/src/modules/rlm_eap/types/rlm_eap_mschapv2/rlm_eap_mschapv2.c @@ -558,7 +558,7 @@ failure: if (data->mppe_keys) { RDEBUG2("Adding stored attributes to parent"); log_request_pair_list(L_DBG_LVL_2, request, NULL, &data->mppe_keys, "&parent.reply."); - MEM(fr_pair_list_copy(parent->reply, &parent->reply_pairs, &data->mppe_keys) >= 0); + MEM(fr_pair_list_copy(parent->reply_ctx, &parent->reply_pairs, &data->mppe_keys) >= 0); } else { RDEBUG2("No stored attributes to copy to parent"); } @@ -572,7 +572,7 @@ failure: */ request->options &= ~RAD_REQUEST_OPTION_PROXY_EAP; #endif - MEM(fr_pair_list_copy(parent->reply, &parent->reply_pairs, &data->reply) >= 0); + MEM(fr_pair_list_copy(parent->reply_ctx, &parent->reply_pairs, &data->reply) >= 0); RETURN_MODULE_OK; } REDEBUG("Sent SUCCESS expecting SUCCESS (or ACK) but got %d", ccode); diff --git a/src/modules/rlm_eap/types/rlm_eap_peap/peap.c b/src/modules/rlm_eap/types/rlm_eap_peap/peap.c index e5dba6f63e2..9ce5b25f670 100644 --- a/src/modules/rlm_eap/types/rlm_eap_peap/peap.c +++ b/src/modules/rlm_eap/types/rlm_eap_peap/peap.c @@ -156,8 +156,7 @@ static int eap_peap_soh(request_t *request,fr_tls_session_t *tls_session) return 1; } -static void eap_peap_soh_verify(request_t *request, fr_radius_packet_t *packet, - uint8_t const *data, unsigned int data_len) { +static void eap_peap_soh_verify(request_t *request, uint8_t const *data, unsigned int data_len) { fr_pair_t *vp; uint8_t eap_method_base; @@ -165,7 +164,7 @@ static void eap_peap_soh_verify(request_t *request, fr_radius_packet_t *packet, uint32_t eap_method; int rv; - MEM(vp = fr_pair_afrom_da(packet, attr_soh_supported)); + MEM(vp = fr_pair_afrom_da(request->request_ctx, attr_soh_supported)); vp->vp_bool = false; fr_pair_add(&request->request_pairs, vp); @@ -263,20 +262,18 @@ static int eap_peap_verify(request_t *request, peap_tunnel_t *peap_tunnel, /* * Convert a pseudo-EAP packet to a list of fr_pair_t's. */ -static fr_pair_t *eap_peap_inner_to_pairs(UNUSED request_t *request, fr_radius_packet_t *packet, - eap_round_t *eap_round, - uint8_t const *data, size_t data_len) +static void eap_peap_inner_to_pairs(TALLOC_CTX *ctx, fr_pair_list_t *pairs, + eap_round_t *eap_round, + uint8_t const *data, size_t data_len) { size_t total; uint8_t *p; fr_pair_t *vp = NULL; - fr_pair_list_t head; fr_cursor_t cursor; - fr_pair_list_init(&head); - if (data_len > 65535) return NULL; /* paranoia */ + if (data_len > 65535) return; /* paranoia */ - MEM(vp = fr_pair_afrom_da(packet, attr_eap_message)); + MEM(vp = fr_pair_afrom_da(ctx, attr_eap_message)); total = data_len; if (total > 249) total = 249; @@ -290,18 +287,16 @@ static fr_pair_t *eap_peap_inner_to_pairs(UNUSED request_t *request, fr_radius_p p[3] = (data_len + EAP_HEADER_LEN) & 0xff; memcpy(p + EAP_HEADER_LEN, data, total); - fr_cursor_init(&cursor, &head); + fr_cursor_init(&cursor, pairs); fr_cursor_append(&cursor, vp); while (total < data_len) { - MEM(vp = fr_pair_afrom_da(packet, attr_eap_message)); + MEM(vp = fr_pair_afrom_da(ctx, attr_eap_message)); fr_pair_value_memdup(vp, data + total, (data_len - total), false); total += vp->vp_length; fr_cursor_append(&cursor, vp); } - - return head; } @@ -556,9 +551,9 @@ unlang_action_t eap_peap_process(rlm_rcode_t *p_result, request_t *request, break; case PEAP_STATUS_WAIT_FOR_SOH_RESPONSE: - fake = request_alloc_fake(request, NULL); + fake = request_alloc(request, &(request_init_args_t){ .parent = request }); fr_assert(!fake->request_pairs); - eap_peap_soh_verify(fake, fake->packet, data, data_len); + eap_peap_soh_verify(fake, data, data_len); setup_fake_request(request, fake, t); if (t->soh_virtual_server) fake->server_cs = virtual_server_find(t->soh_virtual_server); @@ -657,7 +652,7 @@ unlang_action_t eap_peap_process(rlm_rcode_t *p_result, request_t *request, goto finish; } - fake = request_alloc_fake(request, NULL); + fake = request_alloc(request, &(request_init_args_t){ .parent = request }); fr_assert(!fake->request_pairs); switch (t->status) { @@ -676,7 +671,7 @@ unlang_action_t eap_peap_process(rlm_rcode_t *p_result, request_t *request, len = t->username->vp_length + EAP_HEADER_LEN + 1; t->status = PEAP_STATUS_PHASE2; - MEM(vp = fr_pair_afrom_da(fake->packet, attr_eap_message)); + MEM(vp = fr_pair_afrom_da(fake->request_ctx, attr_eap_message)); MEM(fr_pair_value_mem_alloc(vp, &q, len, false) == 0); q[0] = FR_EAP_CODE_RESPONSE; q[1] = eap_round->response->id; @@ -690,8 +685,8 @@ unlang_action_t eap_peap_process(rlm_rcode_t *p_result, request_t *request, break; case PEAP_STATUS_PHASE2: - fake->request_pairs = eap_peap_inner_to_pairs(request, fake->packet, - eap_round, data, data_len); + eap_peap_inner_to_pairs(fake->request_ctx, &fake->request_pairs, + eap_round, data, data_len); if (!fake->request_pairs) { talloc_free(fake); RDEBUG2("Unable to convert tunneled EAP packet to internal server data structures"); @@ -744,7 +739,7 @@ unlang_action_t eap_peap_process(rlm_rcode_t *p_result, request_t *request, REDEBUG("Unknown RADIUS packet type %d: rejecting tunneled user", fake->reply->code); rcode = RLM_MODULE_REJECT; } else { - rcode = process_reply(eap_session, tls_session, request, fake->reply, &fake->reply_list); + rcode = process_reply(eap_session, tls_session, request, fake->reply, &fake->reply_pairs); } finish: @@ -760,11 +755,11 @@ static int CC_HINT(nonnull) setup_fake_request(request_t *request, request_t *fa /* * Tell the request that it's a fake one. */ - MEM(fr_pair_add_by_da(fake->packet, &vp, &fake->request_pairs, attr_freeradius_proxied_to) >= 0); + MEM(fr_pair_add_by_da(fake->request_ctx, &vp, &fake->request_pairs, attr_freeradius_proxied_to) >= 0); fr_pair_value_from_str(vp, "127.0.0.1", sizeof("127.0.0.1"), '\0', false); if (t->username) { - vp = fr_pair_copy(fake->packet, t->username); + vp = fr_pair_copy(fake->request_ctx, t->username); fr_pair_add(&fake->request_pairs, vp); RDEBUG2("Setting &request.User-Name from tunneled (inner) identity \"%s\"", vp->vp_strvalue); diff --git a/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c b/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c index 4a79e0ee178..81c58a04ad7 100644 --- a/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c +++ b/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c @@ -638,7 +638,7 @@ FR_CODE eap_ttls_process(request_t *request, eap_session_t *eap_session, fr_tls_ * Add the tunneled attributes to the request request. */ fr_cursor_init(&cursor, &request->request_pairs); - if (eap_ttls_decode_pair(request->packet, &cursor, fr_dict_root(fr_dict_internal()), + if (eap_ttls_decode_pair(request->request_ctx, &cursor, fr_dict_root(fr_dict_internal()), data, data_len, tls_session->ssl) < 0) { RPEDEBUG("Decoding TTLS TLVs failed"); code = FR_CODE_ACCESS_REJECT; @@ -686,7 +686,7 @@ FR_CODE eap_ttls_process(request_t *request, eap_session_t *eap_session, fr_tls_ } /* else there WAS a t->username */ if (t->username) { - vp = fr_pair_copy(request->packet, t->username); + vp = fr_pair_copy(request->request_ctx, t->username); fr_pair_add(&request->request_pairs, vp); } } /* else the request ALREADY had a User-Name */ @@ -712,7 +712,7 @@ FR_CODE eap_ttls_process(request_t *request, eap_session_t *eap_session, fr_tls_ if (req->response) { RDEBUG2("sending chbind response"); fr_pair_add(&request->reply_pairs, - eap_chbind_packet2vp(request->reply, req->response)); + eap_chbind_packet2vp(request->reply_ctx, req->response)); } else { RDEBUG2("no chbind response"); } diff --git a/src/modules/rlm_files/rlm_files.c b/src/modules/rlm_files/rlm_files.c index 41fc21ccaf0..0bb590a6e6f 100644 --- a/src/modules/rlm_files/rlm_files.c +++ b/src/modules/rlm_files/rlm_files.c @@ -386,7 +386,7 @@ static unlang_action_t file_common(rlm_rcode_t *p_result, rlm_files_t const *ins for (map = fr_cursor_init(&cursor, &pl->check); map; map = fr_cursor_next(&cursor)) { - if (map_to_vp(request, &vp, request, map, NULL) < 0) { + if (map_to_vp(request->control_ctx, &vp, request, map, NULL) < 0) { fr_pair_list_free(&list); RPWARN("Failed parsing map for check item, skipping entry"); break; @@ -429,7 +429,7 @@ static unlang_action_t file_common(rlm_rcode_t *p_result, rlm_files_t const *ins map = fr_cursor_next(&cursor)) { if (map->op == T_OP_CMP_FALSE) continue; - if (map_to_vp(request->reply, &vp, request, map, NULL) < 0) { + if (map_to_vp(request->reply_ctx, &vp, request, map, NULL) < 0) { RPWARN("Failed parsing map for reply item %s, skipping it", map->rhs->name); break; } diff --git a/src/modules/rlm_isc_dhcp/rlm_isc_dhcp.c b/src/modules/rlm_isc_dhcp/rlm_isc_dhcp.c index e18e111b7d6..592a949279c 100644 --- a/src/modules/rlm_isc_dhcp/rlm_isc_dhcp.c +++ b/src/modules/rlm_isc_dhcp/rlm_isc_dhcp.c @@ -1780,7 +1780,7 @@ static int apply_fixed_ip(rlm_isc_dhcp_t const *inst, request_t *request) if (info->cmd->type != ISC_FIXED_ADDRESS) continue; - MEM(vp = fr_pair_afrom_da(request->reply_pairs, attr_your_ip_address)); + MEM(vp = fr_pair_afrom_da(request->reply_ctx, attr_your_ip_address)); ret = fr_value_box_copy(vp, &(vp->data), info->argv[0]); if (ret < 0) return ret; @@ -1905,7 +1905,7 @@ recurse: while (vp) { fr_pair_t *next, *copy; - copy = fr_pair_copy(request->reply, vp); + copy = fr_pair_copy(request->reply_ctx, vp); if (!copy) return -1; fr_cursor_append(&reply_cursor, copy); diff --git a/src/modules/rlm_lua/lua.c b/src/modules/rlm_lua/lua.c index eaff09e3762..f9b038c6f10 100644 --- a/src/modules/rlm_lua/lua.c +++ b/src/modules/rlm_lua/lua.c @@ -194,12 +194,12 @@ DIAG_ON(type-limits) * - 0 on success. * - -1 on failure. */ -static int fr_lua_unmarshall(fr_pair_t **out, +static int fr_lua_unmarshall(TALLOC_CTX *ctx, fr_pair_t **out, UNUSED rlm_lua_t const *inst, request_t *request, lua_State *L, fr_dict_attr_t const *da) { fr_pair_t *vp; - MEM(vp = fr_pair_afrom_da(request, da)); + MEM(vp = fr_pair_afrom_da(ctx, da)); switch (lua_type(L, -1)) { case LUA_TNUMBER: { @@ -390,7 +390,7 @@ static int _lua_pair_set(lua_State *L) return 0; } - if (fr_lua_unmarshall(&new, inst, request, L, da) < 0) return -1; + if (fr_lua_unmarshall(request->request_ctx, &new, inst, request, L, da) < 0) return -1; /* * If there was already a VP at that index we replace it diff --git a/src/modules/rlm_mruby/rlm_mruby.c b/src/modules/rlm_mruby/rlm_mruby.c index d89819fca2f..4b86f7cb44b 100644 --- a/src/modules/rlm_mruby/rlm_mruby.c +++ b/src/modules/rlm_mruby/rlm_mruby.c @@ -408,7 +408,7 @@ static unlang_action_t CC_HINT(nonnull) do_mruby(rlm_rcode_t *p_result, request_ mruby_set_vps(request, mrb, mruby_request, "@request", &request->request_pairs); mruby_set_vps(request, mrb, mruby_request, "@reply", &request->reply_pairs); mruby_set_vps(request, mrb, mruby_request, "@control", &request->control_pairs); - mruby_set_vps(request, mrb, mruby_request, "@session_state", &request->state_pairs); + mruby_set_vps(request, mrb, mruby_request, "@session_state", &request->session_state_pairs); DIAG_OFF(class-varargs) mruby_result = mrb_funcall(mrb, mrb_obj_value(inst->mruby_module), function_name, 1, mruby_request); @@ -449,8 +449,8 @@ DIAG_ON(class-varargs) RETURN_MODULE_FAIL; } - add_vp_tuple(request->reply, request, &request->reply_pairs, mrb, mrb_ary_entry(mruby_result, 1), function_name); - add_vp_tuple(request, request, &request->control_pairs, mrb, mrb_ary_entry(mruby_result, 2), function_name); + add_vp_tuple(request->reply_ctx, request, &request->reply_pairs, mrb, mrb_ary_entry(mruby_result, 1), function_name); + add_vp_tuple(request->control_ctx, request, &request->control_pairs, mrb, mrb_ary_entry(mruby_result, 2), function_name); RETURN_MODULE_RCODE((rlm_rcode_t)mrb_int(mrb, mrb_ary_entry(mruby_result, 0))); default: diff --git a/src/modules/rlm_mschap/rlm_mschap.c b/src/modules/rlm_mschap/rlm_mschap.c index 335ea851e29..7a373b4a6ec 100644 --- a/src/modules/rlm_mschap/rlm_mschap.c +++ b/src/modules/rlm_mschap/rlm_mschap.c @@ -1543,7 +1543,7 @@ found_password: int ret; fr_pair_t *nt_password; - MEM(nt_password = fr_pair_afrom_da(request, attr_nt_password)); + MEM(nt_password = fr_pair_afrom_da(request->request_ctx, attr_nt_password)); MEM(fr_pair_value_mem_alloc(nt_password, &p, NT_DIGEST_LENGTH, false) == 0); ret = mschap_nt_password_hash(p, password->vp_strvalue); diff --git a/src/modules/rlm_passwd/rlm_passwd.c b/src/modules/rlm_passwd/rlm_passwd.c index 3529ce6d80a..6ff322ece97 100644 --- a/src/modules/rlm_passwd/rlm_passwd.c +++ b/src/modules/rlm_passwd/rlm_passwd.c @@ -564,9 +564,9 @@ static unlang_action_t CC_HINT(nonnull) mod_passwd_map(rlm_rcode_t *p_result, mo if (!pw) continue; do { - result_add(request, inst, request, &request->control_pairs, pw, 0, "config"); - result_add(request->reply, inst, request, &request->reply_pairs, pw, 1, "reply_items"); - result_add(request->packet, inst, request, &request->request_pairs, pw, 2, "request_items"); + result_add(request->control_ctx, inst, request, &request->control_pairs, pw, 0, "config"); + result_add(request->reply_ctx, inst, request, &request->reply_pairs, pw, 1, "reply_items"); + result_add(request->request_ctx, inst, request, &request->request_pairs, pw, 2, "request_items"); } while ((pw = get_next(buffer, inst->ht, &last_found))); found++; diff --git a/src/modules/rlm_perl/rlm_perl.c b/src/modules/rlm_perl/rlm_perl.c index 6b5b94a1d1a..b206bd1fbbd 100644 --- a/src/modules/rlm_perl/rlm_perl.c +++ b/src/modules/rlm_perl/rlm_perl.c @@ -868,10 +868,10 @@ static unlang_action_t do_perl(rlm_rcode_t *p_result, void *instance, request_t rad_request_hv = get_hv("RAD_REQUEST", 1); rad_state_hv = get_hv("RAD_STATE", 1); - perl_store_vps(request->packet, request, &request->request_pairs, rad_request_hv, "RAD_REQUEST", "request"); - perl_store_vps(request->reply, request, &request->reply_pairs, rad_reply_hv, "RAD_REPLY", "reply"); - perl_store_vps(request, request, &request->control_pairs, rad_config_hv, "RAD_CONFIG", "control"); - perl_store_vps(request->state_ctx, request, &request->state_pairs, rad_state_hv, "RAD_STATE", "session-state"); + perl_store_vps(request->request_ctx, request, &request->request_pairs, rad_request_hv, "RAD_REQUEST", "request"); + perl_store_vps(request->reply_ctx, request, &request->reply_pairs, rad_reply_hv, "RAD_REPLY", "reply"); + perl_store_vps(request->control_ctx, request, &request->control_pairs, rad_config_hv, "RAD_CONFIG", "control"); + perl_store_vps(request->session_state_ctx, request, &request->session_state_pairs, rad_state_hv, "RAD_STATE", "session-state"); /* * Store pointer to request structure globally so radiusd::xlat works @@ -911,27 +911,27 @@ static unlang_action_t do_perl(rlm_rcode_t *p_result, void *instance, request_t LEAVE; vp = NULL; - if ((get_hv_content(request->packet, request, rad_request_hv, &vp, "RAD_REQUEST", "request")) == 0) { + if ((get_hv_content(request->request_ctx, request, rad_request_hv, &vp, "RAD_REQUEST", "request")) == 0) { fr_pair_list_free(&request->request_pairs); request->request_pairs = vp; vp = NULL; } - if ((get_hv_content(request->reply, request, rad_reply_hv, &vp, "RAD_REPLY", "reply")) == 0) { + if ((get_hv_content(request->reply_ctx, request, rad_reply_hv, &vp, "RAD_REPLY", "reply")) == 0) { fr_pair_list_free(&request->reply_pairs); request->reply_pairs = vp; vp = NULL; } - if ((get_hv_content(request, request, rad_config_hv, &vp, "RAD_CONFIG", "control")) == 0) { + if ((get_hv_content(request->control_ctx, request, rad_config_hv, &vp, "RAD_CONFIG", "control")) == 0) { fr_pair_list_free(&request->control_pairs); request->control_pairs = vp; vp = NULL; } - if ((get_hv_content(request->state_ctx, request, rad_state_hv, &vp, "RAD_STATE", "session-state")) == 0) { - fr_pair_list_free(&request->state_pairs); - request->state_pairs = vp; + if ((get_hv_content(request->session_state_ctx, request, rad_state_hv, &vp, "RAD_STATE", "session-state")) == 0) { + fr_pair_list_free(&request->session_state_pairs); + request->session_state_pairs = vp; vp = NULL; } } diff --git a/src/modules/rlm_python/rlm_python.c b/src/modules/rlm_python/rlm_python.c index 4ae82a43809..ee7a39cde2f 100644 --- a/src/modules/rlm_python/rlm_python.c +++ b/src/modules/rlm_python/rlm_python.c @@ -350,7 +350,7 @@ static void mod_vptuple(TALLOC_CTX *ctx, rlm_python_t const *inst, request_t *re fr_table_str_by_value(fr_tokens_table, op, "="), s2); } - radius_pairmove(current, vps, &vp, false); + radius_pairmove(request, vps, &vp, false); } } @@ -564,10 +564,10 @@ static unlang_action_t do_python_single(rlm_rcode_t *p_result, /* Now have the return value */ rcode = PyLong_AsLong(p_tuple_int); /* Reply item tuple */ - mod_vptuple(request->reply, inst, request, &request->reply_pairs, + mod_vptuple(request->reply_ctx, inst, request, &request->reply_pairs, PyTuple_GET_ITEM(p_ret, 1), funcname, "reply"); /* Config item tuple */ - mod_vptuple(request, inst, request, &request->control_pairs, + mod_vptuple(request->control_ctx, inst, request, &request->control_pairs, PyTuple_GET_ITEM(p_ret, 2), funcname, "config"); } else if (PyNumber_Check(p_ret)) { diff --git a/src/modules/rlm_radius/rlm_radius_udp.c b/src/modules/rlm_radius/rlm_radius_udp.c index 28480aeca64..935eb11d035 100644 --- a/src/modules/rlm_radius/rlm_radius_udp.c +++ b/src/modules/rlm_radius/rlm_radius_udp.c @@ -359,7 +359,7 @@ static void CC_HINT(nonnull) status_check_alloc(fr_event_list_t *el, udp_handle_ * head before the module destructor * runs. */ - request = request_local_alloc(u); + request = request_local_alloc(u, NULL); request->async = talloc_zero(request, fr_async_t); talloc_const_free(request->name); request->name = talloc_strdup(request, h->module_name); @@ -2375,7 +2375,7 @@ static void request_demux(fr_trunk_connection_t *tconn, fr_connection_t *conn, U /* * Validate and decode the incoming packet */ - reason = decode(request->reply, &reply, &code, h, request, u, rr->vector, h->buffer, (size_t)slen); + reason = decode(request->reply_ctx, &reply, &code, h, request, u, rr->vector, h->buffer, (size_t)slen); if (reason != DECODE_FAIL_NONE) { RWDEBUG("Ignoring invalid response"); continue; @@ -2434,7 +2434,7 @@ static void request_demux(fr_trunk_connection_t *tconn, fr_connection_t *conn, U vp = fr_pair_find_by_da(&request->reply_pairs, attr_packet_type); if (!vp) { - MEM(vp = fr_pair_afrom_da(request->reply, attr_packet_type)); + MEM(vp = fr_pair_afrom_da(request->reply_ctx, attr_packet_type)); vp->vp_uint32 = FR_CODE_ACCESS_CHALLENGE; fr_pair_add(&request->reply_pairs, vp); } @@ -2457,7 +2457,7 @@ static void request_demux(fr_trunk_connection_t *tconn, fr_connection_t *conn, U fr_pair_delete_by_da(&reply, attr_message_authenticator); - MEM(vp = fr_pair_afrom_da(request->reply, attr_message_authenticator)); + MEM(vp = fr_pair_afrom_da(request->reply_ctx, attr_message_authenticator)); (void) fr_pair_value_memdup(vp, (uint8_t const *) "", 1, false); fr_pair_add(&request->reply_pairs, vp); } diff --git a/src/modules/rlm_redis_ippool/rlm_redis_ippool_tool.c b/src/modules/rlm_redis_ippool/rlm_redis_ippool_tool.c index 9fb223799d0..e627ca4fd05 100644 --- a/src/modules/rlm_redis_ippool/rlm_redis_ippool_tool.c +++ b/src/modules/rlm_redis_ippool/rlm_redis_ippool_tool.c @@ -324,7 +324,7 @@ static int driver_do_lease(void *out, void *instance, ippool_tool_operation_t co unsigned int pipelined = 0; - request = request_alloc(inst); + request = request_alloc(inst, NULL); while (more) { fr_ipaddr_t acked = ipaddr; /* Record our progress */ size_t reply_cnt = 0; @@ -728,7 +728,7 @@ static ssize_t driver_get_pools(TALLOC_CTX *ctx, uint8_t **out[], void *instance request_t *request; uint8_t **result; - request = request_alloc(inst); + request = request_alloc(inst, NULL); IPPOOL_BUILD_KEY(key, key_p, "*}:pool", 1); @@ -919,7 +919,7 @@ static int driver_get_stats(ippool_tool_stats_t *out, void *instance, uint8_t co #define STATS_COMMANDS_TOTAL 8 - request = request_alloc(inst); + request = request_alloc(inst, NULL); IPPOOL_BUILD_KEY(key, key_p, key_prefix, key_prefix_len); diff --git a/src/modules/rlm_sigtran/client.c b/src/modules/rlm_sigtran/client.c index 65ca34e3269..47b91f2f094 100644 --- a/src/modules/rlm_sigtran/client.c +++ b/src/modules/rlm_sigtran/client.c @@ -313,19 +313,19 @@ static unlang_action_t sigtran_client_map_resume(rlm_rcode_t *p_result, UNUSED m RDEBUG2("SIM auth vector %i", i); RINDENT(); - MEM(vp = fr_pair_afrom_da(request, attr_eap_aka_sim_rand)); + MEM(vp = fr_pair_afrom_da(request->control_ctx, attr_eap_aka_sim_rand)); MEM(fr_pair_value_memdup_buffer(vp, vec->sim.rand, true) == 0); TALLOC_FREE(vec->sim.rand); RDEBUG2("&control.%pP", vp); fr_cursor_append(&cursor, vp); - MEM(vp = fr_pair_afrom_da(request, attr_eap_aka_sim_sres)); + MEM(vp = fr_pair_afrom_da(request->control_ctx, attr_eap_aka_sim_sres)); MEM(fr_pair_value_memdup_buffer(vp, vec->sim.sres, true) == 0); TALLOC_FREE(vec->sim.sres); RDEBUG2("&control.%pP", vp); fr_cursor_append(&cursor, vp); - MEM(vp = fr_pair_afrom_da(request, attr_eap_aka_sim_kc)); + MEM(vp = fr_pair_afrom_da(request->control_ctx, attr_eap_aka_sim_kc)); MEM(fr_pair_value_memdup_buffer(vp, vec->sim.kc, true) == 0); TALLOC_FREE(vec->sim.kc); RDEBUG2("&control.%pP", vp); @@ -344,31 +344,31 @@ static unlang_action_t sigtran_client_map_resume(rlm_rcode_t *p_result, UNUSED m RDEBUG2("UMTS auth vector %i", i); RINDENT(); - MEM(vp = fr_pair_afrom_da(request, attr_eap_aka_sim_rand)); + MEM(vp = fr_pair_afrom_da(request->control_ctx, attr_eap_aka_sim_rand)); MEM(fr_pair_value_memdup_buffer(vp, vec->umts.rand, true) == 0); TALLOC_FREE(vec->umts.rand); RDEBUG2("&control.%pP", vp); fr_cursor_append(&cursor, vp); - MEM(vp = fr_pair_afrom_da(request, attr_eap_aka_sim_xres)); + MEM(vp = fr_pair_afrom_da(request->control_ctx, attr_eap_aka_sim_xres)); MEM(fr_pair_value_memdup_buffer(vp, vec->umts.xres, true) == 0); TALLOC_FREE(vec->umts.xres); RDEBUG2("&control.%pP", vp); fr_cursor_append(&cursor, vp); - MEM(vp = fr_pair_afrom_da(request, attr_eap_aka_sim_ck)); + MEM(vp = fr_pair_afrom_da(request->control_ctx, attr_eap_aka_sim_ck)); MEM(fr_pair_value_memdup_buffer(vp, vec->umts.ck, true) == 0); TALLOC_FREE(vec->umts.ck); RDEBUG2("&control.%pP", vp); fr_cursor_append(&cursor, vp); - MEM(vp = fr_pair_afrom_da(request, attr_eap_aka_sim_ik)); + MEM(vp = fr_pair_afrom_da(request->control_ctx, attr_eap_aka_sim_ik)); MEM(fr_pair_value_memdup_buffer(vp, vec->umts.ik, true) == 0); TALLOC_FREE(vec->umts.ik); RDEBUG2("&control.%pP", vp); fr_cursor_append(&cursor, vp); - MEM(vp = fr_pair_afrom_da(request, attr_eap_aka_sim_autn)); + MEM(vp = fr_pair_afrom_da(request->control_ctx, attr_eap_aka_sim_autn)); MEM(fr_pair_value_memdup_buffer(vp, vec->umts.authn, true) == 0); TALLOC_FREE(vec->umts.authn); RDEBUG2("&control.%pP", vp); diff --git a/src/modules/rlm_soh/rlm_soh.c b/src/modules/rlm_soh/rlm_soh.c index cf0629cc497..7e003e6e6d4 100644 --- a/src/modules/rlm_soh/rlm_soh.c +++ b/src/modules/rlm_soh/rlm_soh.c @@ -175,7 +175,7 @@ static unlang_action_t CC_HINT(nonnull) mod_post_auth(rlm_rcode_t *p_result, mod RDEBUG2("SoH adding NAP marker to DHCP reply"); /* client probe; send "NAP" in the reply */ - MEM(vp = fr_pair_afrom_da(request->reply, attr_dhcp_vendor)); + MEM(vp = fr_pair_afrom_da(request->reply_ctx, attr_dhcp_vendor)); MEM(fr_pair_value_mem_alloc(vp, &p, 5, false) == 0); p[0] = 220; p[1] = 3; diff --git a/src/modules/rlm_sql/rlm_sql.c b/src/modules/rlm_sql/rlm_sql.c index fcc70a25f01..722ccf195b1 100644 --- a/src/modules/rlm_sql/rlm_sql.c +++ b/src/modules/rlm_sql/rlm_sql.c @@ -871,7 +871,7 @@ static unlang_action_t rlm_sql_process_groups(rlm_rcode_t *p_result, } fr_cursor_init(&cursor, &check_tmp); - rows = sql_getvpdata(request, inst, request, handle, &cursor, expanded); + rows = sql_getvpdata(request->control_ctx, inst, request, handle, &cursor, expanded); TALLOC_FREE(expanded); if (rows < 0) { REDEBUG("Error retrieving check pairs for group %s", entry->name); @@ -926,7 +926,7 @@ static unlang_action_t rlm_sql_process_groups(rlm_rcode_t *p_result, } fr_cursor_init(&cursor, &reply_tmp); - rows = sql_getvpdata(request->reply, inst, request, handle, &cursor, expanded); + rows = sql_getvpdata(request->reply_ctx, inst, request, handle, &cursor, expanded); TALLOC_FREE(expanded); if (rows < 0) { REDEBUG("Error retrieving reply pairs for group %s", entry->name); @@ -1254,7 +1254,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authorize(rlm_rcode_t *p_result, mod } fr_cursor_init(&cursor, &check_tmp); - rows = sql_getvpdata(request, inst, request, &handle, &cursor, expanded); + rows = sql_getvpdata(request->control_ctx, inst, request, &handle, &cursor, expanded); TALLOC_FREE(expanded); if (rows < 0) { REDEBUG("Failed getting check attributes"); @@ -1304,7 +1304,7 @@ static unlang_action_t CC_HINT(nonnull) mod_authorize(rlm_rcode_t *p_result, mod } fr_cursor_init(&cursor, &reply_tmp); - rows = sql_getvpdata(request->reply, inst, request, &handle, &cursor, expanded); + rows = sql_getvpdata(request->reply_ctx, inst, request, &handle, &cursor, expanded); TALLOC_FREE(expanded); if (rows < 0) { REDEBUG("SQL query error getting reply attributes"); diff --git a/src/modules/rlm_sqlippool/rlm_sqlippool.c b/src/modules/rlm_sqlippool/rlm_sqlippool.c index b08f22e451f..bd928efbfb6 100644 --- a/src/modules/rlm_sqlippool/rlm_sqlippool.c +++ b/src/modules/rlm_sqlippool/rlm_sqlippool.c @@ -628,7 +628,7 @@ static unlang_action_t CC_HINT(nonnull) mod_alloc(rlm_rcode_t *p_result, module_ * See if we can create the VP from the returned data. If not, * error out. If so, add it to the list. */ - MEM(vp = fr_pair_afrom_da(request->reply, inst->allocated_address_da)); + MEM(vp = fr_pair_afrom_da(request->reply_ctx, inst->allocated_address_da)); if (fr_pair_value_from_str(vp, allocation, allocation_len, '\0', true) < 0) { DO_PART(alloc_commit); diff --git a/src/modules/rlm_stats/rlm_stats.c b/src/modules/rlm_stats/rlm_stats.c index 10e3b4f8b17..3ec2513506d 100644 --- a/src/modules/rlm_stats/rlm_stats.c +++ b/src/modules/rlm_stats/rlm_stats.c @@ -302,7 +302,7 @@ static unlang_action_t CC_HINT(nonnull) mod_stats(rlm_rcode_t *p_result, module_ } if (vp ) { - vp = fr_pair_copy(request->reply, vp); + vp = fr_pair_copy(request->reply_ctx, vp); if (vp) { fr_cursor_append(&cursor, vp); (void) fr_cursor_tail(&cursor); @@ -320,7 +320,7 @@ static unlang_action_t CC_HINT(nonnull) mod_stats(rlm_rcode_t *p_result, module_ da = fr_dict_attr_by_name(NULL, fr_dict_root(dict_radius), buffer); if (!da) continue; - MEM(vp = fr_pair_afrom_da(request->reply, da)); + MEM(vp = fr_pair_afrom_da(request->reply_ctx, da)); vp->vp_uint64 = local_stats[i]; fr_cursor_append(&cursor, vp); diff --git a/src/modules/rlm_unpack/rlm_unpack.c b/src/modules/rlm_unpack/rlm_unpack.c index 787ca92421f..3a825d23906 100644 --- a/src/modules/rlm_unpack/rlm_unpack.c +++ b/src/modules/rlm_unpack/rlm_unpack.c @@ -172,7 +172,7 @@ static ssize_t unpack_xlat(UNUSED TALLOC_CTX *ctx, char **out, size_t outlen, goto nothing; } - MEM(vp = fr_pair_afrom_da(request, da)); + MEM(vp = fr_pair_afrom_da(request->request_ctx, da)); /* * Call the generic routines to get data from the diff --git a/src/tests/util/atomic_queue_test.c b/src/tests/util/atomic_queue_test.c index 6e8d2ed9ed7..110ef8cffc5 100644 --- a/src/tests/util/atomic_queue_test.c +++ b/src/tests/util/atomic_queue_test.c @@ -39,15 +39,9 @@ static int debug_lvl = 0; /**********************************************************************/ typedef struct request_s request_t; -request_t *request_alloc(UNUSED TALLOC_CTX *ctx); void request_verify(UNUSED char const *file, UNUSED int line, UNUSED request_t *request); int talloc_const_free(void const *ptr); -request_t *request_alloc(UNUSED TALLOC_CTX *ctx) -{ - return NULL; -} - void request_verify(UNUSED char const *file, UNUSED int line, UNUSED request_t *request) { } diff --git a/src/tests/util/channel_test.c b/src/tests/util/channel_test.c index 61190a58e37..baaaaa74326 100644 --- a/src/tests/util/channel_test.c +++ b/src/tests/util/channel_test.c @@ -53,7 +53,7 @@ static bool touch_memory = false; /**********************************************************************/ typedef struct request_s request_t; -request_t *request_alloc(UNUSED TALLOC_CTX *ctx) +request_t *request_alloc(UNUSED TALLOC_CTX *ctx, UNUSED request_init_args_t const *args) { return NULL; } diff --git a/src/tests/util/control_test.c b/src/tests/util/control_test.c index bf5fa269c1e..eb6bd29b855 100644 --- a/src/tests/util/control_test.c +++ b/src/tests/util/control_test.c @@ -51,11 +51,11 @@ static fr_ring_buffer_t *rb = NULL; /**********************************************************************/ typedef struct request_s request_t; -request_t *request_alloc(UNUSED TALLOC_CTX *ctx); +request_t *request_alloc(UNUSED TALLOC_CTX *ctx, UNUSED request_init_args_t const *args); void request_verify(UNUSED char const *file, UNUSED int line, UNUSED request_t *request); int talloc_const_free(void const *ptr); -request_t *request_alloc(UNUSED TALLOC_CTX *ctx) +request_t *request_alloc(UNUSED TALLOC_CTX *ctx, UNUSED request_init_args_t const *args) { return NULL; } diff --git a/src/tests/util/message_set_test.c b/src/tests/util/message_set_test.c index 5e1b2e28e4e..2c6e5bbee36 100644 --- a/src/tests/util/message_set_test.c +++ b/src/tests/util/message_set_test.c @@ -63,11 +63,11 @@ static size_t allocation_mask = 0x3ff; /**********************************************************************/ typedef struct request_s request_t; -request_t *request_alloc(UNUSED TALLOC_CTX *ctx); +request_t *request_alloc(UNUSED TALLOC_CTX *ctx, UNUSED request_init_args_t const *args); void request_verify(UNUSED char const *file, UNUSED int line, UNUSED request_t *request); int talloc_const_free(void const *ptr); -request_t *request_alloc(UNUSED TALLOC_CTX *ctx) +request_t *request_alloc(UNUSED TALLOC_CTX *ctx, UNUSED request_init_args_t const *args) { return NULL; } diff --git a/src/tests/util/ring_buffer_test.c b/src/tests/util/ring_buffer_test.c index d85e5a4e95f..620d120b839 100644 --- a/src/tests/util/ring_buffer_test.c +++ b/src/tests/util/ring_buffer_test.c @@ -46,11 +46,11 @@ static size_t seed_string_len = 3; /**********************************************************************/ typedef struct request_s request_t; -request_t *request_alloc(UNUSED TALLOC_CTX *ctx); +request_t *request_alloc(UNUSED TALLOC_CTX *ctx, UNUSED request_init_args_t const *args); void request_verify(UNUSED char const *file, UNUSED int line, UNUSED request_t *request); int talloc_const_free(void const *ptr); -request_t *request_alloc(UNUSED TALLOC_CTX *ctx) +request_t *request_alloc(UNUSED TALLOC_CTX *ctx, UNUSED request_init_args_t const *args) { return NULL; } diff --git a/src/tests/util/worker_test.c b/src/tests/util/worker_test.c index 3d15454b24a..ca8164d5fe9 100644 --- a/src/tests/util/worker_test.c +++ b/src/tests/util/worker_test.c @@ -67,7 +67,7 @@ static fr_schedule_worker_t workers[MAX_WORKERS]; /**********************************************************************/ typedef struct request_s request_t; -request_t *request_alloc(UNUSED TALLOC_CTX *ctx) +request_t *request_alloc(UNUSED TALLOC_CTX *ctx, UNUSED request_init_args_t const *args) { return NULL; }