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()

5 files changed, 157 insertions, 59 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;
 }
diff --git a/server/infopipe/infopipe.h b/server/infopipe/infopipe.h
index ed5092ed..6e6b7583 100644
--- a/server/infopipe/infopipe.h
+++ b/server/infopipe/infopipe.h
@@ -47,13 +47,13 @@
  * org.freedesktop.DBus.Introspectable interface
  */
 #define INFP_INTROSPECT "Introspect"
-int infp_introspect(DBusMessage *message, void *data, DBusMessage **r);
+int infp_introspect(DBusMessage *message, struct sbus_conn_ctx *sconn);
 
 /**********************************************************
  * Permission Methods (from infopipe.c)                   *
  **********************************************************/
 #define INFP_CHECK_PERMISSIONS "CheckPermissions1"
-int infp_check_permissions(DBusMessage *message, void *data, DBusMessage **r);
+int infp_check_permissions(DBusMessage *message, struct sbus_conn_ctx *sconn);
 
 #define INFP_PERMISSION_METHODS \
     {INFP_CHECK_PERMISSIONS,infp_check_permissions},
@@ -62,22 +62,22 @@ int infp_check_permissions(DBusMessage *message, void *data, DBusMessage **r);
  * User Methods (from infopipe_users.c)                   *
  **********************************************************/
 #define INFP_USERS_GET_CACHED "GetCachedUsers1"
-int infp_users_get_cached(DBusMessage *message, void *data, DBusMessage **r);
+int infp_users_get_cached(DBusMessage *message, struct sbus_conn_ctx *sconn);
 
 #define INFP_USERS_CREATE "CreateUser1"
-int infp_users_create(DBusMessage *message, void *data, DBusMessage **r);
+int infp_users_create(DBusMessage *message, struct sbus_conn_ctx *sconn);
 
 #define INFP_USERS_DELETE "DeleteUser1"
-int infp_users_delete(DBusMessage *message, void *data, DBusMessage **r);
+int infp_users_delete(DBusMessage *message, struct sbus_conn_ctx *sconn);
 
 #define INFP_USERS_GET_ATTR "GetUserAttributes1"
-int infp_users_get_attr(DBusMessage *message, void *data, DBusMessage **r);
+int infp_users_get_attr(DBusMessage *message, struct sbus_conn_ctx *sconn);
 
 #define INFP_USERS_SET_ATTR "SetUserAttributes1"
-int infp_users_set_attr(DBusMessage *message, void *data, DBusMessage **r);
+int infp_users_set_attr(DBusMessage *message, struct sbus_conn_ctx *sconn);
 
 #define INFP_USERS_SET_UID "Set_YouReallyDoNotWantToUseThisFunction_UserUID1"
-int infp_users_set_uid(DBusMessage *message, void *data, DBusMessage **r);
+int infp_users_set_uid(DBusMessage *message, struct sbus_conn_ctx *sconn);
 
 #define INFP_USER_METHODS \
     {INFP_USERS_GET_CACHED, infp_users_get_cached}, \
@@ -92,19 +92,19 @@ int infp_users_set_uid(DBusMessage *message, void *data, DBusMessage **r);
  **********************************************************/
 
 #define INFP_GROUPS_CREATE "CreateGroup1"
-int infp_groups_create(DBusMessage *message, void *data, DBusMessage **r);
+int infp_groups_create(DBusMessage *message, struct sbus_conn_ctx *sconn);
 
 #define INFP_GROUPS_DELETE "DeleteGroup1"
-int infp_groups_delete(DBusMessage *message, void *data, DBusMessage **r);
+int infp_groups_delete(DBusMessage *message, struct sbus_conn_ctx *sconn);
 
 #define INFP_GROUPS_ADD_MEMBERS "AddGroupMembers1"
-int infp_groups_add_members(DBusMessage *message, void *data, DBusMessage **r);
+int infp_groups_add_members(DBusMessage *message, struct sbus_conn_ctx *sconn);
 
 #define INFP_GROUPS_REMOVE_MEMBERS "RemoveGroupMembers1"
-int infp_groups_remove_members(DBusMessage *message, void *data, DBusMessage **r);
+int infp_groups_remove_members(DBusMessage *message, struct sbus_conn_ctx *sconn);
 
 #define INFP_GROUPS_SET_GID "Set_YouReallyDoNotWantToUseThisFunction_GroupGID1"
-int infp_groups_set_gid(DBusMessage *message, void *data, DBusMessage **r);
+int infp_groups_set_gid(DBusMessage *message, struct sbus_conn_ctx *sconn);
 
 #define INFP_GROUP_METHODS \
     {INFP_GROUPS_CREATE, infp_groups_create}, \
diff --git a/server/infopipe/infopipe_groups.c b/server/infopipe/infopipe_groups.c
index 4b9cfc22..7ec85296 100644
--- a/server/infopipe/infopipe_groups.c
+++ b/server/infopipe/infopipe_groups.c
@@ -23,32 +23,67 @@
 #include "util/util.h"
 #include "infopipe.h"
 
-int infp_groups_create(DBusMessage *message, void *data, DBusMessage **r)
+int infp_groups_create(DBusMessage *message, struct sbus_conn_ctx *sconn)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    DBusMessage *reply;
+
+    reply = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+
+    /* send reply */
+    sbus_conn_send_reply(sconn, reply);
+
+    dbus_message_unref(reply);
     return EOK;
 }
 
-int infp_groups_delete(DBusMessage *message, void *data, DBusMessage **r)
+int infp_groups_delete(DBusMessage *message, struct sbus_conn_ctx *sconn)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    DBusMessage *reply;
+
+    reply = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+
+    /* send reply */
+    sbus_conn_send_reply(sconn, reply);
+
+    dbus_message_unref(reply);
     return EOK;
 }
 
-int infp_groups_add_members(DBusMessage *message, void *data, DBusMessage **r)
+int infp_groups_add_members(DBusMessage *message, struct sbus_conn_ctx *sconn)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    DBusMessage *reply;
+
+    reply = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+
+    /* send reply */
+    sbus_conn_send_reply(sconn, reply);
+
+    dbus_message_unref(reply);
     return EOK;
 }
 
-int infp_groups_remove_members(DBusMessage *message, void *data, DBusMessage **r)
+int infp_groups_remove_members(DBusMessage *message, struct sbus_conn_ctx *sconn)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    DBusMessage *reply;
+
+    reply = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+
+    /* send reply */
+    sbus_conn_send_reply(sconn, reply);
+
+    dbus_message_unref(reply);
     return EOK;
 }
 
-int infp_groups_set_gid(DBusMessage *message, void *data, DBusMessage **r)
+int infp_groups_set_gid(DBusMessage *message, struct sbus_conn_ctx *sconn)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    DBusMessage *reply;
+
+    reply = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+
+    /* send reply */
+    sbus_conn_send_reply(sconn, reply);
+
+    dbus_message_unref(reply);
     return EOK;
 }
diff --git a/server/infopipe/infopipe_users.c b/server/infopipe/infopipe_users.c
index 632e624f..2c107bc4 100644
--- a/server/infopipe/infopipe_users.c
+++ b/server/infopipe/infopipe_users.c
@@ -22,38 +22,80 @@
 #include "util/util.h"
 #include "infopipe.h"
 
-int infp_users_get_cached(DBusMessage *message, void *data, DBusMessage **r)
+int infp_users_get_cached(DBusMessage *message, struct sbus_conn_ctx *sconn)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    DBusMessage *reply;
+
+    reply = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+
+    /* send reply */
+    sbus_conn_send_reply(sconn, reply);
+
+    dbus_message_unref(reply);
     return EOK;
 }
 
-int infp_users_create(DBusMessage *message, void *data, DBusMessage **r)
+int infp_users_create(DBusMessage *message, struct sbus_conn_ctx *sconn)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    DBusMessage *reply;
+
+    reply = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+
+    /* send reply */
+    sbus_conn_send_reply(sconn, reply);
+
+    dbus_message_unref(reply);
     return EOK;
 }
 
-int infp_users_delete(DBusMessage *message, void *data, DBusMessage **r)
+int infp_users_delete(DBusMessage *message, struct sbus_conn_ctx *sconn)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    DBusMessage *reply;
+
+    reply = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+
+    /* send reply */
+    sbus_conn_send_reply(sconn, reply);
+
+    dbus_message_unref(reply);
     return EOK;
 }
 
-int infp_users_get_attr(DBusMessage *message, void *data, DBusMessage **r)
+int infp_users_get_attr(DBusMessage *message, struct sbus_conn_ctx *sconn)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    DBusMessage *reply;
+
+    reply = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+
+    /* send reply */
+    sbus_conn_send_reply(sconn, reply);
+
+    dbus_message_unref(reply);
     return EOK;
 }
 
-int infp_users_set_attr(DBusMessage *message, void *data, DBusMessage **r)
+int infp_users_set_attr(DBusMessage *message, struct sbus_conn_ctx *sconn)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    DBusMessage *reply;
+
+    reply = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+
+    /* send reply */
+    sbus_conn_send_reply(sconn, reply);
+
+    dbus_message_unref(reply);
     return EOK;
 }
 
-int infp_users_set_uid(DBusMessage *message, void *data, DBusMessage **r)
+int infp_users_set_uid(DBusMessage *message, struct sbus_conn_ctx *sconn)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    DBusMessage *reply;
+
+    reply = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+
+    /* send reply */
+    sbus_conn_send_reply(sconn, reply);
+
+    dbus_message_unref(reply);
     return EOK;
 }
diff --git a/server/infopipe/sysbus.c b/server/infopipe/sysbus.c
index a6b352fa..5c272e66 100644
--- a/server/infopipe/sysbus.c
+++ b/server/infopipe/sysbus.c
@@ -125,6 +125,7 @@ int sysbus_init(TALLOC_CTX *mem_ctx, struct sysbus_ctx **sysbus,
     if (ret != DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER) {
         /* We were unable to register on the system bus */
         DEBUG(0, ("Unable to request name on the system bus. Error: %s\n", dbus_error.message));
+        if (dbus_error_is_set(&dbus_error)) dbus_error_free(&dbus_error);
         talloc_free(system_bus);
         return EIO;
     }