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 3ffc17087..c47d4bb16 100644
--- a/src/responder/pam/pamsrv.h
+++ b/src/responder/pam/pamsrv.h
@@ -38,6 +38,10 @@ struct pam_ctx {
     time_t id_timeout;
 };
 
+struct pam_auth_dp_req {
+    struct pam_auth_req *preq;
+};
+
 struct pam_auth_req {
     struct cli_ctx *cctx;
     struct sss_domain_info *domain;
@@ -49,6 +53,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 b7c6fe2c2..5c614e332 100644
--- a/src/responder/pam/pamsrv_cmd.c
+++ b/src/responder/pam/pamsrv_cmd.c
@@ -697,6 +697,17 @@ static int pam_check_user_search(struct pam_auth_req *preq);
 static int pam_check_user_done(struct pam_auth_req *preq, int ret);
 static void pam_dom_forwarder(struct pam_auth_req *preq);
 
+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;
+}
+
 /* TODO: we should probably return some sort of cookie that is set in the
  * PAM_ENVIRONMENT, so that we can save performing some calls and cache
  * data. */
@@ -717,6 +728,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 = talloc_zero(preq, struct pam_data);
diff --git a/src/responder/pam/pamsrv_dp.c b/src/responder/pam/pamsrv_dp.c
index 40d4f3031..fc6c16008 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(4, ("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;
 }