On 03/06/2013 07:27 PM, Michal Židek wrote:
On 03/06/2013 07:18 PM, Michal Židek wrote:
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
There should be goto done, not ret... will send another patch soon.
Here it is.
Michal
>From 59c1ded53c52341fe3adf957f0eaf5602c620f87 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..610f6a0 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"));
+ goto done;
}
/* 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
