From 13421cbe0af4343f9d110600755ffa756690b282 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher Date: Mon, 23 Feb 2009 15:43:31 -0500 Subject: 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. --- server/providers/data_provider.c | 46 +++++++++++------------- server/providers/data_provider_be.c | 71 ++++++++++++++++--------------------- 2 files changed, 51 insertions(+), 66 deletions(-) (limited to 'server/providers') diff --git a/server/providers/data_provider.c b/server/providers/data_provider.c index aa66c8e5..c1dc2801 100644 --- a/server/providers/data_provider.c +++ b/server/providers/data_provider.c @@ -76,9 +76,9 @@ struct dp_frontend { static int dp_backend_destructor(void *ctx); static int dp_frontend_destructor(void *ctx); -static int service_identity(DBusMessage *message, void *data, DBusMessage **r); -static int service_pong(DBusMessage *message, void *data, DBusMessage **r); -static int service_reload(DBusMessage *message, void *data, DBusMessage **r); +static int service_identity(DBusMessage *message, struct sbus_message_ctx *reply); +static int service_pong(DBusMessage *message, struct sbus_message_ctx *reply); +static int service_reload(DBusMessage *message, struct sbus_message_ctx *reply); struct sbus_method mon_sbus_methods[] = { { SERVICE_METHOD_IDENTITY, service_identity }, @@ -87,7 +87,7 @@ struct sbus_method mon_sbus_methods[] = { { NULL, NULL } }; -static int dp_get_account_info(DBusMessage *message, void *data, DBusMessage **r); +static int dp_get_account_info(DBusMessage *message, struct sbus_message_ctx *reply); struct sbus_method dp_sbus_methods[] = { { DP_SRV_METHOD_GETACCTINFO, dp_get_account_info }, @@ -108,17 +108,16 @@ struct dp_be_request { struct dp_backend *be; }; -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 = DATA_PROVIDER_VERSION; const char *name = DATA_PROVIDER_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); @@ -126,33 +125,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); } static int dp_monitor_init(struct dp_ctx *dpctx) @@ -538,14 +534,13 @@ static int dp_send_acct_req(struct dp_be_request *bereq, return EOK; } -static int dp_get_account_info(DBusMessage *message, void *data, DBusMessage **r) +static int dp_get_account_info(DBusMessage *message, struct sbus_message_ctx *reply) { struct sbus_message_handler_ctx *smh_ctx; struct dp_client *dpcli; struct dp_be_request *bereq; struct dp_request *dpreq = NULL; struct dp_backend *dpbe; - DBusMessage *reply; DBusError dbus_error; dbus_bool_t dbret; void *user_data; @@ -554,9 +549,11 @@ static int dp_get_account_info(DBusMessage *message, void *data, DBusMessage **r const char *errmsg = NULL; int dpret = 0, ret = 0; - if (!data) return EINVAL; - smh_ctx = talloc_get_type(data, struct sbus_message_handler_ctx); + if (!reply) return EINVAL; + + smh_ctx = reply->mh_ctx; if (!smh_ctx) return EINVAL; + user_data = sbus_conn_get_private_data(smh_ctx->conn_ctx); if (!user_data) return EINVAL; dpcli = talloc_get_type(user_data, struct dp_client); @@ -578,7 +575,7 @@ static int dp_get_account_info(DBusMessage *message, void *data, DBusMessage **r DEBUG(4, ("Got request for [%s][%u][%s][%s]\n", domain, type, attrs, filter)); - reply = dbus_message_new_method_return(message); + reply->reply_message = dbus_message_new_method_return(message); /* search for domain */ if (!domain) { @@ -606,7 +603,7 @@ static int dp_get_account_info(DBusMessage *message, void *data, DBusMessage **r goto respond; } - dpreq->reply = reply; + dpreq->reply = reply->reply_message; dpreq->src_cli = dpcli; dpreq->pending_replies = 0; @@ -662,7 +659,7 @@ static int dp_get_account_info(DBusMessage *message, void *data, DBusMessage **r goto respond; } - dpreq->reply = reply; + dpreq->reply = reply->reply_message; dpreq->src_cli = dpcli; dpreq->pending_replies = 1; @@ -698,14 +695,13 @@ static int dp_get_account_info(DBusMessage *message, void *data, DBusMessage **r return EOK; respond: - dbret = dbus_message_append_args(reply, + dbret = dbus_message_append_args(reply->reply_message, DBUS_TYPE_UINT16, &dpret, DBUS_TYPE_UINT32, &ret, DBUS_TYPE_STRING, &errmsg, DBUS_TYPE_INVALID); if (!dbret) return EIO; - *r = reply; return EOK; } diff --git a/server/providers/data_provider_be.c b/server/providers/data_provider_be.c index 7b491ce7..c12e2b2f 100644 --- a/server/providers/data_provider_be.c +++ b/server/providers/data_provider_be.c @@ -44,8 +44,8 @@ typedef int (*be_init_fn_t)(TALLOC_CTX *, struct be_mod_ops **, void **); -static int service_identity(DBusMessage *message, void *data, DBusMessage **r); -static int service_pong(DBusMessage *message, void *data, DBusMessage **r); +static int service_identity(DBusMessage *message, struct sbus_message_ctx *reply); +static int service_pong(DBusMessage *message, struct sbus_message_ctx *reply); struct sbus_method mon_sbus_methods[] = { { SERVICE_METHOD_IDENTITY, service_identity }, @@ -53,9 +53,9 @@ struct sbus_method mon_sbus_methods[] = { { NULL, NULL } }; -static int be_identity(DBusMessage *message, void *data, DBusMessage **r); -static int be_check_online(DBusMessage *message, void *data, DBusMessage **r); -static int be_get_account_info(DBusMessage *message, void *data, DBusMessage **r); +static int be_identity(DBusMessage *message, struct sbus_message_ctx *reply); +static int be_check_online(DBusMessage *message, struct sbus_message_ctx *reply); +static int be_get_account_info(DBusMessage *message, struct sbus_message_ctx *reply); struct sbus_method be_methods[] = { { DP_CLI_METHOD_IDENTITY, be_identity }, @@ -64,17 +64,16 @@ struct sbus_method be_methods[] = { { NULL, NULL } }; -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 = DATA_PROVIDER_VERSION; struct sbus_message_handler_ctx *smh_ctx; struct be_ctx *ctx; - DBusMessage *reply; dbus_bool_t ret; void *user_data; - if (!data) return EINVAL; - smh_ctx = talloc_get_type(data, struct sbus_message_handler_ctx); + if (!reply) return EINVAL; + smh_ctx = reply->mh_ctx; if (!smh_ctx) return EINVAL; user_data = sbus_conn_get_private_data(smh_ctx->conn_ctx); if (!user_data) return EINVAL; @@ -83,8 +82,8 @@ static int service_identity(DBusMessage *message, void *data, DBusMessage **r) DEBUG(4,("Sending ID reply: (%s,%d)\n", ctx->identity, 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, &ctx->identity, DBUS_TYPE_UINT16, &version, DBUS_TYPE_INVALID); @@ -92,37 +91,33 @@ 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 be_identity(DBusMessage *message, void *data, DBusMessage **r) +static int be_identity(DBusMessage *message, struct sbus_message_ctx *reply) { dbus_uint16_t version = DATA_PROVIDER_VERSION; dbus_uint16_t clitype = DP_CLI_BACKEND; struct sbus_message_handler_ctx *smh_ctx; struct be_ctx *ctx; - DBusMessage *reply; dbus_bool_t ret; void *user_data; - if (!data) return EINVAL; - smh_ctx = talloc_get_type(data, struct sbus_message_handler_ctx); + if (!reply) return EINVAL; + smh_ctx = reply->mh_ctx; if (!smh_ctx) return EINVAL; user_data = sbus_conn_get_private_data(smh_ctx->conn_ctx); if (!user_data) return EINVAL; @@ -132,8 +127,8 @@ static int be_identity(DBusMessage *message, void *data, DBusMessage **r) DEBUG(4,("Sending ID reply: (%d,%d,%s,%s)\n", clitype, version, ctx->name, ctx->domain)); - 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_UINT16, &clitype, DBUS_TYPE_UINT16, &version, DBUS_TYPE_STRING, &ctx->name, @@ -143,7 +138,6 @@ static int be_identity(DBusMessage *message, void *data, DBusMessage **r) return EIO; } - *r = reply; return EOK; } @@ -237,13 +231,12 @@ static void online_chk_callback(struct be_req *req, int status, } -static int be_check_online(DBusMessage *message, void *data, DBusMessage **r) +static int be_check_online(DBusMessage *message, struct sbus_message_ctx *reply) { struct sbus_message_handler_ctx *smh_ctx; struct be_online_req *req; struct be_req *be_req; struct be_ctx *ctx; - DBusMessage *reply; dbus_bool_t dbret; void *user_data; int ret; @@ -252,15 +245,15 @@ static int be_check_online(DBusMessage *message, void *data, DBusMessage **r) dbus_uint32_t err_min; const char *err_msg; - if (!data) return EINVAL; - smh_ctx = talloc_get_type(data, struct sbus_message_handler_ctx); + if (!reply) return EINVAL; + smh_ctx = reply->mh_ctx; if (!smh_ctx) return EINVAL; user_data = sbus_conn_get_private_data(smh_ctx->conn_ctx); if (!user_data) return EINVAL; ctx = talloc_get_type(user_data, struct be_ctx); if (!ctx) return EINVAL; - reply = dbus_message_new_method_return(message); + reply->reply_message = dbus_message_new_method_return(message); /* process request */ be_req = talloc(ctx, struct be_req); @@ -273,7 +266,7 @@ static int be_check_online(DBusMessage *message, void *data, DBusMessage **r) } be_req->be_ctx = ctx; be_req->fn = online_chk_callback; - be_req->pvt = reply; + be_req->pvt = reply->reply_message; req = talloc(be_req, struct be_online_req); if (!req) { @@ -303,7 +296,7 @@ done: talloc_free(be_req); } - dbret = dbus_message_append_args(reply, + dbret = dbus_message_append_args(reply->reply_message, DBUS_TYPE_UINT16, &online, DBUS_TYPE_UINT16, &err_maj, DBUS_TYPE_UINT32, &err_min, @@ -311,7 +304,6 @@ done: DBUS_TYPE_INVALID); if (!dbret) return EIO; - *r = reply; return EOK; } @@ -355,13 +347,12 @@ static void acctinfo_callback(struct be_req *req, int status, talloc_free(req); } -static int be_get_account_info(DBusMessage *message, void *data, DBusMessage **r) +static int be_get_account_info(DBusMessage *message, struct sbus_message_ctx *reply) { struct sbus_message_handler_ctx *smh_ctx; struct be_acct_req *req; struct be_req *be_req; struct be_ctx *ctx; - DBusMessage *reply; DBusError dbus_error; dbus_bool_t dbret; void *user_data; @@ -375,10 +366,9 @@ static int be_get_account_info(DBusMessage *message, void *data, DBusMessage **r const char *err_msg; be_req = NULL; - *r = NULL; - if (!data) return EINVAL; - smh_ctx = talloc_get_type(data, struct sbus_message_handler_ctx); + if (!reply) return EINVAL; + smh_ctx = reply->mh_ctx; if (!smh_ctx) return EINVAL; user_data = sbus_conn_get_private_data(smh_ctx->conn_ctx); if (!user_data) return EINVAL; @@ -399,7 +389,7 @@ static int be_get_account_info(DBusMessage *message, void *data, DBusMessage **r DEBUG(4, ("Got request for [%u][%s][%s]\n", type, attrs, filter)); - reply = dbus_message_new_method_return(message); + reply->reply_message = dbus_message_new_method_return(message); if (attrs) { if (strcmp(attrs, "core") == 0) attr_type = BE_ATTR_CORE; @@ -448,7 +438,7 @@ static int be_get_account_info(DBusMessage *message, void *data, DBusMessage **r } be_req->be_ctx = ctx; be_req->fn = acctinfo_callback; - be_req->pvt = reply; + be_req->pvt = reply->reply_message; req = talloc(be_req, struct be_acct_req); if (!req) { @@ -479,7 +469,7 @@ done: talloc_free(be_req); } - dbret = dbus_message_append_args(reply, + dbret = dbus_message_append_args(reply->reply_message, DBUS_TYPE_UINT16, &err_maj, DBUS_TYPE_UINT32, &err_min, DBUS_TYPE_STRING, &err_msg, @@ -489,7 +479,6 @@ done: DEBUG(4, ("Request processed. Returned %d,%d,%s\n", err_maj, err_min, err_msg)); - *r = reply; return EOK; } -- cgit