On Tue, 13 Apr 2010 12:51:39 -0400
Simo Sorce <sso...@redhat.com> wrote:
>
>
> This patch attempts to address the problem Eugene found with
> sdap_handle_release()
Attached a follow-up patch to address Eugene's legitimate concern about
restrictions put on _send() functions. We never free gsh in a _send()
either but only in the _recv() function of the call that actually
establish a new connection. This guarantees that gsh will never be
freed during sdap_handle_release() or any other error code path.
I can squash the 2 patches in a single one if people feels that is
better. Comments welcome.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
>From d785acc68d383c9283fe1cfb54b8fa8c119f9b7e Mon Sep 17 00:00:00 2001
From: Simo Sorce <sso...@redhat.com>
Date: Wed, 14 Apr 2010 10:31:49 -0400
Subject: [PATCH] Free sdap_handle in a better place
By freeing sdap_handle only in the connection _recv() function we
guarantee it can never be done within sdap_handle_release() but only
in a following event.
---
src/providers/ipa/ipa_access.c | 8 --------
src/providers/ldap/ldap_id.c | 6 ------
src/providers/ldap/ldap_id_enum.c | 4 ----
src/providers/ldap/sdap_async_connection.c | 3 +++
4 files changed, 3 insertions(+), 18 deletions(-)
diff --git a/src/providers/ipa/ipa_access.c b/src/providers/ipa/ipa_access.c
index a1b3fe4..56aa987 100644
--- a/src/providers/ipa/ipa_access.c
+++ b/src/providers/ipa/ipa_access.c
@@ -325,10 +325,6 @@ static struct tevent_req *hbac_get_host_info_send(TALLOC_CTX *memctx,
}
if (sdap_ctx->gsh == NULL || ! sdap_ctx->gsh->connected) {
- if (sdap_ctx->gsh != NULL) {
- talloc_zfree(sdap_ctx->gsh);
- }
-
subreq = sdap_cli_connect_send(state, ev, sdap_ctx->opts,
sdap_ctx->be, sdap_ctx->service, NULL);
if (!subreq) {
@@ -780,10 +776,6 @@ static struct tevent_req *hbac_get_rules_send(TALLOC_CTX *memctx,
}
if (sdap_ctx->gsh == NULL || ! sdap_ctx->gsh->connected) {
- if (sdap_ctx->gsh != NULL) {
- talloc_zfree(sdap_ctx->gsh);
- }
-
subreq = sdap_cli_connect_send(state, ev, sdap_ctx->opts,
sdap_ctx->be, sdap_ctx->service, NULL);
if (!subreq) {
diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
index a2109f3..ecab692 100644
--- a/src/providers/ldap/ldap_id.c
+++ b/src/providers/ldap/ldap_id.c
@@ -99,8 +99,6 @@ struct tevent_req *users_get_send(TALLOC_CTX *memctx,
if (!sdap_connected(ctx)) {
- if (ctx->gsh) talloc_zfree(ctx->gsh);
-
/* FIXME: add option to decide if tls should be used
* or SASL/GSSAPI, etc ... */
subreq = sdap_cli_connect_send(state, ev, ctx->opts,
@@ -299,8 +297,6 @@ struct tevent_req *groups_get_send(TALLOC_CTX *memctx,
if (!sdap_connected(ctx)) {
- if (ctx->gsh) talloc_zfree(ctx->gsh);
-
/* FIXME: add option to decide if tls should be used
* or SASL/GSSAPI, etc ... */
subreq = sdap_cli_connect_send(state, ev, ctx->opts,
@@ -465,8 +461,6 @@ static struct tevent_req *groups_by_user_send(TALLOC_CTX *memctx,
if (!sdap_connected(ctx)) {
- if (ctx->gsh) talloc_zfree(ctx->gsh);
-
/* FIXME: add option to decide if tls should be used
* or SASL/GSSAPI, etc ... */
subreq = sdap_cli_connect_send(state, ev, ctx->opts,
diff --git a/src/providers/ldap/ldap_id_enum.c b/src/providers/ldap/ldap_id_enum.c
index 6c5570d..d1281df 100644
--- a/src/providers/ldap/ldap_id_enum.c
+++ b/src/providers/ldap/ldap_id_enum.c
@@ -438,8 +438,6 @@ static struct tevent_req *enum_users_send(TALLOC_CTX *memctx,
if (!sdap_connected(ctx)) {
- if (ctx->gsh) talloc_zfree(ctx->gsh);
-
/* FIXME: add option to decide if tls should be used
* or SASL/GSSAPI, etc ... */
subreq = sdap_cli_connect_send(state, ev, ctx->opts,
@@ -593,8 +591,6 @@ static struct tevent_req *enum_groups_send(TALLOC_CTX *memctx,
if (!sdap_connected(ctx)) {
- if (ctx->gsh) talloc_zfree(ctx->gsh);
-
/* FIXME: add option to decide if tls should be used
* or SASL/GSSAPI, etc ... */
subreq = sdap_cli_connect_send(state, ev, ctx->opts,
diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c
index 586733f..e27c196 100644
--- a/src/providers/ldap/sdap_async_connection.c
+++ b/src/providers/ldap/sdap_async_connection.c
@@ -1117,6 +1117,9 @@ int sdap_cli_connect_recv(struct tevent_req *req,
}
if (gsh) {
+ if (*gsh) {
+ talloc_zfree(*gsh);
+ }
*gsh = talloc_steal(memctx, state->sh);
if (!*gsh) {
return ENOMEM;
--
1.6.6.1
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
