[SSSD] [PATCH] Issues found by the new Coverity instance

Jakub Hrozek jhrozek at redhat.com
Wed Apr 3 09:26:35 UTC 2013


Hi,

the new Coverity instance found a couple of issues the old one didn't.
I've checked most of the reports by now and while many of them are false
positives, there is a couple of genuine defects, too.

The ones that could be solved immediatelly have patches attached, the
ones that require a little more thought have tickets assigned.
-------------- next part --------------
>From 6b94739450b4d2357e995288d0adcfc38986d79b Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 3 Apr 2013 09:42:40 +0200
Subject: [PATCH 1/4] pidfile(): Do not leak fd on error

https://fedorahosted.org/sssd/ticket/1860
---
 src/util/server.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/util/server.c b/src/util/server.c
index b3073fcd130ba67a8cc7b5d63b8d1a20e28807ac..beba57b29ead898bbe0dcc9614f563d16bb28073 100644
--- a/src/util/server.c
+++ b/src/util/server.c
@@ -221,6 +221,7 @@ int pidfile(const char *path, const char *name)
         err = errno;
         DEBUG(SSSDBG_CRIT_FAILURE,
               ("write failed [%d][%s]\n", err, strerror(err)));
+        close(fd);
         return err;
     }
 
-- 
1.8.1.4

-------------- next part --------------
>From 41ab67f2efe66602282d8baaf8a6101f614d3352 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 3 Apr 2013 10:15:37 +0200
Subject: [PATCH 2/4] Fix potential out-of-bounds write in
 sss_idmap_sid_to_dom_sid

https://fedorahosted.org/sssd/ticket/1861
---
 src/lib/idmap/sss_idmap_conv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/lib/idmap/sss_idmap_conv.c b/src/lib/idmap/sss_idmap_conv.c
index a33604226f21315e12779fbbdd8971b8e8e250f4..ef93f2a9fb782fcba1bccacf100422d33bf65fee 100644
--- a/src/lib/idmap/sss_idmap_conv.c
+++ b/src/lib/idmap/sss_idmap_conv.c
@@ -289,7 +289,7 @@ enum idmap_error_code sss_idmap_sid_to_dom_sid(struct sss_idmap_ctx *ctx,
     }
 
     do {
-        if (dom_sid->num_auths > SID_SUB_AUTHS) {
+        if (dom_sid->num_auths >= SID_SUB_AUTHS) {
             err = IDMAP_SID_INVALID;
             goto done;
         }
-- 
1.8.1.4

-------------- next part --------------
>From f2a4502580ac061824b72a6418ac0a05a07e48e8 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 3 Apr 2013 10:41:50 +0200
Subject: [PATCH 3/4] Return errno, not -1 on failure in files.c

https://fedorahosted.org/sssd/ticket/1862
---
 src/tools/files.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/tools/files.c b/src/tools/files.c
index 10fd6fe4396b3826a3ff65b8d84d3e0a1baddf5b..186f18b9bb241e39720da3e26476879e6cf09bd2 100644
--- a/src/tools/files.c
+++ b/src/tools/files.c
@@ -703,6 +703,7 @@ copy_dir(struct copy_ctx *cctx,
      * reset these bits. */
     ret = fchmod(dest_dir_fd, mode);
     if (ret == -1) {
+        ret = errno;
         DEBUG(SSSDBG_OP_FAILURE,
               ("Error setting mode of '%s': %s",
               dest_dir_path, strerror(ret)));
-- 
1.8.1.4

-------------- next part --------------
>From 841b99df7e5794ecb1ad713b08cd2a971becf6a2 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek at redhat.com>
Date: Wed, 3 Apr 2013 11:18:31 +0200
Subject: [PATCH 4/4] Check for correct variable name

https://fedorahosted.org/sssd/ticket/1864
---
 src/providers/krb5/krb5_child.c            | 4 ----
 src/providers/proxy/proxy_auth.c           | 2 +-
 src/resolv/async_resolv.c                  | 2 +-
 src/responder/sudo/sudosrv_get_sudorules.c | 2 +-
 src/util/crypto/nss/nss_obfuscate.c        | 2 +-
 src/util/usertools.c                       | 2 +-
 6 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 4ccd75d26abf46cd07a70031569a29576223f0a2..5fdb1d20bda75a1e998e2a8627f56d834186b5a5 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -1974,10 +1974,6 @@ static int k5c_setup(struct krb5_req *kr, uint32_t offline)
      * expired. The library shall not use the prompter to ask for a new password
      * but shall return KRB5KDC_ERR_KEY_EXP. */
     krb5_get_init_creds_opt_set_change_password_prompt(kr->options, 0);
-    if (kerr != 0) {
-        KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);
-        return kerr;
-    }
 #endif
 
     lifetime_str = getenv(SSSD_KRB5_RENEWABLE_LIFETIME);
diff --git a/src/providers/proxy/proxy_auth.c b/src/providers/proxy/proxy_auth.c
index 011de4fbb8d855e7f1b75d51bafb5b23998fe79a..99b1a76c705975da91594eabd70e6a1a94eb6c18 100644
--- a/src/providers/proxy/proxy_auth.c
+++ b/src/providers/proxy/proxy_auth.c
@@ -531,7 +531,7 @@ static void proxy_child_sig_handler(struct tevent_context *ev,
 
         /* schedule another immediate timer to delete the sigchld handler */
         imm2 = tevent_create_immediate(ev);
-        if (imm == NULL) {
+        if (imm2 == NULL) {
             DEBUG(1, ("tevent_create_immediate failed.\n"));
             return;
         }
diff --git a/src/resolv/async_resolv.c b/src/resolv/async_resolv.c
index 0465151184ee81694b247be0088e849d5e4d0795..4ecc4836b32f7a9ece6ea5c38b43fac397d80614 100644
--- a/src/resolv/async_resolv.c
+++ b/src/resolv/async_resolv.c
@@ -1664,7 +1664,7 @@ resolv_getsrv_done(void *arg, int status, int timeouts, unsigned char *abuf, int
     }
 
     ret = ares_parse_srv_reply(abuf, alen, &reply_list);
-    if (status != ARES_SUCCESS) {
+    if (ret != ARES_SUCCESS) {
         DEBUG(2, ("SRV record parsing failed: %d: %s\n", ret, ares_strerror(ret)));
         ret = return_code(ret);
         goto fail;
diff --git a/src/responder/sudo/sudosrv_get_sudorules.c b/src/responder/sudo/sudosrv_get_sudorules.c
index f1537820d6b552467957a2a91f7c591371f4a6c5..6b6e6b23c020ce4b6432608eeaec331a43273c2e 100644
--- a/src/responder/sudo/sudosrv_get_sudorules.c
+++ b/src/responder/sudo/sudosrv_get_sudorules.c
@@ -197,7 +197,7 @@ static errno_t sudosrv_get_user(struct sudo_dom_ctx *dctx)
         /* user is stored in cache, remember cased and original name */
         original_name = ldb_msg_find_attr_as_string(user->msgs[0],
                                                     SYSDB_NAME, NULL);
-        if (name == NULL) {
+        if (original_name == NULL) {
             DEBUG(SSSDBG_CRIT_FAILURE, ("A user with no name?\n"));
             ret = EFAULT;
             goto done;
diff --git a/src/util/crypto/nss/nss_obfuscate.c b/src/util/crypto/nss/nss_obfuscate.c
index c0882d4ee58a25e7a9139fd9aec665b2b5d63c1b..842b58d1c8bd76800a88f004f8556fdc23b1c746 100644
--- a/src/util/crypto/nss/nss_obfuscate.c
+++ b/src/util/crypto/nss/nss_obfuscate.c
@@ -112,7 +112,7 @@ static int generate_random_key(TALLOC_CTX *mem_ctx,
     }
 
     randkeydata = PK11_GetKeyData(randkey);
-    if (randkey == NULL) {
+    if (randkeydata == NULL) {
         DEBUG(1, ("Failure to get key data (err %d)\n",
                   PR_GetError()));
         ret = EIO;
diff --git a/src/util/usertools.c b/src/util/usertools.c
index 587fd9cbfe083dcf0082eb2f8a41b5ef4912333c..7323d9f8260580f32b4ab55c8c2db5bd7eec20ed 100644
--- a/src/util/usertools.c
+++ b/src/util/usertools.c
@@ -383,7 +383,7 @@ int sss_parse_name_for_domains(TALLOC_CTX *memctx,
             rdomain = NULL;
             if (default_domain != NULL) {
                 rdomain = talloc_strdup(tmp_ctx, default_domain);
-                if (default_domain == NULL) {
+                if (rdomain == NULL) {
                     DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed.\n"));
                     ret = ENOMEM;
                     goto done;
-- 
1.8.1.4



More information about the sssd-devel mailing list