Allocate PAM DP request data on responder context

https://fedorahosted.org/sssd/ticket/1869

Currently the private data passed to the PAM request is a structure
allocated on the client context. But in the odd case where the back end
would be stopped or stuck until the idle timeout hits, the DP callback
would access data that were freed when the client timed out.

This patch introduces a new structure allocated on responder context,
whose only purpose is to live as long as the request is active.

3 files changed, 54 insertions, 5 deletions

diff --git a/src/responder/pam/pamsrv.h b/src/responder/pam/pamsrv.h
index 3617231d..129419b5 100644
--- a/src/responder/pam/pamsrv.h
+++ b/src/responder/pam/pamsrv.h
@@ -39,6 +39,10 @@ struct pam_ctx {
     hash_table_t *id_table;
 };
 
+struct pam_auth_dp_req {
+    struct pam_auth_req *preq;
+};
+
 struct pam_auth_req {
     struct cli_ctx *cctx;
     struct sss_domain_info *domain;
@@ -50,6 +54,8 @@ struct pam_auth_req {
     struct ldb_result *res;
     bool check_provider;
     void *data;
+
+    struct pam_auth_dp_req *dpreq_spy;
 };
 
 struct sss_cmd_table *get_pam_cmds(void);
diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c
index e6300a75..ba4e969f 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -739,6 +739,17 @@ done:
     return ret;
 }
 
+static int pam_auth_req_destructor(struct pam_auth_req *preq)
+{
+    if (preq && preq->dpreq_spy) {
+        /* If there is still a request pending, tell the spy
+         * the client is going away
+         */
+        preq->dpreq_spy->preq = NULL;
+    }
+    return 0;
+}
+
 static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd)
 {
     struct sss_domain_info *dom;
@@ -754,6 +765,7 @@ static int pam_forwarder(struct cli_ctx *cctx, int pam_cmd)
     if (!preq) {
         return ENOMEM;
     }
+    talloc_set_destructor(preq, pam_auth_req_destructor);
     preq->cctx = cctx;
 
     preq->pd = create_pam_data(preq);
diff --git a/src/responder/pam/pamsrv_dp.c b/src/responder/pam/pamsrv_dp.c
index afaca593..46c54dad 100644
--- a/src/responder/pam/pamsrv_dp.c
+++ b/src/responder/pam/pamsrv_dp.c
@@ -37,13 +37,26 @@ static void pam_dp_process_reply(DBusPendingCall *pending, void *ptr)
     DBusMessage* msg;
     int ret;
     int type;
-    struct pam_auth_req *preq;
+    struct pam_auth_req *preq = NULL;
+    struct pam_auth_dp_req *pdp_req;
 
-    preq = talloc_get_type(ptr, struct pam_auth_req);
+    pdp_req = talloc_get_type(ptr, struct pam_auth_dp_req);
+    preq = pdp_req->preq;
+    talloc_free(pdp_req);
 
     dbus_error_init(&dbus_error);
-
     msg = dbus_pending_call_steal_reply(pending);
+
+    /* Check if the client still exists. If not, simply free all the resources
+     * and quit */
+    if (preq == NULL) {
+        DEBUG(SSSDBG_MINOR_FAILURE, ("Client already disconnected\n"));
+        dbus_pending_call_unref(pending);
+        dbus_message_unref(msg);
+        return;
+    }
+
+    /* Sanity-check of message validity */
     if (msg == NULL) {
         DEBUG(0, ("Severe error. A reply callback was called but no reply was"
                   "received and no timeout occurred\n"));
@@ -51,7 +64,6 @@ static void pam_dp_process_reply(DBusPendingCall *pending, void *ptr)
         goto done;
     }
 
-
     type = dbus_message_get_type(msg);
     switch (type) {
         case DBUS_MESSAGE_TYPE_METHOD_RETURN:
@@ -79,6 +91,16 @@ done:
     preq->callback(preq);
 }
 
+static int pdp_req_destructor(struct pam_auth_dp_req *pdp_req)
+{
+    if (pdp_req && pdp_req->preq) {
+        /* If there is still a client waiting, reset the
+         * spy */
+        pdp_req->preq->dpreq_spy = NULL;
+    }
+    return 0;
+}
+
 int pam_dp_send_req(struct pam_auth_req *preq, int timeout)
 {
     struct pam_data *pd = preq->pd;
@@ -86,6 +108,7 @@ int pam_dp_send_req(struct pam_auth_req *preq, int timeout)
     DBusMessage *msg;
     dbus_bool_t ret;
     int res;
+    struct pam_auth_dp_req *pdp_req;
 
     /* double check dp_ctx has actually been initialized.
      * in some pathological cases it may happen that nss starts up before
@@ -118,9 +141,17 @@ int pam_dp_send_req(struct pam_auth_req *preq, int timeout)
         return EIO;
     }
 
+    pdp_req = talloc(preq->cctx->rctx, struct pam_auth_dp_req);
+    if (pdp_req == NULL) {
+        return ENOMEM;
+    }
+    pdp_req->preq = preq;
+    preq->dpreq_spy = pdp_req;
+    talloc_set_destructor(pdp_req, pdp_req_destructor);
+
     res = sbus_conn_send(be_conn->conn, msg,
                          timeout, pam_dp_process_reply,
-                         preq, NULL);
+                         pdp_req, NULL);
     dbus_message_unref(msg);
     return res;
 }