[SSSD] [PATCHES] Fix more assorted bugs found by Coverity

Sumit Bose sbose at redhat.com
Tue Dec 14 21:55:21 UTC 2010


Hi,

here are some more fixes for issues found by Coverity. Please have a
careful look on the patch for ticket #727. I think the patch will fix
the issue, but I'm not sure if some other kind of fix is needed here.

bye,
Sumit
-------------- next part --------------
From d4c69f19b705173882e6e0bd0f1e211be2cf49d6 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 14 Dec 2010 10:23:07 +0100
Subject: [PATCH 01/11] Fix unchecked return value in sdap_get_msg_dn()

https://fedorahosted.org/sssd/ticket/712
---
 src/providers/ldap/sdap.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
index 1735ac5..cec439e 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -268,9 +268,14 @@ int sdap_get_msg_dn(TALLOC_CTX *memctx, struct sdap_handle *sh,
 {
     char *str;
     int lerrno;
+    int ret;
 
     lerrno = 0;
-    ldap_set_option(sh->ldap, LDAP_OPT_RESULT_CODE, &lerrno);
+    ret = ldap_set_option(sh->ldap, LDAP_OPT_RESULT_CODE, &lerrno);
+    if (ret != LDAP_OPT_SUCCESS) {
+        DEBUG(1, ("ldap_set_option failed [%s], ignored.\n",
+                  ldap_err2string(ret)));
+    }
 
     str = ldap_get_dn(sh->ldap, sm->msg);
     if (!str) {
-- 
1.7.3.2

-------------- next part --------------
From 887b69e3b5081989f048932d1e65e43595074349 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 14 Dec 2010 10:25:20 +0100
Subject: [PATCH 02/11] Fix unchecked return value in sdap_parse_entry()

https://fedorahosted.org/sssd/ticket/712
---
 src/providers/ldap/sdap.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
index cec439e..573de17 100644
--- a/src/providers/ldap/sdap.c
+++ b/src/providers/ldap/sdap.c
@@ -104,7 +104,11 @@ int sdap_parse_entry(TALLOC_CTX *memctx,
     bool store;
 
     lerrno = 0;
-    ldap_set_option(sh->ldap, LDAP_OPT_RESULT_CODE, &lerrno);
+    ret = ldap_set_option(sh->ldap, LDAP_OPT_RESULT_CODE, &lerrno);
+    if (ret != LDAP_OPT_SUCCESS) {
+        DEBUG(1, ("ldap_set_option failed [%s], ignored.\n",
+                  ldap_err2string(ret)));
+    }
 
     attrs = sysdb_new_attrs(memctx);
     if (!attrs) return ENOMEM;
-- 
1.7.3.2

-------------- next part --------------
From cfd7c44ab39cd6a6c461ec0e680c91898b572d90 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 14 Dec 2010 10:30:29 +0100
Subject: [PATCH 03/11] Remove unused newauthtok variable in LOCAL_pam_handler

https://fedorahosted.org/sssd/ticket/716
---
 src/responder/pam/pam_LOCAL_domain.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/src/responder/pam/pam_LOCAL_domain.c b/src/responder/pam/pam_LOCAL_domain.c
index 0df9499..6e0631a 100644
--- a/src/responder/pam/pam_LOCAL_domain.c
+++ b/src/responder/pam/pam_LOCAL_domain.c
@@ -230,7 +230,6 @@ int LOCAL_pam_handler(struct pam_auth_req *preq)
     struct ldb_result *res;
     const char *username = NULL;
     const char *password = NULL;
-    char *newauthtok = NULL;
     char *new_hash = NULL;
     char *authtok = NULL;
     struct pam_data *pd = preq->pd;
@@ -352,8 +351,6 @@ done:
         memset(authtok, 0, pd->authtok_size);
     if (pd->newauthtok != NULL)
         memset(pd->newauthtok, 0, pd->newauthtok_size);
-    if (newauthtok != NULL)
-        memset(newauthtok, 0, pd->newauthtok_size);
 
     prepare_reply(lreq);
     return EOK;
-- 
1.7.3.2

-------------- next part --------------
From 6738dabca4ab2a9a81a305b88c1305c4dcc76866 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 14 Dec 2010 10:42:26 +0100
Subject: [PATCH 04/11] Fix improper NULL check in fo_add_srv_server()

https://fedorahosted.org/sssd/ticket/717
---
 src/providers/fail_over.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c
index 858d6ba..0c51b05 100644
--- a/src/providers/fail_over.c
+++ b/src/providers/fail_over.c
@@ -511,7 +511,8 @@ fo_add_srv_server(struct fo_service *service, const char *srv,
 
         if (fo_is_srv_lookup(server)) {
             if (((dns_domain == NULL && server->srv_data->dns_domain == NULL) ||
-                  strcasecmp(server->srv_data->dns_domain, dns_domain) == 0) &&
+                 (dns_domain != NULL && server->srv_data->dns_domain != NULL &&
+                  strcasecmp(server->srv_data->dns_domain, dns_domain) == 0)) &&
                 strcasecmp(server->srv_data->proto, proto) == 0) {
                 return EEXIST;
             }
-- 
1.7.3.2

-------------- next part --------------
From ac4aa20c0bf07ec1530ef4a9f0480229670d1b52 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 14 Dec 2010 10:45:32 +0100
Subject: [PATCH 05/11] Fix incorrect return value on failure in resolve_get_domain_send()

https://fedorahosted.org/sssd/ticket/721
---
 src/providers/fail_over.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c
index 0c51b05..2dabaa3 100644
--- a/src/providers/fail_over.c
+++ b/src/providers/fail_over.c
@@ -1170,6 +1170,7 @@ resolve_get_domain_send(TALLOC_CTX *mem_ctx,
 
     ret = gethostname(state->hostname, HOST_NAME_MAX);
     if (ret) {
+        ret = errno;
         DEBUG(2, ("gethostname() failed: [%d]: %s\n",ret, strerror(ret)));
         return NULL;
     }
-- 
1.7.3.2

-------------- next part --------------
From c918729293d2e914f5bc8d2bab4a53346ffba7a9 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 14 Dec 2010 10:48:52 +0100
Subject: [PATCH 06/11] Fix incorrect return value on failure in check_and_export_options()

https://fedorahosted.org/sssd/ticket/722
---
 src/providers/krb5/krb5_common.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c
index 9d114e3..434fc7f 100644
--- a/src/providers/krb5/krb5_common.c
+++ b/src/providers/krb5/krb5_common.c
@@ -85,6 +85,7 @@ errno_t check_and_export_lifetime(struct dp_option *opts, const int opt_id,
 
     ret = setenv(env_name, str, 1);
     if (ret != EOK) {
+        ret = errno;
         DEBUG(2, ("setenv [%s] failed.\n", env_name));
         goto done;
     }
-- 
1.7.3.2

-------------- next part --------------
From 9dee649da156d9a5141652d110e70ab0bfa15b2b Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 14 Dec 2010 11:14:26 +0100
Subject: [PATCH 07/11] Fix uninitialized value error in sdap_account_expired_shadow()

https://fedorahosted.org/sssd/ticket/726
---
 src/providers/ldap/ldap_common.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index 302cfc6..9945b4b 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -783,9 +783,9 @@ errno_t string_to_shadowpw_days(const char *s, long *d)
         return EINVAL;
     }
 
-    if (*d < -1) {
+    if (l < 0) {
         DEBUG(1, ("Input string contains not allowed negative value [%d].\n",
-                  *d));
+                  l));
         return EINVAL;
     }
 
-- 
1.7.3.2

-------------- next part --------------
From c70d176ca53bb92651cced378641359f47a4ea7d Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 14 Dec 2010 11:28:15 +0100
Subject: [PATCH 08/11] Fix uninitialized value error in lookup_netgr_step()

https://fedorahosted.org/sssd/ticket/727
---
 src/responder/nss/nsssrv_netgroup.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/src/responder/nss/nsssrv_netgroup.c b/src/responder/nss/nsssrv_netgroup.c
index 1ac2608..b51487e 100644
--- a/src/responder/nss/nsssrv_netgroup.c
+++ b/src/responder/nss/nsssrv_netgroup.c
@@ -464,8 +464,6 @@ static errno_t lookup_netgr_step(struct setent_step_ctx *step_ctx)
     /* If we've gotten here, then no domain contained this netgroup */
     DEBUG(2, ("No matching domain found for [%s], fail!\n",
               step_ctx->name));
-    netgr->ready = true;
-    netgr->entries = NULL;
     return ENOENT;
 }
 
-- 
1.7.3.2

-------------- next part --------------
From 727d01ec9971ce214d23eaf97539fadfd4a23587 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 14 Dec 2010 11:31:16 +0100
Subject: [PATCH 09/11] Fix uninitialized value error in setup_test in fail_over-tests.c

https://fedorahosted.org/sssd/ticket/728
---
 src/tests/fail_over-tests.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/tests/fail_over-tests.c b/src/tests/fail_over-tests.c
index 1864a09..a96549d 100644
--- a/src/tests/fail_over-tests.c
+++ b/src/tests/fail_over-tests.c
@@ -78,6 +78,7 @@ setup_test(void)
         fail("Could not init resolv context");
     }
 
+    memset(&fopts, 0, sizeof(fopts));
     fopts.retry_timeout = 30;
     fopts.family_order  = IPV4_FIRST;
 
-- 
1.7.3.2

-------------- next part --------------
From 609ddb3e95ab675abfceb2f777e0ed1b86625b49 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 14 Dec 2010 22:14:34 +0100
Subject: [PATCH 10/11] Fix improper bit manipulation in pam_sss

https://fedorahosted.org/sssd/ticket/715
---
 src/sss_client/pam_sss.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c
index 2031dbc..0745dc6 100644
--- a/src/sss_client/pam_sss.c
+++ b/src/sss_client/pam_sss.c
@@ -1506,7 +1506,7 @@ static int pam_sss(enum sss_cli_command task, pam_handle_t *pamh,
             retry = true;
             retries--;
 
-            flags &= !FLAGS_USE_FIRST_PASS;
+            flags &= ~FLAGS_USE_FIRST_PASS;
             ret = pam_set_item(pamh, PAM_AUTHTOK, NULL);
             if (ret != PAM_SUCCESS) {
                 D(("Failed to unset PAM_AUTHTOK [%s]",
-- 
1.7.3.2

-------------- next part --------------
From b96183baa127cdd500e1a5b8ff8ec2506571718f Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Tue, 14 Dec 2010 22:44:24 +0100
Subject: [PATCH 11/11] Fix possible memory leak in sss_nss_recv_rep()

https://fedorahosted.org/sssd/ticket/723
---
 src/sss_client/common.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/sss_client/common.c b/src/sss_client/common.c
index 0efdbdf..905c0df 100644
--- a/src/sss_client/common.c
+++ b/src/sss_client/common.c
@@ -170,11 +170,13 @@ static enum nss_status sss_nss_send_req(enum sss_cli_command cmd,
  */
 
 static enum nss_status sss_nss_recv_rep(enum sss_cli_command cmd,
-                                        uint8_t **buf, int *len,
+                                        uint8_t **_buf, int *_len,
                                         int *errnop)
 {
     uint32_t header[4];
     size_t datarecv;
+    uint8_t *buf;
+    int len;
 
     header[0] = SSS_NSS_HEADER_SIZE; /* unitl we know the real lenght */
     header[1] = 0;
@@ -182,8 +184,8 @@ static enum nss_status sss_nss_recv_rep(enum sss_cli_command cmd,
     header[3] = 0;
 
     datarecv = 0;
-    *buf = NULL;
-    *len = 0;
+    buf = NULL;
+    len = 0;
     *errnop = 0;
 
     while (datarecv < header[0]) {
@@ -237,7 +239,7 @@ static enum nss_status sss_nss_recv_rep(enum sss_cli_command cmd,
         } else {
             bufrecv = datarecv - SSS_NSS_HEADER_SIZE;
             res = read(sss_cli_sd,
-                       (char *)(*buf) + bufrecv,
+                       (char *) buf + bufrecv,
                        header[0] - datarecv);
         }
         error = errno;
@@ -262,7 +264,7 @@ static enum nss_status sss_nss_recv_rep(enum sss_cli_command cmd,
 
         datarecv += res;
 
-        if (datarecv == SSS_NSS_HEADER_SIZE && *len == 0) {
+        if (datarecv == SSS_NSS_HEADER_SIZE && len == 0) {
             /* at this point recv buf is not yet
              * allocated and the header has just
              * been read, do checks and proceed */
@@ -283,9 +285,9 @@ static enum nss_status sss_nss_recv_rep(enum sss_cli_command cmd,
                 return NSS_STATUS_UNAVAIL;
             }
             if (header[0] > SSS_NSS_HEADER_SIZE) {
-                *len = header[0] - SSS_NSS_HEADER_SIZE;
-                *buf = malloc(*len);
-                if (!*buf) {
+                len = header[0] - SSS_NSS_HEADER_SIZE;
+                buf = malloc(len);
+                if (!buf) {
                     sss_cli_close_socket();
                     *errnop =  ENOMEM;
                     return NSS_STATUS_UNAVAIL;
@@ -294,6 +296,9 @@ static enum nss_status sss_nss_recv_rep(enum sss_cli_command cmd,
         }
     }
 
+    *_len = len;
+    *_buf = buf;
+
     return NSS_STATUS_SUCCESS;
 }
 
-- 
1.7.3.2



More information about the sssd-devel mailing list