Proper fix for memory handling problem.

sbus_message_handler is not responsible anymore for sending
back data in any case.
Transfer this responsibility to the handler function called.
This way both synchronous and asynchronous funstions use the
interface the same way and can properly free memory referenced
by the reply after the send buffer has been filled in and all
copies are done in sbus_conn_send_reply()

1 files changed, 44 insertions, 24 deletions

diff --git a/server/infopipe/infopipe.c b/server/infopipe/infopipe.c
index dc7abc54..a4ab5e9e 100644
--- a/server/infopipe/infopipe.c
+++ b/server/infopipe/infopipe.c
@@ -39,7 +39,7 @@ struct infp_ctx {
     struct sysbus_ctx *sysbus;
 };
 
-static int service_identity(DBusMessage *message, void *data, DBusMessage **r)
+static int service_identity(DBusMessage *message, struct sbus_conn_ctx *sconn)
 {
     dbus_uint16_t version = INFOPIPE_VERSION;
     const char *name = INFOPIPE_SERVICE_NAME;
@@ -49,41 +49,54 @@ static int service_identity(DBusMessage *message, void *data, DBusMessage **r)
     DEBUG(4, ("Sending identity data [%s,%d]\n", name, version));
 
     reply = dbus_message_new_method_return(message);
+    if (!reply) return ENOMEM;
+
     ret = dbus_message_append_args(reply,
                                    DBUS_TYPE_STRING, &name,
                                    DBUS_TYPE_UINT16, &version,
                                    DBUS_TYPE_INVALID);
     if (!ret) {
+        dbus_message_unref(reply);
         return EIO;
     }
 
-    *r = reply;
+    /* send reply back */
+    sbus_conn_send_reply(sconn, reply);
+    dbus_message_unref(reply);
+
     return EOK;
 }
 
-static int service_pong(DBusMessage *message, void *data, DBusMessage **r)
+static int service_pong(DBusMessage *message, struct sbus_conn_ctx *sconn)
 {
     DBusMessage *reply;
     dbus_bool_t ret;
 
     reply = dbus_message_new_method_return(message);
+    if (!reply) return ENOMEM;
+
     ret = dbus_message_append_args(reply, DBUS_TYPE_INVALID);
     if (!ret) {
+        dbus_message_unref(reply);
         return EIO;
     }
 
-    *r = reply;
+    /* send reply back */
+    sbus_conn_send_reply(sconn, reply);
+    dbus_message_unref(reply);
+
     return EOK;
 }
 
-static int service_reload(DBusMessage *message, void *data, DBusMessage **r) {
+static int service_reload(DBusMessage *message, struct sbus_conn_ctx *sconn)
+{
     /* Monitor calls this function when we need to reload
      * our configuration information. Perform whatever steps
      * are needed to update the configuration objects.
      */
 
     /* Send an empty reply to acknowledge receipt */
-    return service_pong(message, data, r);
+    return service_pong(message, sconn);
 }
 
 struct sbus_method mon_sbus_methods[] = {
@@ -140,12 +153,11 @@ struct sbus_method infp_methods[] = {
 
 #define INTROSPECT_CHUNK_SIZE 4096 /* Read in one memory page at a time */
 
-int infp_introspect(DBusMessage *message, void *data, DBusMessage **r)
+int infp_introspect(DBusMessage *message, struct sbus_conn_ctx *sconn)
 {
-    struct sbus_message_handler_ctx *mh_ctx;
     DBusMessage *reply;
     FILE *xml_stream;
-    char *introspect_xml;
+    char *introspect_xml = NULL;
     char *chunk;
     TALLOC_CTX *tmp_ctx;
     unsigned long xml_size;
@@ -153,14 +165,13 @@ int infp_introspect(DBusMessage *message, void *data, DBusMessage **r)
     int ret;
     dbus_bool_t dbret;
 
-    mh_ctx = talloc_get_type(data, struct sbus_message_handler_ctx);
-
     tmp_ctx = talloc_new(NULL);
     if(tmp_ctx == NULL) {
         return ENOMEM;
     }
 
-    if (mh_ctx->introspection_xml == NULL) {
+    /* currently always null, to be retrieved form a private pointer later */
+    if (introspect_xml == NULL) {
         /* Read in the Introspection XML the first time */
         xml_stream = fopen(SSSD_INTROSPECT_PATH"/"INFP_INTROSPECT_XML, "r");
         if(xml_stream == NULL) {
@@ -189,12 +200,7 @@ int infp_introspect(DBusMessage *message, void *data, DBusMessage **r)
         introspect_xml[xml_size] = '\0';
         talloc_free(chunk);
 
-        /* Store the instrospection XML for future calls */
-        mh_ctx->introspection_xml = introspect_xml;
-    }
-    else {
-        /* Subsequent calls should just reuse the saved value */
-        introspect_xml = mh_ctx->introspection_xml;
+        /* TODO: Store the instrospection XML for future calls */
     }
 
     /* Return the Introspection XML */
@@ -211,6 +217,10 @@ int infp_introspect(DBusMessage *message, void *data, DBusMessage **r)
         goto done;
     }
 
+    /* send reply back */
+    sbus_conn_send_reply(sconn, reply);
+    dbus_message_unref(reply);
+
     DEBUG(9, ("%s\n", introspect_xml));
     ret = EOK;
 
@@ -346,9 +356,8 @@ bool infp_get_permissions(const char *username,
 /* CheckPermissions(STRING domain, STRING object, STRING instance
  *                  ARRAY(STRING action_type, STRING attribute) actions)
  */
-int infp_check_permissions(DBusMessage *message, void *data, DBusMessage **r)
+int infp_check_permissions(DBusMessage *message, struct sbus_conn_ctx *sconn)
 {
-    struct sbus_message_handler_ctx *mh_ctx;
     DBusMessage *reply;
     TALLOC_CTX *tmp_ctx;
     int current_type;
@@ -371,18 +380,17 @@ int infp_check_permissions(DBusMessage *message, void *data, DBusMessage **r)
     dbus_bool_t *permissions;
     size_t count;
 
-    mh_ctx = talloc_get_type(data, struct sbus_message_handler_ctx);
-
     tmp_ctx = talloc_new(NULL);
     if(tmp_ctx == NULL) {
         return ENOMEM;
     }
 
     /* Get the connection UID */
-    conn = sbus_get_connection(mh_ctx->conn_ctx);
+    conn = sbus_get_connection(sconn);
     conn_name = dbus_message_get_sender(message);
     if (conn_name == NULL) {
         DEBUG(0, ("Critical error: D-BUS client has no unique name\n"));
+        talloc_free(tmp_ctx);
         return EIO;
     }
     dbus_error_init(&error);
@@ -390,11 +398,13 @@ int infp_check_permissions(DBusMessage *message, void *data, DBusMessage **r)
     if (uid == -1) {
         DEBUG(0, ("Could not identify unix user. Error message was '%s:%s'\n", error.name, error.message));
         dbus_error_free(&error);
+        talloc_free(tmp_ctx);
         return EIO;
     }
     username = get_username_from_uid(tmp_ctx, uid);
     if (username == NULL) {
         DEBUG(0, ("No username matched the connected UID\n"));
+        talloc_free(tmp_ctx);
         return EIO;
     }
 
@@ -500,17 +510,27 @@ int infp_check_permissions(DBusMessage *message, void *data, DBusMessage **r)
 
     /* Create response message */
     reply = dbus_message_new_method_return(message);
-    if (reply == NULL) return ENOMEM;
+    if (reply == NULL) {
+        talloc_free(tmp_ctx);
+        return ENOMEM;
+    }
 
     dbus_message_append_args(reply,
                              DBUS_TYPE_ARRAY, DBUS_TYPE_BOOLEAN, &permissions, count,
                              DBUS_TYPE_INVALID);
 
+    /* send reply back */
+    sbus_conn_send_reply(sconn, reply);
+    dbus_message_unref(reply);
+
     talloc_free(tmp_ctx);
     return EOK;
 
 einval:
     reply = dbus_message_new_error(message, DBUS_ERROR_INVALID_ARGS, einval_msg);
+    sbus_conn_send_reply(sconn, reply);
+    dbus_message_unref(reply);
+
     talloc_free(tmp_ctx);
     return EOK;
 }