Hi,
if an nss provider dies during a request the nss responders dies due to
a double free error minutes after the request was sent. The responder
does not handle the unfinished request and so they stay around and the
client has to wait for a response until SSS_CLI_SOCKET_TIMEOUT (5
minutes) is over. During the termination of the request the
sss_dp_callback structure is free two times which lead to an abort in
the nss responder.
The first patch add a handler which removes all open requests after a
reconnect. The second one is only marginally related to the described
issue but makes sure that cmdctx, the parent or grand-parent memory
context of the sss_dp_callback structure, is always allocate with
talloc_zero. The third patch removes the explicit talloc_free of the
sss_dp_callback structures, because they are already freed because
cmdctx was freed during the callback.
bye,
Sumit
From d819922fa1906e5c8c3f65fa171fab11fffb8798 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Fri, 22 Oct 2010 12:01:23 +0200
Subject: [PATCH 1/3] Remove all nss requests after a reconnect
Currently we do not handle the open nss request after a reconnect and
wait until they timeout (which is a couple of minutes!). This patch adds
a handler that terminates all requests after a reconnect. Then responder
will return matching cache entries or nothing.
---
src/responder/common/responder.h | 2 ++
src/responder/common/responder_dp.c | 19 +++++++++++++++++++
src/responder/nss/nsssrv.c | 6 +++++-
3 files changed, 26 insertions(+), 1 deletions(-)
diff --git a/src/responder/common/responder.h b/src/responder/common/responder.h
index 980e561..783f9e4 100644
--- a/src/responder/common/responder.h
+++ b/src/responder/common/responder.h
@@ -161,6 +161,8 @@ struct cli_protocol_version
*register_cli_protocol_version(void);
typedef void (*sss_dp_callback_t)(uint16_t err_maj, uint32_t err_min,
const char *err_msg, void *ptr);
+void handle_requests_after_reconnect(void);
+
int sss_dp_send_acct_req(struct resp_ctx *rctx, TALLOC_CTX *callback_memctx,
sss_dp_callback_t callback, void *callback_ctx,
int timeout, const char *domain,
diff --git a/src/responder/common/responder_dp.c
b/src/responder/common/responder_dp.c
index fd7b976..f8c3b6f 100644
--- a/src/responder/common/responder_dp.c
+++ b/src/responder/common/responder_dp.c
@@ -107,6 +107,25 @@ static int sss_dp_req_destructor(void *ptr)
return 0;
}
+static bool reconnect_handler(hash_entry_t *item, void *user_data)
+{
+ struct sss_dp_req *sdp_req = talloc_get_type(item->value.ptr,
+ struct sss_dp_req);
+
+ return (talloc_free(sdp_req) == EOK ? true : false);
+}
+
+void handle_requests_after_reconnect(void)
+{
+ int ret;
+
+ ret = hash_iterate(dp_requests, reconnect_handler, NULL);
+ if (ret != HASH_SUCCESS) {
+ DEBUG(1, ("hash_iterate failed, "
+ "not all request might be handled after reconnect.\n"));
+ }
+}
+
static void sdp_req_timeout(struct tevent_context *ev,
struct tevent_timer *te,
struct timeval t, void *ptr)
diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c
index 4f5aa62..dfb0312 100644
--- a/src/responder/nss/nsssrv.c
+++ b/src/responder/nss/nsssrv.c
@@ -39,6 +39,7 @@
#include "dbus/dbus.h"
#include "sbus/sssd_dbus.h"
#include "responder/common/responder_packet.h"
+#include "responder/common/responder.h"
#include "providers/data_provider.h"
#include "monitor/monitor_interfaces.h"
#include "sbus/sbus_client.h"
@@ -143,7 +144,10 @@ static void nss_dp_reconnect_init(struct sbus_connection
*conn,
DATA_PROVIDER_VERSION,
"NSS");
/* all fine */
- if (ret == EOK) return;
+ if (ret == EOK) {
+ handle_requests_after_reconnect();
+ return;
+ }
}
/* Failed to reconnect */
--
1.7.2.3
From 1878d676bfd219cab8b75f5742946aca4f3b214c Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Fri, 22 Oct 2010 13:01:31 +0200
Subject: [PATCH 2/3] Always use talloc_zero() to allocate cmdctx
---
src/responder/nss/nsssrv_cmd.c | 4 ++--
src/responder/nss/nsssrv_netgroup.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c
index e6437a6..65d9c58 100644
--- a/src/responder/nss/nsssrv_cmd.c
+++ b/src/responder/nss/nsssrv_cmd.c
@@ -1301,7 +1301,7 @@ static int nss_cmd_getpwent(struct cli_ctx *cctx)
DEBUG(4, ("Requesting info for all accounts\n"));
- cmdctx = talloc(cctx, struct nss_cmd_ctx);
+ cmdctx = talloc_zero(cctx, struct nss_cmd_ctx);
if (!cmdctx) {
return ENOMEM;
}
@@ -2630,7 +2630,7 @@ static int nss_cmd_getgrent(struct cli_ctx *cctx)
DEBUG(4, ("Requesting info for all groups\n"));
- cmdctx = talloc(cctx, struct nss_cmd_ctx);
+ cmdctx = talloc_zero(cctx, struct nss_cmd_ctx);
if (!cmdctx) {
return ENOMEM;
}
diff --git a/src/responder/nss/nsssrv_netgroup.c
b/src/responder/nss/nsssrv_netgroup.c
index feda556..1ac2608 100644
--- a/src/responder/nss/nsssrv_netgroup.c
+++ b/src/responder/nss/nsssrv_netgroup.c
@@ -595,7 +595,7 @@ int nss_cmd_getnetgrent(struct cli_ctx *client)
DEBUG(4, ("Requesting netgroup data\n"));
- cmdctx = talloc(client, struct nss_cmd_ctx);
+ cmdctx = talloc_zero(client, struct nss_cmd_ctx);
if (!cmdctx) {
return ENOMEM;
}
--
1.7.2.3
From bb123f42bb81c17f9c20926ad7b38bceb63cb316 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sb...@redhat.com>
Date: Mon, 25 Oct 2010 11:14:04 +0200
Subject: [PATCH 3/3] Fix double free issue
---
src/responder/common/responder_dp.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/responder/common/responder_dp.c
b/src/responder/common/responder_dp.c
index f8c3b6f..b2b5d40 100644
--- a/src/responder/common/responder_dp.c
+++ b/src/responder/common/responder_dp.c
@@ -95,12 +95,12 @@ static int sss_dp_req_destructor(void *ptr)
cb = sdp_req->cb_list;
while (cb) {
+ next = cb->next;
+ /* It is the responsibility of the callback to free cb */
cb->callback(sdp_req->err_maj,
sdp_req->err_min,
sdp_req->err_msg,
cb->callback_ctx);
- next = cb->next;
- talloc_free(cb);
cb = next;
}
--
1.7.2.3
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
