Fixing serious memory allocation bug in sbus_message_handler.

dbus_message_append_args() adds a reference to memory that is not
copied to the outgoing message until dbus_connection_send() is
called. Since we compile our reply messages in functions and then
return the reply, we need a mechanism for deleting allocated
memory after invoking dbus_connection_send. I have changed the
arguments to sbus_msg_handler_fn so that it takes a talloc ctx
containing the sbus_message_handler_ctx and a pointer to a reply
object. We can now allocate memory as a child of the reply context
and free it after calling dbus_connection_send.

4 files changed, 50 insertions, 53 deletions

diff --git a/server/infopipe/infopipe.c b/server/infopipe/infopipe.c
index 8fe7ee72..1e7a88c8 100644
--- a/server/infopipe/infopipe.c
+++ b/server/infopipe/infopipe.c
@@ -38,17 +38,16 @@ 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_message_ctx *reply)
 {
     dbus_uint16_t version = INFOPIPE_VERSION;
     const char *name = INFOPIPE_SERVICE_NAME;
-    DBusMessage *reply;
     dbus_bool_t ret;
 
     DEBUG(4, ("Sending identity data [%s,%d]\n", name, version));
 
-    reply = dbus_message_new_method_return(message);
-    ret = dbus_message_append_args(reply,
+    reply->reply_message = dbus_message_new_method_return(message);
+    ret = dbus_message_append_args(reply->reply_message,
                                    DBUS_TYPE_STRING, &name,
                                    DBUS_TYPE_UINT16, &version,
                                    DBUS_TYPE_INVALID);
@@ -56,33 +55,30 @@ static int service_identity(DBusMessage *message, void *data, DBusMessage **r)
         return EIO;
     }
 
-    *r = reply;
     return EOK;
 }
 
-static int service_pong(DBusMessage *message, void *data, DBusMessage **r)
+static int service_pong(DBusMessage *message, struct sbus_message_ctx *reply)
 {
-    DBusMessage *reply;
     dbus_bool_t ret;
 
-    reply = dbus_message_new_method_return(message);
-    ret = dbus_message_append_args(reply, DBUS_TYPE_INVALID);
+    reply->reply_message = dbus_message_new_method_return(message);
+    ret = dbus_message_append_args(reply->reply_message, DBUS_TYPE_INVALID);
     if (!ret) {
         return EIO;
     }
 
-    *r = reply;
     return EOK;
 }
 
-static int service_reload(DBusMessage *message, void *data, DBusMessage **r) {
+static int service_reload(DBusMessage *message, struct sbus_message_ctx *reply) {
     /* 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, reply);
 }
 
 struct sbus_method mon_sbus_methods[] = {
@@ -136,12 +132,10 @@ struct sbus_method infp_methods[] = {
     { NULL, NULL }
 };
 
-int infp_introspect(DBusMessage *message, void *data, DBusMessage **r)
+int infp_introspect(DBusMessage *message, struct sbus_message_ctx *reply)
 {
-    /* Return the Introspection XML */
-
     /* TODO: actually return the file */
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    reply->reply_message = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
     return EOK;
 }
 
@@ -185,9 +179,9 @@ static int infp_process_init(TALLOC_CTX *mem_ctx,
     return ret;
 }
 
-int infp_check_permissions(DBusMessage *message, void *data, DBusMessage **r)
+int infp_check_permissions(DBusMessage *message, struct sbus_message_ctx *reply)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    reply->reply_message = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
     return EOK;
 }
 
diff --git a/server/infopipe/infopipe.h b/server/infopipe/infopipe.h
index 8492c1b1..aabc2bfa 100644
--- a/server/infopipe/infopipe.h
+++ b/server/infopipe/infopipe.h
@@ -22,6 +22,9 @@
 #ifndef INFOPIPE_H_
 #define INFOPIPE_H_
 
+#include <dbus/dbus.h>
+#include "sbus/sssd_dbus.h"
+
 #define INFOPIPE_DBUS_NAME "org.freeipa.sssd.infopipe1"
 #define INFOPIPE_INTERFACE "org.freeipa.sssd.infopipe1"
 #define INFOPIPE_PATH "/org/freeipa/sssd/infopipe1"
@@ -42,34 +45,34 @@
  * 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_message_ctx *reply);
 
 /**********************************************************
  * 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_message_ctx *reply);
 
 /**********************************************************
  * 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_message_ctx *reply);
 
 #define INFP_USERS_CREATE "CreateUser1"
-int infp_users_create(DBusMessage *message, void *data, DBusMessage **r);
+int infp_users_create(DBusMessage *message, struct sbus_message_ctx *reply);
 
 #define INFP_USERS_DELETE "DeleteUser1"
-int infp_users_delete(DBusMessage *message, void *data, DBusMessage **r);
+int infp_users_delete(DBusMessage *message, struct sbus_message_ctx *reply);
 
 #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_message_ctx *reply);
 
 #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_message_ctx *reply);
 
 #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_message_ctx *reply);
 
 #define INFP_USER_METHODS \
     {INFP_USERS_GET_CACHED, infp_users_get_cached}, \
@@ -84,19 +87,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_message_ctx *reply);
 
 #define INFP_GROUPS_DELETE "DeleteGroup1"
-int infp_groups_delete(DBusMessage *message, void *data, DBusMessage **r);
+int infp_groups_delete(DBusMessage *message, struct sbus_message_ctx *reply);
 
 #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_message_ctx *reply);
 
 #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_message_ctx *reply);
 
 #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_message_ctx *reply);
 
 #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..6aa115f8 100644
--- a/server/infopipe/infopipe_groups.c
+++ b/server/infopipe/infopipe_groups.c
@@ -23,32 +23,32 @@
 #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_message_ctx *reply)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    reply->reply_message = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
     return EOK;
 }
 
-int infp_groups_delete(DBusMessage *message, void *data, DBusMessage **r)
+int infp_groups_delete(DBusMessage *message, struct sbus_message_ctx *reply)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    reply->reply_message = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
     return EOK;
 }
 
-int infp_groups_add_members(DBusMessage *message, void *data, DBusMessage **r)
+int infp_groups_add_members(DBusMessage *message, struct sbus_message_ctx *reply)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    reply->reply_message = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
     return EOK;
 }
 
-int infp_groups_remove_members(DBusMessage *message, void *data, DBusMessage **r)
+int infp_groups_remove_members(DBusMessage *message, struct sbus_message_ctx *reply)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    reply->reply_message = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
     return EOK;
 }
 
-int infp_groups_set_gid(DBusMessage *message, void *data, DBusMessage **r)
+int infp_groups_set_gid(DBusMessage *message, struct sbus_message_ctx *reply)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    reply->reply_message = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
     return EOK;
 }
diff --git a/server/infopipe/infopipe_users.c b/server/infopipe/infopipe_users.c
index 632e624f..566d5e16 100644
--- a/server/infopipe/infopipe_users.c
+++ b/server/infopipe/infopipe_users.c
@@ -22,38 +22,38 @@
 #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_message_ctx *reply)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    reply->reply_message = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
     return EOK;
 }
 
-int infp_users_create(DBusMessage *message, void *data, DBusMessage **r)
+int infp_users_create(DBusMessage *message, struct sbus_message_ctx *reply)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    reply->reply_message = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
     return EOK;
 }
 
-int infp_users_delete(DBusMessage *message, void *data, DBusMessage **r)
+int infp_users_delete(DBusMessage *message, struct sbus_message_ctx *reply)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    reply->reply_message = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
     return EOK;
 }
 
-int infp_users_get_attr(DBusMessage *message, void *data, DBusMessage **r)
+int infp_users_get_attr(DBusMessage *message, struct sbus_message_ctx *reply)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    reply->reply_message = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
     return EOK;
 }
 
-int infp_users_set_attr(DBusMessage *message, void *data, DBusMessage **r)
+int infp_users_set_attr(DBusMessage *message, struct sbus_message_ctx *reply)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    reply->reply_message = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
     return EOK;
 }
 
-int infp_users_set_uid(DBusMessage *message, void *data, DBusMessage **r)
+int infp_users_set_uid(DBusMessage *message, struct sbus_message_ctx *reply)
 {
-    *r = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
+    reply->reply_message = dbus_message_new_error(message, DBUS_ERROR_NOT_SUPPORTED, "Not yet implemented");
     return EOK;
 }