On Wed, Nov 18, 2015 at 03:06:03PM +0100, Pavel Reichl wrote:
Hello,
attached patch proposes solution for leaking memory when non-existing netgroup is looked
up.
1st patch is just for testing - just call 'pkill -SIGUSR1 sssd_nss' and talloc
report will be generated in /tmp/sssd_nss_talloc_report_full.
For details about the bug please see commit message in 2nd patch.
User who reported the bug confirmed that so far it seems that memory leak has been fixed
and he didn't report any side effects.
Thanks!
From abbf720b832beb6d04f909f598e7114a19f72a03 Mon Sep 17 00:00:00
2001
From: Pavel Reichl <preichl(a)redhat.com>
Date: Wed, 30 Sep 2015 07:54:31 -0400
Subject: [PATCH 1/2] talloc_report for nss responder
---
src/responder/nss/nsssrv.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/src/responder/nss/nsssrv.c b/src/responder/nss/nsssrv.c
index d8eff7968c4929663412aa56d08414689b921a22..79ca9ba89d49a5fd4ee3389e0f881b1a01f9c7a9
100644
--- a/src/responder/nss/nsssrv.c
+++ b/src/responder/nss/nsssrv.c
@@ -403,6 +403,20 @@ static void nss_dp_reconnect_init(struct sbus_connection *conn,
/* nss_shutdown(rctx); */
}
+static void signal_nss_talloc_report(struct tevent_context *ev,
+ struct tevent_signal *se,
+ int signum,
+ int count,
+ void *siginfo,
+ void *private_data)
+{
+ FILE *f = fopen("/tmp/sssd_nss_talloc_report_full", "w");
+ if (f != NULL) {
+ talloc_report_full(NULL, f);
+ fclose(f);
+ }
+}
+
int nss_process_init(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
struct confdb_ctx *cdb)
@@ -417,6 +431,8 @@ int nss_process_init(TALLOC_CTX *mem_ctx,
int hret;
int fd_limit;
+ talloc_enable_leak_report_full();
+
nss_cmds = get_nss_cmds();
ret = sss_process_init(mem_ctx, ev, cdb,
@@ -558,6 +574,16 @@ int nss_process_init(TALLOC_CTX *mem_ctx,
goto fail;
}
+ /* Handle SIGUSR1 to force offline behavior */
+ BlockSignals(false, SIGUSR1);
+ struct tevent_signal *tes;
+ tes = tevent_add_signal(rctx->ev, rctx, SIGUSR1, 0,
+ signal_nss_talloc_report, rctx);
+ if (tes == NULL) {
+ ret = EIO;
+ goto fail;
+ }
+
DEBUG(SSSDBG_TRACE_FUNC, "NSS Initialization complete\n");
return EOK;
--
2.4.3
From 0c05cef940b8a8650537056db4ade09b0b6a6fa6 Mon Sep 17 00:00:00
2001
From: Pavel Reichl <preichl(a)redhat.com>
Date: Tue, 10 Nov 2015 10:56:56 -0500
Subject: [PATCH 2/2] NSS: Fix memory leak netgroup
If netgroup cannot be found in setnetgrent_retry() function
set_netgroup_entry() is called which steals getent_ctx directly to
nss_ctx->netgroups.
Subsequently function lookup_netgr_step() is called that (in case of
nenexisting group) will call create_negcache_netgr() which creates
a new dummy object to serve as negative cache. While doing so it calls
again set_netgroup_entry() for the same netgroup and it calls
hash_enter.
hash_enter will remove previously hashed entry for netgroup (created in
setnetgrent_retry()) from hash table but it won't be freed and thus it
leaks.
This patch sets netgroup lifetime for netgroups allocated in
setnetgrent_retry().
We already set the netgroup lifetime in both the found and notfound case
in lookup_netgr_step(). If I understand the code correctly, the issue is
that we create a new negative result in create_negcache_netgr(),
overwriting the one that we created in setnetgrent_retry(), correct?
Wouldn't it then be better to see if another same object is already in
the hashtable and free it before replacing?