mdssvc: prevent a crash when pending search finishes after the client closed the...

When a search is in-flight and currently being processed against the
Elasticsearch server, we set s->pending. In the destructor of "s" we check "pending"
and reject deallocation of the object.

One instance where "s" is requested to be deallocated is when the client closes
the top-level per-share search connection. This will implicitly close all
searches associated with the mds_ctx from mds_ctx_destructor_cb():

	while (mds_ctx->query_list != NULL) {
		/*
		 * slq destructor removes element from list.
		 * Don't use TALLOC_FREE()!
		 */
		talloc_free(mds_ctx->query_list);
	}

So when this happens the Elasticsearch backend query object stays around,
alongside with any active tevent_req request and a tevent_req timer set with
tevent_req_set_endtime() in mds_es_search_send().

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14915
RN: mdssvc crashes when searches are pending and the client closes the mdssvc IPC pipe

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Noel Power <npower@samba.org>

diff --git a/source3/rpc_server/mdssvc/mdssvc_es.c b/source3/rpc_server/mdssvc/mdssvc_es.c
index 46f2917dbfec8b202c0ad9e8a1a93d380f54e2e7..b16426bc65a11922f381ef4dd081769265f5325d 100644 (file)
--- a/source3/rpc_server/mdssvc/mdssvc_es.c
+++ b/source3/rpc_server/mdssvc/mdssvc_es.c
@@ -115,6 +115,26 @@ static bool mds_es_next_search_trigger(struct mds_es_ctx *mds_es_ctx);
 static void mds_es_search_set_pending(struct sl_es_search *s);
 static void mds_es_search_unset_pending(struct sl_es_search *s);
 
+static int mds_es_ctx_destructor(struct mds_es_ctx *mds_es_ctx)
+{
+       struct sl_es_search *s = mds_es_ctx->searches;
+
+       /*
+        * The per tree-connect state mds_es_ctx (a child of mds_ctx) is about
+        * to go away and has already freed all waiting searches. If there's a
+        * search remaining that's when the search is already active. Reset the
+        * mds_es_ctx pointer, so we can detect this when the search completes.
+        */
+
+       if (s == NULL) {
+               return 0;
+       }
+
+       s->mds_es_ctx = NULL;
+
+       return 0;
+}
+
 static bool mds_es_connect(struct mds_ctx *mds_ctx)
 {
        struct mdssvc_es_ctx *mdssvc_es_ctx = talloc_get_type_abort(
@@ -132,6 +152,7 @@ static bool mds_es_connect(struct mds_ctx *mds_ctx)
        };
 
        mds_ctx->backend_private = mds_es_ctx;
+       talloc_set_destructor(mds_es_ctx, mds_es_ctx_destructor);
 
        subreq = mds_es_connect_send(
                        mds_es_ctx,
@@ -349,6 +370,9 @@ static void mds_es_reconnect_on_error(struct sl_es_search *s)
 
 static int search_destructor(struct sl_es_search *s)
 {
+       if (s->mds_es_ctx == NULL) {
+               return 0;
+       }
        DLIST_REMOVE(s->mds_es_ctx->searches, s);
        return 0;
 }
@@ -444,6 +468,15 @@ static void mds_es_search_done(struct tevent_req *subreq)
        DBG_DEBUG("Search done for search [%p]\n", s);
 
        mds_es_search_unset_pending(s);
+
+       if (mds_es_ctx == NULL) {
+               /*
+                * Search connection closed by the user while s was pending.
+                */
+               TALLOC_FREE(s);
+               return;
+       }
+
        DLIST_REMOVE(mds_es_ctx->searches, s);
 
        ret = mds_es_search_recv(subreq);