From: Nick Porter Date: Tue, 2 May 2023 09:16:28 +0000 (+0100) Subject: Add a dlist of queries still referencing an LDAP connection X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b3264ed7b5a5de77e36f231aa4a82bc182ef6de1;p=thirdparty%2Ffreeradius-server.git Add a dlist of queries still referencing an LDAP connection Queries need to be removed from the rb tree of outstanding queries once a reply has been received as the msgid can be reused. However, the connection needs to persist until all queries referencing it have been freed to prevent use after free issues. This list is used to determine if a connection can be freed. --- diff --git a/src/lib/ldap/base.c b/src/lib/ldap/base.c index 01b3f4d5c7e..57952b59c92 100644 --- a/src/lib/ldap/base.c +++ b/src/lib/ldap/base.c @@ -962,13 +962,18 @@ static int _ldap_query_free(fr_ldap_query_t *query) fr_dlist_talloc_free(&query->referrals); - /* - * If the connection this query was using has no pending queries and - * is no-longer associated with a fr_connection_t then free it - */ - if ((query->ldap_conn) && (query->ldap_conn->conn == NULL) && - (fr_rb_num_elements(query->ldap_conn->queries) == 0)) { - talloc_free(query->ldap_conn); + if (query->ldap_conn) { + /* + * Remove the query from the list of references to its connection + */ + fr_dlist_remove(&query->ldap_conn->refs, query); + + /* + * If the connection this query was using has no pending queries and + * is no-longer associated with a fr_connection_t then free it + */ + if (!query->ldap_conn->conn && (fr_dlist_num_elements(&query->ldap_conn->refs) == 0) && + (fr_rb_num_elements(query->ldap_conn->queries) == 0)) talloc_free(query->ldap_conn); } /* diff --git a/src/lib/ldap/base.h b/src/lib/ldap/base.h index 99e7a8cb3f1..77cf5d7c8c4 100644 --- a/src/lib/ldap/base.h +++ b/src/lib/ldap/base.h @@ -346,6 +346,7 @@ typedef struct { int fd; //!< File descriptor for this connection. fr_rb_tree_t *queries; //!< Outstanding queries on this connection + fr_dlist_head_t refs; //!< Replied to queries still referencing this connection. void *uctx; //!< User data associated with the handle. } fr_ldap_connection_t; @@ -416,6 +417,7 @@ typedef void (*fr_ldap_result_parser_t)(LDAP *handle, fr_ldap_query_t *query, LD */ struct fr_ldap_query_s { fr_rb_node_t node; //!< Entry in the tree of outstanding queries. + fr_dlist_t entry; //!< Entry in the list of connection references. LDAPURLDesc *ldap_url; //!< parsed URL for current query if the source ///< of the query was a URL. diff --git a/src/lib/ldap/connection.c b/src/lib/ldap/connection.c index e1ad4ec2ae6..dd07e791480 100644 --- a/src/lib/ldap/connection.c +++ b/src/lib/ldap/connection.c @@ -218,7 +218,7 @@ static int _ldap_connection_free(fr_ldap_connection_t *c) /* * If there are any pending queries, don't free */ - if ((c->queries) && (fr_rb_num_elements(c->queries) > 0)) return -1; + if (((c->queries) && (fr_rb_num_elements(c->queries) > 0)) || (fr_dlist_num_elements(&c->refs) > 0)) return -1; talloc_free_children(c); /* Force inverted free order */ @@ -283,7 +283,7 @@ static void _ldap_connection_close_watch(fr_connection_t *conn, UNUSED fr_connec { fr_ldap_connection_t *ldap_conn = talloc_get_type_abort(uctx, fr_ldap_connection_t); - if (fr_rb_num_elements(ldap_conn->queries) == 0) return; + if ((fr_rb_num_elements(ldap_conn->queries) == 0) && (fr_dlist_num_elements(&ldap_conn->refs) == 0)) return; talloc_reparent(conn, NULL, ldap_conn); ldap_conn->conn = NULL; @@ -364,6 +364,7 @@ static fr_connection_state_t _ldap_connection_init(void **h, fr_connection_t *co * Initialise tree for outstanding queries handled by this connection */ MEM(c->queries = fr_rb_inline_talloc_alloc(c, fr_ldap_query_t, node, fr_ldap_query_cmp, NULL)); + fr_dlist_init(&c->refs, fr_ldap_query_t, entry); *h = c; /* Set the handle */ @@ -696,6 +697,12 @@ static void ldap_trunk_request_mux(UNUSED fr_event_list_t *el, fr_trunk_connecti if (status != LDAP_PROC_SUCCESS) goto error; + /* + * If the query has previously been associated with a different + * connection, remove that reference. Typically when following references. + */ + if (query->ldap_conn) fr_dlist_remove(&query->ldap_conn->refs, query); + /* * Record which connection was used for this query * - results processing often needs access to an LDAP handle @@ -794,6 +801,12 @@ static void ldap_trunk_request_demux(fr_event_list_t *el, fr_trunk_connection_t */ fr_rb_remove(ldap_conn->queries, query); + /* + * Add the query to the list of queries referencing this connection. + * Prevents the connection from being freed until the query has finished using it. + */ + fr_dlist_insert_tail(&ldap_conn->refs, query); + /* * This really shouldn't happen - as we only retrieve complete sets of results - * but as the query data structure will last until its results are fully handled