URL: https://github.com/SSSD/sssd/pull/395
Author: jhrozek
Title: #395: KCM: Three trivial fixes
Action: synchronized
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/395/head:pr395
git checkout pr395
From 9d8f1f368a6cd226b4c4c8acea91ce59708de293 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Tue, 19 Sep 2017 13:45:19 +0200
Subject: [PATCH 1/3] KCM: Do not leak newly created ccache in case the name is
malformed
This is not a big deal as the mem_ctx parameter of the operation is
typically just a short-lived operation context. Nonetheless, it is best
practice to not rely on how the memory context is set up in utility
functions.
---
src/responder/kcm/kcmsrv_ccache.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/src/responder/kcm/kcmsrv_ccache.c b/src/responder/kcm/kcmsrv_ccache.c
index a22184e0f..d3ed10eee 100644
--- a/src/responder/kcm/kcmsrv_ccache.c
+++ b/src/responder/kcm/kcmsrv_ccache.c
@@ -45,7 +45,7 @@ errno_t kcm_cc_new(TALLOC_CTX *mem_ctx,
krb5_principal princ,
struct kcm_ccache **_cc)
{
- struct kcm_ccache *cc;
+ struct kcm_ccache *cc = NULL;
krb5_error_code kret;
errno_t ret;
@@ -57,13 +57,13 @@ errno_t kcm_cc_new(TALLOC_CTX *mem_ctx,
ret = kcm_check_name(name, owner);
if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Name %s is malformed\n", name);
- return ret;
+ goto done;
}
cc->name = talloc_strdup(cc, name);
if (cc->name == NULL) {
- talloc_free(cc);
- return ENOMEM;
+ ret = ENOMEM;
+ goto done;
}
uuid_generate(cc->uuid);
@@ -74,8 +74,8 @@ errno_t kcm_cc_new(TALLOC_CTX *mem_ctx,
DEBUG(SSSDBG_OP_FAILURE,
"krb5_copy_principal failed: [%d][%s]\n", kret, err_msg);
sss_krb5_free_error_message(k5c, err_msg);
- talloc_free(cc);
- return ERR_INTERNAL;
+ ret = ERR_INTERNAL;
+ goto done;
}
cc->owner.uid = cli_creds_get_uid(owner);
@@ -84,7 +84,12 @@ errno_t kcm_cc_new(TALLOC_CTX *mem_ctx,
talloc_set_destructor(cc, kcm_cc_destructor);
*_cc = cc;
- return EOK;
+ ret = EOK;
+done:
+ if (ret != EOK) {
+ talloc_free(cc);
+ }
+ return ret;
}
const char *kcm_cc_get_name(struct kcm_ccache *cc)
From d4bde136d9d50e567d303932002f68a85a4bc7de Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Wed, 27 Sep 2017 16:48:06 +0200
Subject: [PATCH 2/3] KCM: Use the right memory context
Inside the tevent request, we should use 'state' as the intermediate
memory context and steal the result up to 'mem_ctx' on success.
'mem_ctx' itself should only be used to create the tevent_req as the
first thing during the request creation.
However, this bug is not very severe as the mem_ctx was always the KCM
operation memory context, so the memory was freed when the operation
terminated.
---
src/responder/kcm/kcmsrv_ccache.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/responder/kcm/kcmsrv_ccache.c b/src/responder/kcm/kcmsrv_ccache.c
index d3ed10eee..63dd1f85b 100644
--- a/src/responder/kcm/kcmsrv_ccache.c
+++ b/src/responder/kcm/kcmsrv_ccache.c
@@ -302,7 +302,7 @@ struct tevent_req *kcm_ccdb_nextid_send(TALLOC_CTX *mem_ctx,
goto immediate;
}
- subreq = state->db->ops->nextid_send(mem_ctx, ev, state->db, client);
+ subreq = state->db->ops->nextid_send(state, ev, state->db, client);
if (subreq == NULL) {
ret = ENOMEM;
goto immediate;
@@ -390,7 +390,7 @@ struct tevent_req *kcm_ccdb_list_send(TALLOC_CTX *mem_ctx,
goto immediate;
}
- subreq = state->db->ops->list_send(mem_ctx,
+ subreq = state->db->ops->list_send(state,
ev,
state->db,
client);
@@ -467,7 +467,7 @@ struct tevent_req *kcm_ccdb_get_default_send(TALLOC_CTX *mem_ctx,
goto immediate;
}
- subreq = db->ops->get_default_send(mem_ctx, ev, db, client);
+ subreq = db->ops->get_default_send(state, ev, db, client);
if (subreq == NULL) {
ret = ENOMEM;
goto immediate;
@@ -1034,7 +1034,7 @@ struct tevent_req *kcm_ccdb_create_cc_send(TALLOC_CTX *mem_ctx,
goto immediate;
}
- subreq = state->db->ops->create_send(mem_ctx,
+ subreq = state->db->ops->create_send(state,
ev,
state->db,
client,
@@ -1129,7 +1129,7 @@ struct tevent_req *kcm_ccdb_mod_cc_send(TALLOC_CTX *mem_ctx,
goto immediate;
}
- subreq = state->db->ops->mod_send(mem_ctx,
+ subreq = state->db->ops->mod_send(state,
ev,
state->db,
client,
@@ -1204,7 +1204,7 @@ struct tevent_req *kcm_ccdb_store_cred_blob_send(TALLOC_CTX *mem_ctx,
goto immediate;
}
- subreq = state->db->ops->store_cred_send(mem_ctx,
+ subreq = state->db->ops->store_cred_send(state,
ev,
state->db,
client,
From e94821f7e547fdd3e3bb586fb468cadc86f5cf10 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhro...@redhat.com>
Date: Thu, 28 Sep 2017 17:51:11 +0200
Subject: [PATCH 3/3] KCM: Add some forgotten NULL checks
Several memory allocations across the KCM codebase did not check their
result for NULL. This patch fixes that.
---
src/responder/kcm/kcmsrv_ccache.c | 4 ++++
src/responder/kcm/kcmsrv_ccache_mem.c | 26 ++++++++++++++++++++++++--
2 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/src/responder/kcm/kcmsrv_ccache.c b/src/responder/kcm/kcmsrv_ccache.c
index 63dd1f85b..87a9bab73 100644
--- a/src/responder/kcm/kcmsrv_ccache.c
+++ b/src/responder/kcm/kcmsrv_ccache.c
@@ -1291,6 +1291,10 @@ struct tevent_req *kcm_ccdb_delete_cc_send(TALLOC_CTX *mem_ctx,
state->db,
state->client,
state->uuid);
+ if (subreq == NULL) {
+ ret = ENOMEM;
+ goto immediate;
+ }
tevent_req_set_callback(subreq, kcm_ccdb_delete_done, req);
return req;
diff --git a/src/responder/kcm/kcmsrv_ccache_mem.c b/src/responder/kcm/kcmsrv_ccache_mem.c
index 1c4f3df8d..38bc2050d 100644
--- a/src/responder/kcm/kcmsrv_ccache_mem.c
+++ b/src/responder/kcm/kcmsrv_ccache_mem.c
@@ -405,6 +405,7 @@ static struct tevent_req *ccdb_mem_getbyuuid_send(TALLOC_CTX *mem_ctx,
struct ccdb_mem_getbyuuid_state *state = NULL;
struct ccdb_mem *memdb = talloc_get_type(db->db_handle, struct ccdb_mem);
struct ccache_mem_wrap *ccwrap = NULL;
+ errno_t ret;
req = tevent_req_create(mem_ctx, &state, struct ccdb_mem_getbyuuid_state);
if (req == NULL) {
@@ -414,9 +415,19 @@ static struct tevent_req *ccdb_mem_getbyuuid_send(TALLOC_CTX *mem_ctx,
ccwrap = memdb_get_by_uuid(memdb, client, uuid);
if (ccwrap != NULL) {
state->cc = kcm_ccache_dup(state, ccwrap->cc);
+ if (state->cc == NULL) {
+ ret = ENOMEM;
+ goto immediate;
+ }
}
- tevent_req_done(req);
+ ret = EOK;
+immediate:
+ if (ret == EOK) {
+ tevent_req_done(req);
+ } else {
+ tevent_req_error(req, ret);
+ }
tevent_req_post(req, ev);
return req;
}
@@ -447,6 +458,7 @@ static struct tevent_req *ccdb_mem_getbyname_send(TALLOC_CTX *mem_ctx,
struct ccdb_mem_getbyname_state *state = NULL;
struct ccache_mem_wrap *ccwrap = NULL;
struct ccdb_mem *memdb = talloc_get_type(db->db_handle, struct ccdb_mem);
+ errno_t ret;
req = tevent_req_create(mem_ctx, &state, struct ccdb_mem_getbyname_state);
if (req == NULL) {
@@ -456,9 +468,19 @@ static struct tevent_req *ccdb_mem_getbyname_send(TALLOC_CTX *mem_ctx,
ccwrap = memdb_get_by_name(memdb, client, name);
if (ccwrap != NULL) {
state->cc = kcm_ccache_dup(state, ccwrap->cc);
+ if (state->cc == NULL) {
+ ret = ENOMEM;
+ goto immediate;
+ }
}
- tevent_req_done(req);
+ ret = EOK;
+immediate:
+ if (ret == EOK) {
+ tevent_req_done(req);
+ } else {
+ tevent_req_error(req, ret);
+ }
tevent_req_post(req, ev);
return req;
}
_______________________________________________
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
