Properly handle dbus send attempts on a closed connection

dbus_connection_send_with_reply() will report success and return
a NULL pending_reply when the connection is not open for
communication. This patch creates a new wrapper around
dbus_connection_send_with_reply() to properly detect this
condition and report it as an error.

7 files changed, 109 insertions, 133 deletions

diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 5dff8922..5a86e553 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -650,16 +650,16 @@ static void reload_reply(DBusPendingCall *pending, void *data)
          */
         DEBUG(0, ("A reply callback was called but no reply was received"
                   " and no timeout occurred\n"));
-
         /* Destroy this connection */
         sbus_disconnect(svc->conn);
-        goto done;
+        dbus_pending_call_unref(pending);
+        return;
     }
 
     /* TODO: Handle cases where the call has timed out or returned
      * with an error.
      */
-done:
+
     dbus_pending_call_unref(pending);
     dbus_message_unref(reply);
 }
@@ -687,9 +687,6 @@ static int monitor_update_resolv(struct config_file_ctx *file_ctx,
 static int service_signal(struct mt_svc *svc, const char *svc_signal)
 {
     DBusMessage *msg;
-    dbus_bool_t dbret;
-    DBusConnection *dbus_conn;
-    DBusPendingCall *pending_reply;
 
     if (svc->provider && strcasecmp(svc->provider, "local") == 0) {
         /* The local provider requires no signaling */
@@ -705,7 +702,6 @@ static int service_signal(struct mt_svc *svc, const char *svc_signal)
         return EIO;
     }
 
-    dbus_conn = sbus_get_connection(svc->conn);
     msg = dbus_message_new_method_call(NULL,
                                        MONITOR_PATH,
                                        MONITOR_INTERFACE,
@@ -717,21 +713,9 @@ static int service_signal(struct mt_svc *svc, const char *svc_signal)
         return ENOMEM;
     }
 
-    dbret = dbus_connection_send_with_reply(dbus_conn, msg, &pending_reply,
-                                            svc->mt_ctx->service_id_timeout);
-    if (!dbret || pending_reply == NULL) {
-        DEBUG(0, ("D-BUS send failed.\n"));
-        dbus_message_unref(msg);
-        monitor_kill_service(svc);
-        talloc_free(svc);
-        return EIO;
-    }
-
-    /* Set up the reply handler */
-    dbus_pending_call_set_notify(pending_reply, reload_reply, svc, NULL);
-    dbus_message_unref(msg);
-
-    return EOK;
+    return sbus_conn_send(svc->conn, msg,
+                          svc->mt_ctx->service_id_timeout,
+                          reload_reply, svc, NULL);
 }
 
 static int service_signal_dns_reload(struct mt_svc *svc)
@@ -1859,9 +1843,6 @@ static int monitor_service_init(struct sbus_connection *conn, void *data)
 static int service_send_ping(struct mt_svc *svc)
 {
     DBusMessage *msg;
-    DBusPendingCall *pending_reply;
-    DBusConnection *dbus_conn;
-    dbus_bool_t dbret;
 
     if (!svc->conn) {
         DEBUG(8, ("Service not yet initialized\n"));
@@ -1870,8 +1851,6 @@ static int service_send_ping(struct mt_svc *svc)
 
     DEBUG(4,("Pinging %s\n", svc->name));
 
-    dbus_conn = sbus_get_connection(svc->conn);
-
     /*
      * Set up identity request
      * This should be a well-known path and method
@@ -1887,24 +1866,9 @@ static int service_send_ping(struct mt_svc *svc)
         return ENOMEM;
     }
 
-    dbret = dbus_connection_send_with_reply(dbus_conn, msg, &pending_reply,
-                                            svc->mt_ctx->service_id_timeout);
-    if (!dbret || pending_reply == NULL) {
-        /*
-         * Critical Failure
-         * We can't communicate on this connection
-         * We'll drop it using the default destructor.
-         */
-        DEBUG(0, ("D-BUS send failed.\n"));
-        talloc_zfree(svc->conn);
-        return EIO;
-    }
-
-    /* Set up the reply handler */
-    dbus_pending_call_set_notify(pending_reply, ping_check, svc, NULL);
-    dbus_message_unref(msg);
-
-    return EOK;
+    return sbus_conn_send(svc->conn, msg,
+                          svc->mt_ctx->service_id_timeout,
+                          ping_check, svc, NULL);
 }
 
 static void ping_check(DBusPendingCall *pending, void *data)
diff --git a/src/monitor/monitor_sbus.c b/src/monitor/monitor_sbus.c
index d60a087e..eedb60b3 100644
--- a/src/monitor/monitor_sbus.c
+++ b/src/monitor/monitor_sbus.c
@@ -109,13 +109,9 @@ done:
 int monitor_common_send_id(struct sbus_connection *conn,
                            const char *name, uint16_t version)
 {
-    DBusPendingCall *pending_reply;
-    DBusConnection *dbus_conn;
     DBusMessage *msg;
     dbus_bool_t ret;
 
-    dbus_conn = sbus_get_connection(conn);
-
     /* create the message */
     msg = dbus_message_new_method_call(NULL,
                                        MON_SRV_PATH,
@@ -137,24 +133,9 @@ int monitor_common_send_id(struct sbus_connection *conn,
         return EIO;
     }
 
-    ret = dbus_connection_send_with_reply(dbus_conn, msg, &pending_reply,
-                                          30000 /* TODO: set timeout */);
-    if (!ret || !pending_reply) {
-        /*
-         * Critical Failure
-         * We can't communicate on this connection
-         * We'll drop it using the default destructor.
-         */
-        DEBUG(0, ("D-BUS send failed.\n"));
-        dbus_message_unref(msg);
-        return EIO;
-    }
-
-    /* Set up the reply handler */
-    dbus_pending_call_set_notify(pending_reply, id_callback, NULL, NULL);
-    dbus_message_unref(msg);
-
-    return EOK;
+    return sbus_conn_send(conn, msg, 3000,
+                          id_callback,
+                          NULL, NULL);
 }
 
 int monitor_common_pong(DBusMessage *message,
diff --git a/src/providers/dp_auth_util.c b/src/providers/dp_auth_util.c
index 52911769..e78884aa 100644
--- a/src/providers/dp_auth_util.c
+++ b/src/providers/dp_auth_util.c
@@ -336,13 +336,9 @@ done:
 int dp_common_send_id(struct sbus_connection *conn, uint16_t version,
                       const char *name)
 {
-    DBusPendingCall *pending_reply;
-    DBusConnection *dbus_conn;
     DBusMessage *msg;
     dbus_bool_t ret;
 
-    dbus_conn = sbus_get_connection(conn);
-
     /* create the message */
     msg = dbus_message_new_method_call(NULL,
                                        DP_PATH,
@@ -365,23 +361,6 @@ int dp_common_send_id(struct sbus_connection *conn, uint16_t version,
         return EIO;
     }
 
-    ret = dbus_connection_send_with_reply(dbus_conn, msg, &pending_reply,
-                                          30000 /* TODO: set timeout */);
-    if (!ret || !pending_reply) {
-        /*
-         * Critical Failure
-         * We can't communicate on this connection
-         * We'll drop it using the default destructor.
-         */
-        DEBUG(0, ("D-BUS send failed.\n"));
-        dbus_message_unref(msg);
-        return EIO;
-    }
-
-    /* Set up the reply handler */
-    dbus_pending_call_set_notify(pending_reply, id_callback, NULL, NULL);
-    dbus_message_unref(msg);
-
-    return EOK;
+    return sbus_conn_send(conn, msg, 30000, id_callback, NULL, NULL);
 }
 
diff --git a/src/responder/common/responder_dp.c b/src/responder/common/responder_dp.c
index d5a4ca76..001661ca 100644
--- a/src/responder/common/responder_dp.c
+++ b/src/responder/common/responder_dp.c
@@ -412,7 +412,6 @@ static int sss_dp_send_acct_req_create(struct resp_ctx *rctx,
                                        void *callback_ctx,
                                        struct sss_dp_req **ndp)
 {
-    DBusConnection *dbus_conn;
     DBusMessage *msg;
     DBusPendingCall *pending_reply;
     dbus_bool_t dbret;
@@ -432,7 +431,6 @@ static int sss_dp_send_acct_req_create(struct resp_ctx *rctx,
                   " This maybe a bug, it shouldn't happen!\n", domain));
         return EIO;
     }
-    dbus_conn = sbus_get_connection(be_conn->conn);
 
     /* create the message */
     msg = dbus_message_new_method_call(NULL,
@@ -457,9 +455,16 @@ static int sss_dp_send_acct_req_create(struct resp_ctx *rctx,
         return EIO;
     }
 
-    dbret = dbus_connection_send_with_reply(dbus_conn, msg,
-                                            &pending_reply, timeout);
-    if (!dbret || pending_reply == NULL) {
+    sdp_req = talloc_zero(rctx, struct sss_dp_req);
+    if (!sdp_req) {
+        dbus_message_unref(msg);
+        return ENOMEM;
+    }
+
+    ret = sbus_conn_send(be_conn->conn, msg, timeout,
+                         sss_dp_send_acct_callback,
+                         sdp_req, &pending_reply);
+    if (ret != EOK) {
         /*
          * Critical Failure
          * We can't communicate on this connection
@@ -470,11 +475,6 @@ static int sss_dp_send_acct_req_create(struct resp_ctx *rctx,
         return EIO;
     }
 
-    sdp_req = talloc_zero(rctx, struct sss_dp_req);
-    if (!sdp_req) {
-        dbus_message_unref(msg);
-        return ENOMEM;
-    }
     sdp_req->ev = rctx->ev;
     sdp_req->pending_reply = pending_reply;
 
@@ -493,18 +493,6 @@ static int sss_dp_send_acct_req_create(struct resp_ctx *rctx,
         talloc_set_destructor((TALLOC_CTX *)cb, sss_dp_callback_destructor);
     }
 
-    /* Set up the reply handler */
-    dbret = dbus_pending_call_set_notify(pending_reply,
-                                         sss_dp_send_acct_callback,
-                                         sdp_req, NULL);
-    if (!dbret) {
-        DEBUG(0, ("Could not queue up pending request!\n"));
-        talloc_zfree(sdp_req);
-        dbus_pending_call_cancel(pending_reply);
-        dbus_message_unref(msg);
-        return EIO;
-    }
-
     dbus_message_unref(msg);
 
     *ndp = sdp_req;
diff --git a/src/responder/pam/pamsrv_dp.c b/src/responder/pam/pamsrv_dp.c
index 071d09b8..d9431f22 100644
--- a/src/responder/pam/pamsrv_dp.c
+++ b/src/responder/pam/pamsrv_dp.c
@@ -43,10 +43,10 @@ static void pam_dp_process_reply(DBusPendingCall *pending, void *ptr)
 
     dbus_error_init(&dbus_error);
 
-    dbus_pending_call_block(pending);
     msg = dbus_pending_call_steal_reply(pending);
     if (msg == NULL) {
-        DEBUG(0, ("Severe error. A reply callback was called but no reply was received and no timeout occurred\n"));
+        DEBUG(0, ("Severe error. A reply callback was called but no reply was"
+                  "received and no timeout occurred\n"));
         preq->pd->pam_status = PAM_SYSTEM_ERR;
         goto done;
     }
@@ -84,8 +84,6 @@ int pam_dp_send_req(struct pam_auth_req *preq, int timeout)
     struct pam_data *pd = preq->pd;
     struct be_conn *be_conn;
     DBusMessage *msg;
-    DBusPendingCall *pending_reply;
-    DBusConnection *dbus_conn;
     dbus_bool_t ret;
     int res;
 
@@ -100,7 +98,6 @@ int pam_dp_send_req(struct pam_auth_req *preq, int timeout)
                   " This maybe a bug, it shouldn't happen!\n", preq->domain));
         return EIO;
     }
-    dbus_conn = sbus_get_connection(be_conn->conn);
 
     msg = dbus_message_new_method_call(NULL,
                                        DP_PATH,
@@ -121,22 +118,8 @@ int pam_dp_send_req(struct pam_auth_req *preq, int timeout)
         return EIO;
     }
 
-    ret = dbus_connection_send_with_reply(dbus_conn, msg, &pending_reply, timeout);
-    if (!ret || pending_reply == NULL) {
-        /*
-         * Critical Failure
-         * We can't communicate on this connection
-         * We'll drop it using the default destructor.
-         */
-        DEBUG(0, ("D-BUS send failed.\n"));
-        dbus_message_unref(msg);
-        return EIO;
-    }
-
-    dbus_pending_call_set_notify(pending_reply,
-                                 pam_dp_process_reply, preq, NULL);
-    dbus_message_unref(msg);
-
-    return EOK;
+    return sbus_conn_send(be_conn->conn, msg,
+                          timeout, pam_dp_process_reply,
+                          preq, NULL);
 }
 
diff --git a/src/sbus/sssd_dbus.h b/src/sbus/sssd_dbus.h
index ac02c444..2dbf4ab7 100644
--- a/src/sbus/sssd_dbus.h
+++ b/src/sbus/sssd_dbus.h
@@ -144,6 +144,22 @@ DBusHandlerResult sbus_message_handler(DBusConnection *conn,
                                   DBusMessage *message,
                                   void *user_data);
 
+/*
+ * Send a message across the SBUS
+ * If requested, the DBusPendingCall object will
+ * be returned to the caller.
+ *
+ * This function will return EAGAIN in the event
+ * that the connection is not open for
+ * communication.
+ */
+int sbus_conn_send(struct sbus_connection *conn,
+                   DBusMessage *msg,
+                   int timeout_ms,
+                   DBusPendingCallNotifyFunction reply_handler,
+                   void *pvt,
+                   DBusPendingCall **pending);
+
 void sbus_conn_send_reply(struct sbus_connection *conn,
                           DBusMessage *reply);
 
diff --git a/src/sbus/sssd_dbus_connection.c b/src/sbus/sssd_dbus_connection.c
index 38ccc6ab..d2918fbc 100644
--- a/src/sbus/sssd_dbus_connection.c
+++ b/src/sbus/sssd_dbus_connection.c
@@ -685,6 +685,71 @@ bool sbus_conn_disconnecting(struct sbus_connection *conn)
     return false;
 }
 
+/*
+ * Send a message across the SBUS
+ * If requested, the DBusPendingCall object will
+ * be returned to the caller.
+ *
+ * This function will return EAGAIN in the event
+ * that the connection is not open for
+ * communication.
+ */
+int sbus_conn_send(struct sbus_connection *conn,
+                   DBusMessage *msg,
+                   int timeout_ms,
+                   DBusPendingCallNotifyFunction reply_handler,
+                   void *pvt,
+                   DBusPendingCall **pending)
+{
+    DBusPendingCall *pending_reply;
+    DBusConnection *dbus_conn;
+    dbus_bool_t dbret;
+
+    dbus_conn = sbus_get_connection(conn);
+
+    dbret = dbus_connection_send_with_reply(dbus_conn, msg,
+                                            &pending_reply,
+                                            timeout_ms);
+    if (!dbret) {
+        /*
+         * Critical Failure
+         * Insufficient memory to send message
+         */
+        DEBUG(0, ("D-BUS send failed.\n"));
+        return ENOMEM;
+    }
+
+    if (pending_reply) {
+        /* Set up the reply handler */
+        dbret = dbus_pending_call_set_notify(pending_reply, reply_handler,
+                                             pvt, NULL);
+        if (!dbret) {
+            /*
+             * Critical Failure
+             * Insufficient memory to create pending call notify
+             */
+            DEBUG(0, ("D-BUS send failed.\n"));
+            dbus_pending_call_cancel(pending_reply);
+            dbus_pending_call_unref(pending_reply);
+            return ENOMEM;
+        }
+
+        if(pending) {
+            *pending = pending_reply;
+        }
+        return EOK;
+    }
+
+    /* If pending_reply is NULL, the connection was not
+     * open for sending.
+     */
+
+    /* TODO: Create a callback into the reconnection logic so this
+     * request is invoked when the connection is re-established
+     */
+    return EAGAIN;
+}
+
 void sbus_conn_send_reply(struct sbus_connection *conn, DBusMessage *reply)
 {
     dbus_connection_send(conn->dbus.conn, reply, NULL);