From: Arran Cudbard-Bell Date: Tue, 3 Sep 2019 17:55:28 +0000 (-0400) Subject: Split request_data functions out and teach them about parent/child relationships X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a523ea39ce5f09f9bcabdb79d7e52dffdc473968;p=thirdparty%2Ffreeradius-server.git Split request_data functions out and teach them about parent/child relationships --- diff --git a/src/lib/server/all.mk b/src/lib/server/all.mk index 5cbb011da7d..d2592037bb2 100644 --- a/src/lib/server/all.mk +++ b/src/lib/server/all.mk @@ -28,6 +28,7 @@ SOURCES := \ pool.c \ rcode.c \ regex.c \ + request_data.c \ request.c \ snmp.c \ state.c \ diff --git a/src/lib/server/base.h b/src/lib/server/base.h index 92d9faaeb63..32b52a6272c 100644 --- a/src/lib/server/base.h +++ b/src/lib/server/base.h @@ -57,6 +57,7 @@ RCSIDH(base_h, "$Id$") #include #include #include +#include #include #include #include diff --git a/src/lib/server/regex.c b/src/lib/server/regex.c index 937392dd0bc..d830b510a70 100644 --- a/src/lib/server/regex.c +++ b/src/lib/server/regex.c @@ -26,6 +26,7 @@ RCSID("$Id$") #include +#include #include #ifdef HAVE_REGEX diff --git a/src/lib/server/request.c b/src/lib/server/request.c index a9a9f05a650..09c129d8060 100644 --- a/src/lib/server/request.c +++ b/src/lib/server/request.c @@ -28,22 +28,6 @@ RCSID("$Id$") #include #include -/** Per-request opaque data, added by modules - * - */ -struct request_data_t { - fr_dlist_t list; //!< Next opaque request data struct linked to this request. - - void const *unique_ptr; //!< Key to lookup request data. - int unique_int; //!< Alternative key to lookup request data. - char const *type; //!< Opaque type e.g. VALUE_PAIR, fr_dict_attr_t etc... - void *opaque; //!< Opaque data. - bool free_on_replace; //!< Whether to talloc_free(opaque) when the request data is removed. - bool free_on_parent; //!< Whether to talloc_free(opaque) when the request is freed - bool persist; //!< Whether this data should be transfered to a session_entry_t - //!< after we're done processing this request. -}; - /** Callback for freeing a request struct * */ @@ -114,7 +98,7 @@ REQUEST *request_alloc(TALLOC_CTX *ctx) /* * Initialise the request data list */ - fr_dlist_talloc_init(&request->data, request_data_t, list); + request_data_list_init(&request->data); return request; } @@ -298,433 +282,6 @@ int request_detach(REQUEST *fake, bool will_free) return 0; } -/* Initialise a dlist for storing request data - * - * @param[in] list to initialise. - */ -void request_data_list_init(fr_dlist_head_t *data) -{ - fr_dlist_talloc_init(data, request_data_t, list); -} - -/** Ensure opaque data is freed by binding its lifetime to the request_data_t - * - * @param rd Request data being freed. - * @return - * - 0 if free on parent is false or there's no opaque data. - * - ...else whatever the destructor for the opaque data returned. - */ -static int _request_data_free(request_data_t *rd) -{ - if (rd->free_on_parent && rd->opaque) { - int ret; - - DEBUG4("%s: Freeing request data %s%s%p at %p:%i via destructor", - __FUNCTION__, - rd->type ? rd->type : "", rd->type ? " " : "", - rd->opaque, rd->unique_ptr, rd->unique_int); - ret = talloc_free(rd->opaque); - rd->opaque = NULL; - - return ret; - } - return 0; -} - -/** Add opaque data to a REQUEST - * - * The unique ptr is meant to be a module configuration, and the unique - * integer allows the caller to have multiple opaque data associated with a REQUEST. - * - * @param[in] request to associate data with. - * @param[in] unique_ptr Identifier for the data. - * @param[in] unique_int Qualifier for the identifier. - * @param[in] type Type of data (if talloced) - * @param[in] opaque Data to associate with the request. May be NULL. - * @param[in] free_on_replace Free opaque data if this request_data is replaced. - * @param[in] free_on_parent Free opaque data if the request is freed. - * Must not be set if the opaque data is also parented by - * the request or state (double free). - * @param[in] persist Transfer request data to an #fr_state_entry_t, and - * add it back to the next request we receive for the - * session. - * @return - * - -2 on bad arguments. - * - -1 on memory allocation error. - * - 0 on success. - */ -int _request_data_add(REQUEST *request, void const *unique_ptr, int unique_int, char const *type, void *opaque, - bool free_on_replace, bool free_on_parent, bool persist) -{ - request_data_t *rd = NULL; - - /* - * Request must have a state ctx - */ - rad_assert(request); - rad_assert(!persist || request->state_ctx); - rad_assert(!persist || - (talloc_parent(opaque) == request->state_ctx) || - (talloc_parent(opaque) == talloc_null_ctx())); - rad_assert(!free_on_parent || (talloc_parent(opaque) != request)); - -#ifndef TALLOC_GET_TYPE_ABORT_NOOP - if (type) opaque = _talloc_get_type_abort(opaque, type, __location__); -#endif - - while ((rd = fr_dlist_next(&request->data, rd))) { - if ((rd->unique_ptr != unique_ptr) || (rd->unique_int != unique_int)) continue; - - fr_dlist_remove(&request->data, rd); /* Unlink from the list */ - - /* - * If caller requires custom behaviour on free - * they must set a destructor. - */ - if (rd->free_on_replace && rd->opaque) { - RDEBUG4("%s: Freeing %s%s%p at %p:%i via replacement", - __FUNCTION__, - rd->type ? rd->type : "", rd->type ? " " : "", - rd->opaque, rd->unique_ptr, rd->unique_int); - talloc_free(rd->opaque); - } - /* - * Need a new one, rd one's parent is wrong. - * And no, we can't just steal. - */ - if (rd->persist != persist) { - rd->free_on_parent = false; - TALLOC_FREE(rd); - } - - break; /* replace the existing entry */ - } - - /* - * Only alloc new memory if we're not replacing - * an existing entry. - * - * Tie the lifecycle of the data to either the state_ctx - * or the request, depending on whether it should - * persist or not. - */ - if (!rd) { - if (persist) { - rad_assert(request->state_ctx); - rd = talloc_zero(request->state_ctx, request_data_t); - } else { - rd = talloc_zero(request, request_data_t); - } - talloc_set_destructor(rd, _request_data_free); - } - if (!rd) return -1; - - rd->unique_ptr = unique_ptr; - rd->unique_int = unique_int; - rd->type = type; - rd->opaque = opaque; - rd->free_on_replace = free_on_replace; - rd->free_on_parent = free_on_parent; - rd->persist = persist; - - fr_dlist_insert_head(&request->data, rd); - - RDEBUG4("%s: %s%s%p at %p:%i, free_on_replace: %s, free_on_parent: %s, persist: %s", - __FUNCTION__, - rd->type ? rd->type : "", rd->type ? " " : "", - rd->opaque, rd->unique_ptr, rd->unique_int, - free_on_replace ? "yes" : "no", - free_on_parent ? "yes" : "no", - persist ? "yes" : "no"); - - return 0; -} - -/** Get opaque data from a request - * - * @note The unique ptr is meant to be a module configuration, and the unique - * integer allows the caller to have multiple opaque data associated with a REQUEST. - * - * @param[in] request to retrieve data from. - * @param[in] unique_ptr Identifier for the data. - * @param[in] unique_int Qualifier for the identifier. - * @return - * - NULL if no opaque data could be found. - * - the opaque data. The entry holding the opaque data is removed from the request. - */ -void *request_data_get(REQUEST *request, void const *unique_ptr, int unique_int) -{ - request_data_t *rd = NULL; - - if (!request) return NULL; - - while ((rd = fr_dlist_next(&request->data, rd))) { - void *ptr; - - if ((rd->unique_ptr != unique_ptr) || (rd->unique_int != unique_int)) continue; - - ptr = rd->opaque; - - rd->free_on_parent = false; /* Don't free opaque data we're handing back */ - fr_dlist_remove(&request->data, rd); - -#ifndef TALLOC_GET_TYPE_ABORT_NOOP - if (rd->type) ptr = _talloc_get_type_abort(ptr, rd->type, __location__); -#endif - - RDEBUG4("%s: %s%s%p at %p:%i retrieved and unlinked", - __FUNCTION__, - rd->type ? rd->type : "", rd->type ? " " : "", - rd->opaque, rd->unique_ptr, rd->unique_int); - - talloc_free(rd); - - return ptr; - } - - RDEBUG4("%s: No request data found at %p:%i", __FUNCTION__, unique_ptr, unique_int); - - return NULL; /* wasn't found, too bad... */ -} - -/** Get opaque data from a request without removing it - * - * @note The unique ptr is meant to be a module configuration, and the unique - * integer allows the caller to have multiple opaque data associated with a REQUEST. - * - * @param request to retrieve data from. - * @param unique_ptr Identifier for the data. - * @param unique_int Qualifier for the identifier. - * @return - * - NULL if no opaque data could be found. - * - the opaque data. - */ -void *request_data_reference(REQUEST *request, void const *unique_ptr, int unique_int) -{ - request_data_t *rd = NULL; - - if (!request) return NULL; - - while ((rd = fr_dlist_next(&request->data, rd))) { - if ((rd->unique_ptr != unique_ptr) || (rd->unique_int != unique_int)) continue; - -#ifndef TALLOC_GET_TYPE_ABORT_NOOP - if (rd->type) rd->opaque = _talloc_get_type_abort(rd->opaque, rd->type, __location__); -#endif - - RDEBUG4("%s: %s%s%p at %p:%i retrieved", - __FUNCTION__, - rd->type ? rd->type : "", rd->type ? " " : "", - rd->opaque, rd->unique_ptr, rd->unique_int); - - return rd->opaque; - } - - RDEBUG4("%s: No request data found at %p:%i", __FUNCTION__, unique_ptr, unique_int); - - return NULL; /* wasn't found, too bad... */ -} - -/** Loop over all the request data, pulling out ones matching persist state - * - * @param[out] out Head of result list. - * @param[in] request to search for request_data_t in. - * @param[in] persist Whether to pull persistable or non-persistable data. - * @return number of request_data_t retrieved. - */ -int request_data_by_persistance(fr_dlist_head_t *out, REQUEST *request, bool persist) -{ - int count = 0; - request_data_t *rd = NULL, *prev; - - while ((rd = fr_dlist_next(&request->data, rd))) { - if (rd->persist != persist) continue; - - prev = fr_dlist_remove(&request->data, rd); - fr_dlist_insert_tail(out, rd); - rd = prev; - } - - return count; -} - -/** Return how many request data entries exist of a given persistence - * - * @param[in] request to check in. - * @param[in] persist Whether to count persistable or non-persistable data. - * @return number of request_data_t that exist in persistable or non-persistable form - */ -int request_data_by_persistance_count(REQUEST *request, bool persist) -{ - int count = 0; - request_data_t *rd = NULL; - - while ((rd = fr_dlist_next(&request->data, rd))) { - if (rd->persist != persist) continue; - - count++; - } - - return count; -} - -/** Add request data back to a request - * - * @note May add multiple entries (if they're linked). - * @note Will not check for duplicates. - * - * @param request to add data to. - * @param in Data to add. - */ -void request_data_restore(REQUEST *request, fr_dlist_head_t *in) -{ - fr_dlist_move(&request->data, in); -} - -/** Realloc any request_data_t structs in a new ctx - * - */ -void request_data_ctx_change(TALLOC_CTX *state_ctx, REQUEST *request) -{ - fr_dlist_head_t head; - request_data_t *rd = NULL, *prev; - - fr_dlist_talloc_init(&head, request_data_t, list); - - while ((rd = fr_dlist_next(&request->data, rd))) { - request_data_t *new; - - if (!rd->persist) continue; /* Parented by the request */ - - prev = fr_dlist_remove(&request->data, rd); /* Unlink from the list */ - new = talloc(state_ctx, request_data_t); - memcpy(new, rd, sizeof(*new)); - rd->free_on_parent = false; - talloc_free(rd); - rd = prev; - - fr_dlist_insert_tail(&head, new); - } - - fr_dlist_move(&request->data, &head); -} - -/** Used for removing data from subrequests that are about to be freed - * - * @param[in] request to remove persistable data from. - */ -void request_data_persistable_free(REQUEST *request) -{ - fr_dlist_head_t head; - - fr_dlist_talloc_init(&head, request_data_t, list); - - request_data_by_persistance(&head, request, true); - - fr_dlist_talloc_free(&head); -} - -void request_data_dump(REQUEST *request) -{ - request_data_t *rd = NULL; - int count = 0; - - if (fr_dlist_empty(&request->data)) { - RDEBUG("No request data"); - return; - } - - RDEBUG("Current request data:"); - RINDENT(); - while ((rd = fr_dlist_next(&request->data, rd))) { - RDEBUG("[%i] %s%p %s at %p:%i", - count, - rd->type ? rd->type : "", - rd->opaque, - rd->persist ? "[persist]" : "", - rd->unique_ptr, - rd->unique_int); - - count++; - } - REXDENT(); -} - -/** Free any subrequest request data if the dlist head is freed - * - */ -static int _free_subrequest_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 - * - * @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. - */ -void request_data_store_in_parent(REQUEST *request, void *unique_ptr, int unique_int) -{ - fr_dlist_head_t *head; - - if (request_data_by_persistance_count(request, true) == 0) return; - - MEM(head = talloc_zero(request->parent->state_ctx, fr_dlist_head_t)); - fr_dlist_talloc_init(head, request_data_t, list); - talloc_set_destructor(head, _free_subrequest_data); - - /* - * Pull everything out of the child, - * add it to our temporary list head... - */ - request_data_by_persistance(head, request, true); - - /* - * ...add that to the parent request under - * the specified unique identifiers. - */ - request_data_add(request->parent, unique_ptr, unique_int, head, true, false, true); -} - -/** 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. - */ -void request_data_restore_to_child(REQUEST *request, void *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. - */ - rad_assert(talloc_get_size(request->state_ctx) == 0); - 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; - - request_data_restore(request, head); - talloc_free(head); -} - #ifdef WITH_VERIFY_PTR /* * Verify a packet. @@ -801,7 +358,7 @@ void request_verify(char const *file, int line, REQUEST const *request) while ((rd = fr_dlist_next(&request->data, rd))) { (void) talloc_get_type_abort(rd, request_data_t); - if (rd->persist) { + if (request_data_persistable(rd)) { rad_assert(request->state_ctx); rad_assert(talloc_parent(rd) == request->state_ctx); } else { @@ -809,23 +366,4 @@ void request_verify(char const *file, int line, REQUEST const *request) } } } - -/** Verify all request data is parented by the specified context - * - * @note Only available if built with WITH_VERIFY_PTR - * - * @param parent that should hold the request data. - * @param entry to verify. - * @return - * - true if chunk lineage is correct. - * - false if one of the chunks is parented by something else. - */ -bool request_data_verify_parent(TALLOC_CTX *parent, fr_dlist_head_t *entry) -{ - request_data_t *rd = NULL; - - while ((rd = fr_dlist_next(entry, rd))) if (talloc_parent(rd) != parent) return false; - - return true; -} #endif diff --git a/src/lib/server/request.h b/src/lib/server/request.h index ef4f5f83132..9cce5921fb3 100644 --- a/src/lib/server/request.h +++ b/src/lib/server/request.h @@ -34,7 +34,6 @@ extern "C" { typedef struct fr_async_t fr_async_t; typedef struct rad_request REQUEST; -typedef struct request_data_t request_data_t; typedef struct rad_listen rad_listen_t; typedef struct rad_client RADCLIENT; @@ -182,39 +181,6 @@ REQUEST *request_alloc_detachable(REQUEST *request, fr_dict_t const *namespace) int request_detach(REQUEST *fake, bool will_free); -void request_data_list_init(fr_dlist_head_t *data); - -#define request_data_add(_request, _unique_ptr, _unique_int, _opaque, _free_on_replace, _free_on_parent, _persist) \ - _request_data_add(_request, _unique_ptr, _unique_int, NULL, _opaque, \ - _free_on_replace, _free_on_parent, _persist) - -#define request_data_talloc_add(_request, _unique_ptr, _unique_int, _type, _opaque, _free_on_replace, _free_on_parent, _persist) \ - _request_data_add(_request, _unique_ptr, _unique_int, STRINGIFY(_type), _opaque, \ - _free_on_replace, _free_on_parent, _persist) - -int _request_data_add(REQUEST *request, void const *unique_ptr, int unique_int, char const *type, void *opaque, - bool free_on_replace, bool free_on_parent, bool persist); - -void *request_data_get(REQUEST *request, void const *unique_ptr, int unique_int); - -void *request_data_reference(REQUEST *request, void const *unique_ptr, int unique_int); - -int request_data_by_persistance(fr_dlist_head_t *out, REQUEST *request, bool persist); - -int request_data_by_persistance_count(REQUEST *request, bool persist); - -void request_data_restore(REQUEST *request, fr_dlist_head_t *in); - -void request_data_ctx_change(TALLOC_CTX *state_ctx, REQUEST *request); - -void request_data_persistable_free(REQUEST *request); - -void request_data_dump(REQUEST *request); - -void request_data_store_in_parent(REQUEST *request, void *unique_ptr, int unique_int); - -void request_data_restore_to_child(REQUEST *request, void *unique_ptr, int unique_int); - #ifdef WITH_VERIFY_PTR void request_verify(char const *file, int line, REQUEST const *request); /* only for special debug builds */ diff --git a/src/lib/server/request_data.c b/src/lib/server/request_data.c new file mode 100644 index 00000000000..47fc089b83e --- /dev/null +++ b/src/lib/server/request_data.c @@ -0,0 +1,631 @@ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +/** + * $Id$ + * + * @brief Functions for allocating requests and storing internal data in them. + * @file src/lib/server/request_data.c + * + * @copyright 2019 The FreeRADIUS server project + */ +RCSID("$Id$") + +#include +#include + +/** Per-request opaque data, added by modules + * + */ +struct request_data_s { + fr_dlist_t list; //!< Next opaque request data struct linked to this request. + + void const *unique_ptr; //!< Key to lookup request data. + int unique_int; //!< Alternative key to lookup request data. + char const *type; //!< Opaque type e.g. VALUE_PAIR, fr_dict_attr_t etc... + void *opaque; //!< Opaque data. + bool free_on_replace; //!< Whether to talloc_free(opaque) when the request data is removed. + bool free_on_parent; //!< Whether to talloc_free(opaque) when the request is freed + bool persist; //!< Whether this data should be transfered to a session_entry_t + //!< after we're done processing this request. + +#ifndef NDEBUG + char const *file; //!< File where this request data was added. + int line; //!< Line where this request data was added. +#endif +}; + +static char *request_data_description(TALLOC_CTX *ctx, request_data_t *rd) +{ + char *where; + char *what; + char *out; + + /* + * Where was the request data added + */ +#ifndef NDEBUG + where = talloc_typed_asprintf(NULL, " added at %s:%i", rd->file, rd->line); +#else + where = NULL; +#endif + + /* + * What was added + */ + if (rd->type) { + what = talloc_typed_asprintf(NULL, "%p (%s)", rd->opaque, rd->type); + } else { + what = talloc_typed_asprintf(NULL, "%p", rd->opaque); + } + + out = talloc_typed_asprintf(ctx, "[0x%012"PRIxPTR":%i]%s %p, opaque %s%s", + (uintptr_t)rd->unique_ptr, + rd->unique_int, + rd->persist ? "[P]" : "", + rd, + what, + where ? where : ""); + talloc_free(what); + talloc_free(where); + + return out; +} + +/* Initialise a dlist for storing request data + * + * @param[in] list to initialise. + */ +void request_data_list_init(fr_dlist_head_t *data) +{ + fr_dlist_talloc_init(data, request_data_t, list); +} + +/** Ensure opaque data is freed by binding its lifetime to the request_data_t + * + * @param rd Request data being freed. + * @return + * - 0 if free on parent is false or there's no opaque data. + * - ...else whatever the destructor for the opaque data returned. + */ +static int _request_data_free(request_data_t *rd) +{ + char *desc = NULL; + + /* + * In the vast majority of cases the request data will + * unlinked from its list before being freed. + * But in case it's not, do this now. + * + * This helps in a very specific case where there's a list + * of request_data_t, and the state_ctx that the + * request_data_t is parented off is freed without the + * request_data_t being unlinked explicitly, but before + * the request itself is freed something attempts to access + * the request_data_t list, and runs into freed memory. + * + * It's a similar pattern to structs removing themselves + * from trees when they're freed, but with the added bonus + * of never running into use after free errors/ + */ + fr_dlist_entry_unlink(&rd->list); + + if (DEBUG_ENABLED4) desc = request_data_description(rd, rd); + + if (rd->free_on_parent && rd->opaque) { + int ret; + + DEBUG4("%s - freed with opaque data", desc); + + ret = talloc_free(rd->opaque); + rd->opaque = NULL; + + return ret; + } + + DEBUG4("%s - freed, but leaving opaque data", desc); + + return 0; +} + +/** Allocate request data + * + * @param[in] ctx to allocate request data in. + * @return new request data. + */ +static inline request_data_t *request_data_alloc(TALLOC_CTX *ctx) +{ + request_data_t *rd; + + MEM(rd = talloc_zero(ctx, request_data_t)); + talloc_set_destructor(rd, _request_data_free); + + return rd; +} + +/** Add opaque data to a REQUEST + * + * The unique ptr is meant to be a module configuration, and the unique + * integer allows the caller to have multiple opaque data associated with a REQUEST. + * + * @param[in] request to associate data with. + * @param[in] unique_ptr Identifier for the data. + * @param[in] unique_int Qualifier for the identifier. + * @param[in] type Type of data (if talloced) + * @param[in] opaque Data to associate with the request. May be NULL. + * @param[in] free_on_replace Free opaque data if this request_data is replaced. + * @param[in] free_on_parent Free opaque data if the request is freed. + * Must not be set if the opaque data is also parented by + * the request or state (double free). + * @param[in] persist Transfer request data to an #fr_state_entry_t, and + * add it back to the next request we receive for the + * session. + * @param[in] file request data was added in. + * @param[in] line request data was added on. + * @return + * - -2 on bad arguments. + * - -1 on memory allocation error. + * - 0 on success. + */ +int _request_data_add(REQUEST *request, void const *unique_ptr, int unique_int, char const *type, void *opaque, + bool free_on_replace, bool free_on_parent, bool persist, +#ifndef NDEBUG + char const *file, int line +#else + UNUSED char const *file, UNUSED int line +#endif + ) +{ + request_data_t *rd = NULL; + + /* + * Request must have a state ctx + */ + rad_assert(request); + rad_assert(!persist || request->state_ctx); + rad_assert(!persist || + (talloc_parent(opaque) == request->state_ctx) || + (talloc_parent(opaque) == talloc_null_ctx())); + rad_assert(!free_on_parent || (talloc_parent(opaque) != request)); + +#ifndef TALLOC_GET_TYPE_ABORT_NOOP + if (type) opaque = _talloc_get_type_abort(opaque, type, __location__); +#endif + + while ((rd = fr_dlist_next(&request->data, rd))) { + if ((rd->unique_ptr != unique_ptr) || (rd->unique_int != unique_int)) continue; + + fr_dlist_remove(&request->data, rd); /* Unlink from the list */ + + /* + * If caller requires custom behaviour on free + * they must set a destructor. + */ + if (rd->free_on_replace && rd->opaque) { + RDEBUG4("%s: Freeing %s%s%p at %p:%i via replacement", + __FUNCTION__, + rd->type ? rd->type : "", rd->type ? " " : "", + rd->opaque, rd->unique_ptr, rd->unique_int); + talloc_free(rd->opaque); + } + /* + * Need a new one, rd one's parent is wrong. + * And no, we can't just steal. + */ + if (rd->persist != persist) { + rd->free_on_parent = false; + TALLOC_FREE(rd); + } + + break; /* replace the existing entry */ + } + + /* + * Only alloc new memory if we're not replacing + * an existing entry. + * + * Tie the lifecycle of the data to either the state_ctx + * or the request, depending on whether it should + * persist or not. + */ + if (!rd) { + if (persist) { + rad_assert(request->state_ctx); + rd = request_data_alloc(request->state_ctx); + } else { + rd = request_data_alloc(request); + } + + } + if (!rd) return -1; + + rd->unique_ptr = unique_ptr; + rd->unique_int = unique_int; + rd->type = type; + rd->opaque = opaque; + rd->free_on_replace = free_on_replace; + rd->free_on_parent = free_on_parent; + rd->persist = persist; +#ifndef NDEBUG + rd->file = file; + rd->line = line; +#endif + + fr_dlist_insert_head(&request->data, rd); + + RDEBUG4("%s: %s%s%p at %p:%i, free_on_replace: %s, free_on_parent: %s, persist: %s", + __FUNCTION__, + rd->type ? rd->type : "", rd->type ? " " : "", + rd->opaque, rd->unique_ptr, rd->unique_int, + free_on_replace ? "yes" : "no", + free_on_parent ? "yes" : "no", + persist ? "yes" : "no"); + + return 0; +} + +/** Get opaque data from a request + * + * @note The unique ptr is meant to be a module configuration, and the unique + * integer allows the caller to have multiple opaque data associated with a REQUEST. + * + * @param[in] request to retrieve data from. + * @param[in] unique_ptr Identifier for the data. + * @param[in] unique_int Qualifier for the identifier. + * @return + * - NULL if no opaque data could be found. + * - the opaque data. The entry holding the opaque data is removed from the request. + */ +void *request_data_get(REQUEST *request, void const *unique_ptr, int unique_int) +{ + request_data_t *rd = NULL; + + if (!request) return NULL; + + while ((rd = fr_dlist_next(&request->data, rd))) { + void *ptr; + + if ((rd->unique_ptr != unique_ptr) || (rd->unique_int != unique_int)) continue; + + ptr = rd->opaque; + + rd->free_on_parent = false; /* Don't free opaque data we're handing back */ + fr_dlist_remove(&request->data, rd); + +#ifndef TALLOC_GET_TYPE_ABORT_NOOP + if (rd->type) ptr = _talloc_get_type_abort(ptr, rd->type, __location__); +#endif + + RDEBUG4("%s: %s%s%p at %p:%i retrieved and unlinked", + __FUNCTION__, + rd->type ? rd->type : "", rd->type ? " " : "", + rd->opaque, rd->unique_ptr, rd->unique_int); + + talloc_free(rd); + + return ptr; + } + + RDEBUG4("%s: No request data found at %p:%i", __FUNCTION__, unique_ptr, unique_int); + + return NULL; /* wasn't found, too bad... */ +} + +/** Get opaque data from a request without removing it + * + * @note The unique ptr is meant to be a module configuration, and the unique + * integer allows the caller to have multiple opaque data associated with a REQUEST. + * + * @param request to retrieve data from. + * @param unique_ptr Identifier for the data. + * @param unique_int Qualifier for the identifier. + * @return + * - NULL if no opaque data could be found. + * - the opaque data. + */ +void *request_data_reference(REQUEST *request, void const *unique_ptr, int unique_int) +{ + request_data_t *rd = NULL; + + if (!request) return NULL; + + while ((rd = fr_dlist_next(&request->data, rd))) { + if ((rd->unique_ptr != unique_ptr) || (rd->unique_int != unique_int)) continue; + +#ifndef TALLOC_GET_TYPE_ABORT_NOOP + if (rd->type) rd->opaque = _talloc_get_type_abort(rd->opaque, rd->type, __location__); +#endif + + RDEBUG4("%s: %s%s%p at %p:%i retrieved", + __FUNCTION__, + rd->type ? rd->type : "", rd->type ? " " : "", + rd->opaque, rd->unique_ptr, rd->unique_int); + + return rd->opaque; + } + + RDEBUG4("%s: No request data found at %p:%i", __FUNCTION__, unique_ptr, unique_int); + + return NULL; /* wasn't found, too bad... */ +} + +/** Loop over all the request data, pulling out ones matching persist state + * + * @param[out] out Head of result list. + * @param[in] request to search for request_data_t in. + * @param[in] persist Whether to pull persistable or non-persistable data. + * @return number of request_data_t retrieved. + */ +int request_data_by_persistance(fr_dlist_head_t *out, REQUEST *request, bool persist) +{ + int count = 0; + request_data_t *rd = NULL, *prev; + + while ((rd = fr_dlist_next(&request->data, rd))) { + if (rd->persist != persist) continue; + + prev = fr_dlist_remove(&request->data, rd); + fr_dlist_insert_tail(out, rd); + rd = prev; + } + + return count; +} + +/** Loop over all the request data, copying, then freeing ones matching persist state + * + * @param[in] ctx To allocate new request_data_t. + * @param[out] out Head of result list. If NULL, data + * will be reparented in place. + * @param[in] request to search for request_data_t in. + * @param[in] persist Whether to pull persistable or non-persistable data. + * @return number of request_data_t retrieved. + */ +int request_data_by_persistance_reparent(TALLOC_CTX *ctx, fr_dlist_head_t *out, REQUEST *request, bool persist) +{ + int count = 0; + request_data_t *rd = NULL, *new, *prev; + fr_dlist_head_t head; + + fr_dlist_talloc_init(&head, request_data_t, list); + + while ((rd = fr_dlist_next(&request->data, rd))) { + if (rd->persist != persist) continue; + + prev = fr_dlist_remove(&request->data, rd); + + new = request_data_alloc(ctx); + memcpy(new, rd, sizeof(*new)); + + /* + * Clear the list pointers... + */ + memset(&new->list, 0, sizeof(new->list)); + rd->free_on_parent = false; + talloc_free(rd); + + if (out) { + fr_dlist_insert_tail(out, new); + } else { + fr_dlist_insert_tail(&head, new); + } + rd = prev; + } + + if (!out) fr_dlist_move(&request->data, &head); + + return count; +} + +/** Return how many request data entries exist of a given persistence + * + * @param[in] request to check in. + * @param[in] persist Whether to count persistable or non-persistable data. + * @return number of request_data_t that exist in persistable or non-persistable form + */ +int request_data_by_persistance_count(REQUEST *request, bool persist) +{ + int count = 0; + request_data_t *rd = NULL; + + while ((rd = fr_dlist_next(&request->data, rd))) { + if (rd->persist != persist) continue; + + count++; + } + + return count; +} + +/** Add request data back to a request + * + * @note May add multiple entries (if they're linked). + * @note Will not check for duplicates. + * + * @param request to add data to. + * @param in Data to add. + */ +void request_data_restore(REQUEST *request, fr_dlist_head_t *in) +{ + fr_dlist_move(&request->data, in); +} + +/** Used for removing data from subrequests that are about to be freed + * + * @param[in] request to remove persistable data from. + */ +void request_data_persistable_free(REQUEST *request) +{ + fr_dlist_head_t head; + + fr_dlist_talloc_init(&head, request_data_t, list); + + request_data_by_persistance(&head, request, true); + + fr_dlist_talloc_free(&head); +} + + +void request_data_list_dump(REQUEST *request, fr_dlist_head_t *head) +{ + request_data_t *rd = NULL; + int count = 0; + + if (fr_dlist_empty(head)) return; + + while ((rd = fr_dlist_next(head, rd))) { + char *desc; + + desc = request_data_description(NULL, rd); + ROPTIONAL(RDEBUG, DEBUG, "%s", desc); + talloc_free(desc); + count++; + } +} + +void request_data_dump(REQUEST *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. + */ +void request_data_store_in_parent(REQUEST *request, void *unique_ptr, int unique_int) +{ + fr_dlist_head_t *child_list; + + if (request_data_by_persistance_count(request, true) == 0) return; + + 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... + */ + if (request->state_ctx == request->parent->state_ctx) { + request_data_by_persistance(child_list, request, true); + /* + * This will only happen where + * request_data_restore_to_child hasn't + * been called on the request first. + * + * Which in most cases will have been + * the case because request_data_store_in_parent + * and request_data_restore_to_child + * are called in pairs. + * + * ...but I can imagine someone trying + * to be clever and avoid calling + * request_data_restore_to_child if they + * think there's no possibility of request + * data being available, so this deals + * with that *sigh* + */ + } else { + request_data_by_persistance_reparent(request->parent->state_ctx, + child_list, request, true); + } + + /* + * ...add that to the parent request under + * the specified unique identifiers. + */ + request_data_talloc_add(request->parent, unique_ptr, unique_int, + fr_dlist_head_t, child_list, true, false, true); +} + +/** 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. + */ +void request_data_restore_to_child(REQUEST *request, void *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. + */ + rad_assert(talloc_get_size(request->state_ctx) == 0); + 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; + + request_data_restore(request, head); + talloc_free(head); +} + +#ifdef WITH_VERIFY_PTR +bool request_data_persistable(request_data_t *rd) +{ + return rd->persist; +} + +/** Verify all request data is parented by the specified context + * + * @note Only available if built with WITH_VERIFY_PTR + * + * @param parent that should hold the request data. + * @param entry to verify. + * @return + * - true if chunk lineage is correct. + * - false if one of the chunks is parented by something else. + */ +bool request_data_verify_parent(TALLOC_CTX *parent, fr_dlist_head_t *entry) +{ + request_data_t *rd = NULL; + + while ((rd = fr_dlist_next(entry, rd))) if (talloc_parent(rd) != parent) return false; + + return true; +} +#endif diff --git a/src/lib/server/request_data.h b/src/lib/server/request_data.h new file mode 100644 index 00000000000..7a961f5fffe --- /dev/null +++ b/src/lib/server/request_data.h @@ -0,0 +1,82 @@ +#pragma once +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +/** + * $Id$ + * + * @file lib/server/request_data.h + * @brief Request data management functions. + * + * @copyright 2019 The FreeRADIUS server project + */ +RCSIDH(request_data_h, "$Id$") + +#include + +#ifdef __cplusplus +extern "C" { +#endif + +typedef struct request_data_s request_data_t; + +void request_data_list_init(fr_dlist_head_t *data); + +#define request_data_add(_request, _unique_ptr, _unique_int, _opaque, _free_on_replace, _free_on_parent, _persist) \ + _request_data_add(_request, _unique_ptr, _unique_int, NULL, _opaque, \ + _free_on_replace, _free_on_parent, _persist, __FILE__, __LINE__) + +#define request_data_talloc_add(_request, _unique_ptr, _unique_int, _type, _opaque, _free_on_replace, _free_on_parent, _persist) \ + _request_data_add(_request, _unique_ptr, _unique_int, STRINGIFY(_type), _opaque, \ + _free_on_replace, _free_on_parent, _persist, __FILE__, __LINE__) + +int _request_data_add(REQUEST *request, void const *unique_ptr, int unique_int, char const *type, void *opaque, + bool free_on_replace, bool free_on_parent, bool persist, char const *file, int line); + +void *request_data_get(REQUEST *request, void const *unique_ptr, int unique_int); + +void *request_data_reference(REQUEST *request, void const *unique_ptr, int unique_int); + +int request_data_by_persistance(fr_dlist_head_t *out, REQUEST *request, bool persist); + +int request_data_by_persistance_reparent(TALLOC_CTX *ctx, fr_dlist_head_t *out, + REQUEST *request, bool persist); + +int request_data_by_persistance_count(REQUEST *request, bool persist); + +void request_data_restore(REQUEST *request, fr_dlist_head_t *in); + +void request_data_persistable_free(REQUEST *request); + +void request_data_list_dump(REQUEST *request, fr_dlist_head_t *head); + +void request_data_dump(REQUEST *request); + +void request_data_store_in_parent(REQUEST *request, void *unique_ptr, int unique_int); + +void request_data_restore_to_child(REQUEST *request, void *unique_ptr, int unique_int); + +#ifdef WITH_VERIFY_PTR +bool request_data_persistable(request_data_t *rd); + +void request_verify(char const *file, int line, REQUEST const *request); /* only for special debug builds */ + +bool request_data_verify_parent(TALLOC_CTX *parent, fr_dlist_head_t *entry); +#endif + +#ifdef __cplusplus +} +#endif diff --git a/src/lib/server/state.c b/src/lib/server/state.c index 11ff1ccebea..e9688d03682 100644 --- a/src/lib/server/state.c +++ b/src/lib/server/state.c @@ -729,15 +729,42 @@ void fr_state_store_in_parent(REQUEST *request, void *unique_ptr, int unique_int { if (unlikely(request->parent == NULL)) return; - RDEBUG3("Subrequest state - saved to %s", request->parent->name); + RDEBUG3("Subrequest state - saved to request %s", request->parent->name); /* * Shove this into the child to make * it easier to store/restore the * whole lot... */ - request_data_add(request, (void *)fr_state_store_in_parent, 0, request->state, true, false, true); - request->state = NULL; + if (request->state) { + VALUE_PAIR *head; + + /* + * 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. + */ + if (request->parent->state_ctx == request->state_ctx) { + request_data_talloc_add(request, (void *)fr_state_store_in_parent, 0, VALUE_PAIR, + request->state, true, false, true); + request->state = NULL; + + /* + * If request_data_restore_to_child has been + * called then this won't be necessary as + * parent and child will share a state_ctx + * but we can't always guarantee that'll have + * happened, and it seems quite fragile to rely + * on that. + */ + } else { + MEM(fr_pair_list_copy(request->parent->state_ctx, &head, request->state) >= 0); + request_data_talloc_add(request, (void *)fr_state_store_in_parent, 0, VALUE_PAIR, + request->state, true, false, true); + fr_pair_list_free(&request->state); + } + } request_data_store_in_parent(request, unique_ptr, unique_int); } @@ -754,7 +781,7 @@ void fr_state_restore_to_child(REQUEST *request, void *unique_ptr, int unique_in { if (unlikely(request->parent == NULL)) return; - RDEBUG3("Subrequest state - restored from %s", request->parent->name); + RDEBUG3("Subrequest state - restored from request %s", request->parent->name); request_data_restore_to_child(request, unique_ptr, unique_int); @@ -803,7 +830,8 @@ void fr_state_detach(REQUEST *request, bool will_free) } MEM(new_state_ctx = talloc_init("session-state")); - request_data_ctx_change(new_state_ctx, request); + 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); fr_pair_list_free(&request->state);