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 01b3f4d5c7ec71646704089b3736cd6fe74fbbe5..57952b59c928ed43f50e7bdb90e51ecda68f9fd6 100644 (file)
--- 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 99e7a8cb3f1b5eec4511b2481adc1694b40c8397..77cf5d7c8c474d9487749d5f5fc4e294b7f29586 100644 (file)
--- 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 e1ad4ec2ae651e9792d06131fbe9fee2af50d99a..dd07e7914804d60821b053192ffb7d63896b164a 100644 (file)
--- 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