[SSSD] [PATCH] NSS: Fix the error handler in sss_mc_create_file

Jakub Hrozek jhrozek at redhat.com
Mon Dec 10 17:10:01 UTC 2012


On Mon, Dec 10, 2012 at 09:06:16AM -0500, Simo Sorce wrote:
> On Mon, 2012-12-10 at 08:39 +0100, Jakub Hrozek wrote:
> > Coverity bug again.
> > 
> > https://fedorahosted.org/sssd/ticket/1704
> 
> Any reason why you do not close the file anymore in a lock error ?
> 
> I know the caller will also cleanup but I would expect that if the
> create function fails no file is open and no file is on disk ...
> 

Hmm, did I attach the wrong patch? In the version I have in tree, there
is close(mc_ctx->fd); called in case sss_br_lock_file() fails. It's not
very readable from the diff itself, but in the source, the close would be
on line 721 in src/responder/nss/nsssrv_mmap_cache.c

Anyway, I'm attaching the patch again.
-------------- next part --------------
>From 55881e0068ea3ce7663a7e24e7d7e426f18d316e Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Sun, 9 Dec 2012 15:16:44 +0100
Subject: [PATCH] NSS: Fix the error handler in sss_mc_create_file

https://fedorahosted.org/sssd/ticket/1704

The function is short enough so that we can simply stick with return and
release resources before returning as appropriate.
---
 src/responder/nss/nsssrv_mmap_cache.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/src/responder/nss/nsssrv_mmap_cache.c b/src/responder/nss/nsssrv_mmap_cache.c
index c6c5a5558400938f7afa701a642070821542b160..4b8b7b7317e10fefc37f5a44a33f91ea0a209bfc 100644
--- a/src/responder/nss/nsssrv_mmap_cache.c
+++ b/src/responder/nss/nsssrv_mmap_cache.c
@@ -704,28 +704,22 @@ static errno_t sss_mc_create_file(struct sss_mc_ctx *mc_ctx)
      * by everyone for now */
     old_mask = umask(0022);
 
-    ret = 0;
+    errno = 0;
     mc_ctx->fd = open(mc_ctx->file, O_CREAT | O_EXCL | O_RDWR, 0644);
+    umask(old_mask);
     if (mc_ctx->fd == -1) {
         ret = errno;
         DEBUG(SSSDBG_CRIT_FAILURE, ("Failed to open mmap file %s: %d(%s)\n",
                                     mc_ctx->file, ret, strerror(ret)));
-        goto done;
+        return ret;
     }
 
     ret = sss_br_lock_file(mc_ctx->fd, 0, 1, retries, t);
     if (ret != EOK) {
         DEBUG(SSSDBG_FATAL_FAILURE,
               ("Failed to lock file %s.\n", mc_ctx->file));
-        goto done;
-    }
-
-done:
-    /* reset mask back */
-    umask(old_mask);
-
-    if (ret) {
         close(mc_ctx->fd);
+        return ret;
     }
 
     return ret;
-- 
1.8.0.1



More information about the sssd-devel mailing list