On 03/06/2013 06:33 PM, Simo Sorce wrote:
On Wed, 2013-03-06 at 17:09 +0100, Michal Židek wrote:
https://fedorahosted.org/sssd/ticket/1826
See commit message.
It would be better if you can use a destructor attached to the mc_ctx
so any other path where we need to free it is automatically covered.
Simo.
New patch attached.
Thanks
Michal
>From c72f4e087086a27e2b19ac54950fd13ab42c1cd1 Mon Sep 17 00:00:00 2001
From: Michal Zidek <mzi...@redhat.com>
Date: Wed, 6 Mar 2013 16:46:32 +0100
Subject: [PATCH] File descriptor leak in nss responder.
The function sss_mmap_cache_reinit leaked file descriptors and
also left the old memory cache still maped in memory (munmap was
not called).
https://fedorahosted.org/sssd/ticket/1826
---
src/responder/nss/nsssrv_mmap_cache.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index f72923e..9969fc0 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -897,6 +897,32 @@ static void sss_mc_header_update(struct sss_mc_ctx *mc_ctx, int status)
MC_LOWER_BARRIER(h);
}
+static int mc_ctx_destructor(struct sss_mc_ctx *mc_ctx)
+{
+ int ret;
+
+ ret = close(mc_ctx->fd);
+ if (ret == -1) {
+ /* Failing to close the file descriptor is minor issue and
+ * the destructor will succeed. */
+ ret = errno;
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ ("Failed to close old memory cache file."
+ "[%d]: %s\n", ret, strerror(ret)));
+ }
+
+ ret = munmap(mc_ctx->mmap_base, mc_ctx->mmap_size);
+ if (ret == -1) {
+ /* Failing to release the mmaped memory is bigger issue so
+ * we return error code and let the destructor fail. */
+ ret = errno;
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ ("Failed to unmap old memory cache file."
+ "[%d]: %s\n", ret, strerror(ret)));
+ }
+ return ret;
+}
+
errno_t sss_mmap_cache_init(TALLOC_CTX *mem_ctx, const char *name,
enum sss_mc_type type, size_t n_elem,
time_t timeout, struct sss_mc_ctx **mcc)
@@ -1018,6 +1044,7 @@ done:
talloc_free(mc_ctx);
} else {
*mcc = mc_ctx;
+ talloc_set_destructor(mc_ctx, mc_ctx_destructor);
}
return ret;
}
@@ -1063,8 +1090,9 @@ errno_t sss_mmap_cache_reinit(TALLOC_CTX *mem_ctx, size_t n_elem,
if (ret != 0) {
/* This can happen only if destructor is associated with this
* context */
- DEBUG(SSSDBG_MINOR_FAILURE, ("Destructor asociated with memory"
- " context failed.\n"));
+ DEBUG(SSSDBG_MINOR_FAILURE,
+ ("Failed to talloc_free mmap cache context.\n"));
+ return ret;
}
/* make sure we do not leave a potentially freed pointer around */
--
1.7.11.2
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
