summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJakub Hrozek <jhrozek@redhat.com>2012-10-10 21:50:44 +0200
committerJakub Hrozek <jhrozek@redhat.com>2012-10-11 13:44:36 +0200
commit8ba8222afca3026fd67af08e224b1d9e848aceaa (patch)
tree2d92d9477b9564097408e098d45d9189529d42dd
parent89b93a44d1ce24ec208ee244f7e5b1689fc6ff1a (diff)
downloadsssd-8ba8222afca3026fd67af08e224b1d9e848aceaa.tar.gz
sssd-8ba8222afca3026fd67af08e224b1d9e848aceaa.tar.bz2
sssd-8ba8222afca3026fd67af08e224b1d9e848aceaa.zip
Fix memory hierarchy in subdomains discovery
https://fedorahosted.org/sssd/ticket/1571 The patch changes the subdomains discovery to use the tevent_req style. Previously, the code violated several rules which made the code very unreadable and led to memory hierarchy issues and use-after-free errors.
-rw-r--r--src/responder/common/responder_get_domains.c276
1 files changed, 160 insertions, 116 deletions
diff --git a/src/responder/common/responder_get_domains.c b/src/responder/common/responder_get_domains.c
index aff06e63..9e81630f 100644
--- a/src/responder/common/responder_get_domains.c
+++ b/src/responder/common/responder_get_domains.c
@@ -23,128 +23,61 @@
#include "providers/data_provider.h"
#include "db/sysdb.h"
+/* ========== Get subdomains for a domain ================= */
+static DBusMessage *sss_dp_get_domains_msg(void *pvt);
+
struct sss_dp_domains_info {
- struct resp_ctx *rctx;
struct sss_domain_info *dom;
const char *hint;
bool force;
-
- struct sss_dp_req_state *state;
};
-static DBusMessage *sss_dp_get_domains_msg(void *pvt);
-static errno_t get_domains_next(struct tevent_req *req);
-static void sss_dp_get_domains_callback(struct tevent_req *subreq);
-
-static errno_t get_domains_done(struct tevent_req *req);
-static errno_t check_last_request(struct resp_ctx *rctx, const char *hint);
-
-struct tevent_req *sss_dp_get_domains_send(TALLOC_CTX *mem_ctx,
- struct resp_ctx *rctx,
- bool force,
- const char *hint)
+static struct tevent_req *
+get_subdomains_send(TALLOC_CTX *mem_ctx, struct resp_ctx *rctx,
+ struct sss_domain_info *dom,
+ const bool force,
+ const char *hint)
{
errno_t ret;
struct tevent_req *req;
+ struct sss_dp_req_state *state;
struct sss_dp_domains_info *info;
+ char *key;
- req = tevent_req_create(mem_ctx, &info, struct sss_dp_domains_info);
+ req = tevent_req_create(mem_ctx, &state, struct sss_dp_req_state);
if (req == NULL) {
- return NULL;
- }
-
- if (rctx->domains == NULL) {
- DEBUG(SSSDBG_CRIT_FAILURE, ("No domains configured.\n"));
- ret = EINVAL;
- goto immediately;
+ return NULL;
}
- if (!force) {
- ret = check_last_request(rctx, hint);
-
- if (ret == EOK) {
- DEBUG(SSSDBG_TRACE_FUNC,
- ("Last call was too recent, nothing to do!\n"));
- goto immediately;
- } else if (ret != EAGAIN) {
- DEBUG(SSSDBG_TRACE_FUNC, ("check_domain_request failed with [%d][%s]\n",
- ret, strerror(ret)));
- goto immediately;
- }
+ info = talloc_zero(state, struct sss_dp_domains_info);
+ if (!info) {
+ ret = ENOMEM;
+ goto fail;
}
-
- info->rctx = rctx;
- info->dom = rctx->domains;
+ info->hint = hint;
info->force = force;
- if (hint != NULL) {
- info->hint = hint;
- } else {
- info->hint = talloc_strdup(info, "");
- if (info->hint == NULL) {
- ret = ENOMEM;
- goto immediately;
- }
- }
-
- ret = get_domains_next(req);
- if (ret != EAGAIN) {
- goto immediately;
- }
-
- return req;
-
-immediately:
- if (ret == EOK) {
- tevent_req_done(req);
- } else {
- tevent_req_error(req, ret);
- }
- tevent_req_post(req, rctx->ev);
-
- return req;
-}
-
-static errno_t get_domains_next(struct tevent_req *req)
-{
- struct sss_dp_domains_info *info;
- struct tevent_req *subreq;
- struct sss_dp_req_state *state;
- errno_t ret;
- char *key;
-
- info = tevent_req_data(req, struct sss_dp_domains_info);
+ info->dom = dom;
- /* Skip all local domains. */
- while(info->dom != NULL && !NEED_CHECK_PROVIDER(info->dom->provider)) {
- info->dom = info->dom->next;
- }
-
- if (info->dom == NULL) {
- return EOK;
- }
-
- subreq = tevent_req_create(info, &state, struct sss_dp_req_state);
- if (subreq == NULL) {
- return ENOMEM;
- }
-
- key = talloc_asprintf(info, "domains@%s", info->dom->name);
+ key = talloc_asprintf(state, "domains@%s", dom->name);
if (key == NULL) {
- talloc_free(subreq);
- return ENOMEM;
+ ret = ENOMEM;
+ goto fail;
}
- ret = sss_dp_issue_request(info, info->rctx, key, info->dom,
- sss_dp_get_domains_msg, info, subreq);
+ ret = sss_dp_issue_request(state, rctx, key, dom,
+ sss_dp_get_domains_msg, info, req);
talloc_free(key);
if (ret != EOK) {
- talloc_free(subreq);
- return ret;
+ ret = EIO;
+ goto fail;
}
- tevent_req_set_callback(subreq, sss_dp_get_domains_callback, req);
+ return req;
- return EAGAIN;
+fail:
+ tevent_req_error(req, ret);
+ tevent_req_post(req, rctx->ev);
+ return req;
}
static DBusMessage *
@@ -186,37 +119,152 @@ sss_dp_get_domains_msg(void *pvt)
return msg;
}
-static void sss_dp_get_domains_callback(struct tevent_req *subreq)
+static errno_t
+get_next_domain_recv(TALLOC_CTX *mem_ctx,
+ struct tevent_req *req,
+ dbus_uint16_t *dp_err,
+ dbus_uint32_t *dp_ret,
+ char **err_msg)
+{
+ return sss_dp_req_recv(mem_ctx, req, dp_err, dp_ret, err_msg);
+}
+
+/* ====== Iterate over all domains, searching for their subdomains ======= */
+static errno_t process_subdomains(struct sss_domain_info *dom);
+static errno_t check_last_request(struct resp_ctx *rctx, const char *hint);
+
+struct sss_dp_get_domains_state {
+ struct resp_ctx *rctx;
+ struct sss_domain_info *dom;
+ const char *hint;
+ bool force;
+};
+
+static void
+sss_dp_get_domains_process(struct tevent_req *subreq);
+
+struct tevent_req *sss_dp_get_domains_send(TALLOC_CTX *mem_ctx,
+ struct resp_ctx *rctx,
+ bool force,
+ const char *hint)
{
- struct tevent_req *req = tevent_req_callback_data(subreq, struct tevent_req);
- struct sss_dp_domains_info *info = tevent_req_data(req, struct sss_dp_domains_info);
errno_t ret;
+ struct tevent_req *req;
+ struct tevent_req *subreq;
+ struct sss_dp_get_domains_state *state;
+
+ req = tevent_req_create(mem_ctx, &state, struct sss_dp_get_domains_state);
+ if (req == NULL) {
+ return NULL;
+ }
+
+ if (rctx->domains == NULL) {
+ DEBUG(SSSDBG_CRIT_FAILURE, ("No domains configured.\n"));
+ ret = EINVAL;
+ goto immediately;
+ }
+
+ if (!force) {
+ ret = check_last_request(rctx, hint);
+ if (ret == EOK) {
+ DEBUG(SSSDBG_TRACE_FUNC,
+ ("Last call was too recent, nothing to do!\n"));
+ goto immediately;
+ } else if (ret != EAGAIN) {
+ DEBUG(SSSDBG_TRACE_FUNC, ("check_domain_request failed with [%d][%s]\n",
+ ret, strerror(ret)));
+ goto immediately;
+ }
+ }
+
+ state->rctx = rctx;
+ state->force = force;
+ if (hint != NULL) {
+ state->hint = hint;
+ } else {
+ state->hint = talloc_strdup(state, "");
+ if (state->hint == NULL) {
+ ret = ENOMEM;
+ goto immediately;
+ }
+ }
+
+ state->dom = rctx->domains;
+ while(state->dom != NULL && !NEED_CHECK_PROVIDER(state->dom->provider)) {
+ state->dom = state->dom->next;
+ }
+
+ if (state->dom == NULL) {
+ /* All domains were local */
+ ret = EOK;
+ goto immediately;
+ }
+
+ subreq = get_subdomains_send(req, rctx, state->dom,
+ state->force, state->hint);
+ if (ret != EAGAIN) {
+ goto immediately;
+ }
+ tevent_req_set_callback(subreq, sss_dp_get_domains_process, req);
+
+ return req;
+
+immediately:
+ if (ret == EOK) {
+ tevent_req_done(req);
+ } else {
+ tevent_req_error(req, ret);
+ }
+ tevent_req_post(req, rctx->ev);
+
+ return req;
+}
+
+static void
+sss_dp_get_domains_process(struct tevent_req *subreq)
+{
+ errno_t ret;
+ struct tevent_req *req = tevent_req_callback_data(subreq,
+ struct tevent_req);
+ struct sss_dp_get_domains_state *state = tevent_req_data(req,
+ struct sss_dp_get_domains_state);
dbus_uint16_t dp_err;
dbus_uint32_t dp_ret;
char *err_msg;
- /* TODO: handle errors better */
- ret = sss_dp_req_recv(info, subreq, &dp_err, &dp_ret, &err_msg);
- talloc_free(subreq);
+ ret = get_next_domain_recv(req, subreq, &dp_err, &dp_ret, &err_msg);
+ talloc_zfree(subreq);
if (ret != EOK) {
goto fail;
}
- ret = get_domains_done(req);
+ ret = process_subdomains(state->dom);
if (ret != EOK) {
- DEBUG(SSSDBG_OP_FAILURE, ("get_domains_done failed, "
+ DEBUG(SSSDBG_OP_FAILURE, ("process_subdomains failed, "
"trying next domain.\n"));
goto fail;
}
- info->dom = info->dom->next;
- ret = get_domains_next(req);
- if (ret == EOK) {
+ /* Advance to the next domain */
+ state->dom = state->dom->next;
+
+ /* Skip local domains */
+ while(state->dom != NULL && !NEED_CHECK_PROVIDER(state->dom->provider)) {
+ state->dom = state->dom->next;
+ }
+
+ if (state->dom == NULL) {
+ /* All domains were local */
tevent_req_done(req);
- } else if (ret != EAGAIN) {
- goto fail;
+ return;
}
+ subreq = get_subdomains_send(req, state->rctx, state->dom, state->force, state->hint);
+ if (subreq == NULL) {
+ ret = ENOMEM;
+ goto fail;
+ }
+ tevent_req_set_callback(subreq, sss_dp_get_domains_process, req);
return;
fail:
@@ -224,20 +272,16 @@ fail:
return;
}
-static errno_t get_domains_done(struct tevent_req *req)
+static errno_t
+process_subdomains(struct sss_domain_info *domain)
{
int ret;
size_t c;
- struct sss_dp_domains_info *state;
- struct sss_domain_info *domain;
struct sss_domain_info **new_sd_list = NULL;
size_t subdomain_count;
struct sysdb_subdom **subdomains;
struct sysdb_subdom *master_info;
- state = tevent_req_data(req, struct sss_dp_domains_info);
- domain = state->dom;
-
/* Retrieve all subdomains of this domain from sysdb
* and create their struct sss_domain_info representations
*/