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

Sumit Bose sbose at redhat.com
Wed Dec 15 14:22:59 UTC 2010


Hi,

here are some more fixes for bugs found by Coverity.

bye,
Sumit
-------------- next part --------------
From 1e82b3a7570af268c107d9f50407515dd51906a4 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 15 Dec 2010 14:26:16 +0100
Subject: [PATCH 1/4] Fix uninitialized value error in main() in stress-tests.c

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

diff --git a/src/tests/stress-tests.c b/src/tests/stress-tests.c
index b645c25..5075348 100644
--- a/src/tests/stress-tests.c
+++ b/src/tests/stress-tests.c
@@ -274,6 +274,7 @@ int main(int argc, const char *argv[])
 
     /* Reap the children in a handler asynchronously so we can
      * somehow protect against too many processes */
+    memset(&action, 0, sizeof(action));
     action.sa_handler = child_handler;
     sigemptyset(&action.sa_mask);
     sigaddset(&action.sa_mask, SIGCHLD);
-- 
1.7.3.2

-------------- next part --------------
From e5952b1c4a3613f65ea97234ac79c1f6aa7dc463 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 15 Dec 2010 14:55:27 +0100
Subject: [PATCH 2/4] Fix possible memory leak in do_pam_conversation

https://fedorahosted.org/sssd/ticket/731
---
 src/sss_client/pam_sss.c |   44 ++++++++++++++++++++++++++++----------------
 1 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c
index 0745dc6..539acb0 100644
--- a/src/sss_client/pam_sss.c
+++ b/src/sss_client/pam_sss.c
@@ -305,7 +305,7 @@ enum {
 static int do_pam_conversation(pam_handle_t *pamh, const int msg_style,
                                const char *msg,
                                const char *reenter_msg,
-                               char **answer)
+                               char **_answer)
 {
     int ret;
     int state = SSS_PAM_CONV_STD;
@@ -313,13 +313,14 @@ static int do_pam_conversation(pam_handle_t *pamh, const int msg_style,
     const struct pam_message *mesg[1];
     struct pam_message *pam_msg;
     struct pam_response *resp=NULL;
+    char *answer = NULL;
 
     if ((msg_style == PAM_TEXT_INFO || msg_style == PAM_ERROR_MSG) &&
         msg == NULL) return PAM_SYSTEM_ERR;
 
     if ((msg_style == PAM_PROMPT_ECHO_OFF ||
          msg_style == PAM_PROMPT_ECHO_ON) &&
-        (msg == NULL || answer == NULL)) return PAM_SYSTEM_ERR;
+        (msg == NULL || _answer == NULL)) return PAM_SYSTEM_ERR;
 
     if (msg_style == PAM_TEXT_INFO || msg_style == PAM_ERROR_MSG) {
         logger(pamh, LOG_INFO, "User %s message: %s",
@@ -334,7 +335,8 @@ static int do_pam_conversation(pam_handle_t *pamh, const int msg_style,
         pam_msg = malloc(sizeof(struct pam_message));
         if (pam_msg == NULL) {
             D(("Malloc failed."));
-            return PAM_SYSTEM_ERR;
+            ret = PAM_SYSTEM_ERR;
+            goto failed;
         }
 
         pam_msg->msg_style = msg_style;
@@ -351,48 +353,52 @@ static int do_pam_conversation(pam_handle_t *pamh, const int msg_style,
         free(pam_msg);
         if (ret != PAM_SUCCESS) {
             D(("Conversation failure: %s.",  pam_strerror(pamh,ret)));
-            return ret;
+            goto failed;
         }
 
         if (msg_style == PAM_PROMPT_ECHO_OFF ||
             msg_style == PAM_PROMPT_ECHO_ON) {
             if (resp == NULL) {
                 D(("response expected, but resp==NULL"));
-                return PAM_SYSTEM_ERR;
+                ret = PAM_SYSTEM_ERR;
+                goto failed;
             }
 
             if (state == SSS_PAM_CONV_REENTER) {
-                if (null_strcmp(*answer, resp[0].resp) != 0) {
+                if (null_strcmp(answer, resp[0].resp) != 0) {
                     logger(pamh, LOG_NOTICE, "Passwords do not match.");
                     _pam_overwrite((void *)resp[0].resp);
                     free(resp[0].resp);
-                    if (*answer != NULL) {
-                        _pam_overwrite((void *)*answer);
-                        free(*answer);
-                        *answer = NULL;
+                    if (answer != NULL) {
+                        _pam_overwrite((void *) answer);
+                        free(answer);
+                        answer = NULL;
                     }
                     ret = do_pam_conversation(pamh, PAM_ERROR_MSG,
                                               _("Passwords do not match"),
                                               NULL, NULL);
                     if (ret != PAM_SUCCESS) {
                         D(("do_pam_conversation failed."));
-                        return PAM_SYSTEM_ERR;
+                        ret = PAM_SYSTEM_ERR;
+                        goto failed;
                     }
-                    return PAM_CRED_ERR;
+                    ret = PAM_CRED_ERR;
+                    goto failed;
                 }
                 _pam_overwrite((void *)resp[0].resp);
                 free(resp[0].resp);
             } else {
                 if (resp[0].resp == NULL) {
                     D(("Empty password"));
-                    *answer = NULL;
+                    answer = NULL;
                 } else {
-                    *answer = strndup(resp[0].resp, MAX_AUTHTOK_SIZE);
+                    answer = strndup(resp[0].resp, MAX_AUTHTOK_SIZE);
                     _pam_overwrite((void *)resp[0].resp);
                     free(resp[0].resp);
-                    if(*answer == NULL) {
+                    if(answer == NULL) {
                         D(("strndup failed"));
-                        return PAM_BUF_ERR;
+                        ret = PAM_BUF_ERR;
+                        goto failed;
                     }
                 }
             }
@@ -407,7 +413,13 @@ static int do_pam_conversation(pam_handle_t *pamh, const int msg_style,
         }
     } while (state != SSS_PAM_CONV_DONE);
 
+    *_answer = answer;
     return PAM_SUCCESS;
+
+failed:
+    free(answer);
+    return ret;
+
 }
 
 static errno_t display_pw_reset_message(pam_handle_t *pamh,
-- 
1.7.3.2

-------------- next part --------------
From 9e3eef02b91ab6c894665c63928a775635b291cb Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 15 Dec 2010 15:06:39 +0100
Subject: [PATCH 3/4] Fix another possible memory leak in sss_nss_recv_rep()

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

diff --git a/src/sss_client/common.c b/src/sss_client/common.c
index 905c0df..20def9b 100644
--- a/src/sss_client/common.c
+++ b/src/sss_client/common.c
@@ -175,8 +175,9 @@ static enum nss_status sss_nss_recv_rep(enum sss_cli_command cmd,
 {
     uint32_t header[4];
     size_t datarecv;
-    uint8_t *buf;
+    uint8_t *buf = NULL;
     int len;
+    int ret;
 
     header[0] = SSS_NSS_HEADER_SIZE; /* unitl we know the real lenght */
     header[1] = 0;
@@ -228,7 +229,8 @@ static enum nss_status sss_nss_recv_rep(enum sss_cli_command cmd,
         }
         if (*errnop) {
             sss_cli_close_socket();
-            return NSS_STATUS_UNAVAIL;
+            ret = NSS_STATUS_UNAVAIL;
+            goto failed;
         }
 
         errno = 0;
@@ -259,7 +261,8 @@ static enum nss_status sss_nss_recv_rep(enum sss_cli_command cmd,
 
             sss_cli_close_socket();
             *errnop = errno;
-            return NSS_STATUS_UNAVAIL;
+            ret = NSS_STATUS_UNAVAIL;
+            goto failed;
         }
 
         datarecv += res;
@@ -273,24 +276,28 @@ static enum nss_status sss_nss_recv_rep(enum sss_cli_command cmd,
                 sss_cli_close_socket();
                 *errnop = header[2];
                 if (*errnop == EAGAIN) {
-                    return NSS_STATUS_TRYAGAIN;
+                    ret = NSS_STATUS_TRYAGAIN;
+                    goto failed;
                 } else {
-                    return NSS_STATUS_UNAVAIL;
+                    ret = NSS_STATUS_UNAVAIL;
+                    goto failed;
                 }
             }
             if (header[1] != cmd) {
                 /* wrong command id */
                 sss_cli_close_socket();
                 *errnop = EBADMSG;
-                return NSS_STATUS_UNAVAIL;
+                ret = NSS_STATUS_UNAVAIL;
+                goto failed;
             }
             if (header[0] > SSS_NSS_HEADER_SIZE) {
                 len = header[0] - SSS_NSS_HEADER_SIZE;
                 buf = malloc(len);
                 if (!buf) {
                     sss_cli_close_socket();
-                    *errnop =  ENOMEM;
-                    return NSS_STATUS_UNAVAIL;
+                    *errnop = ENOMEM;
+                    ret = NSS_STATUS_UNAVAIL;
+                    goto failed;
                 }
             }
         }
@@ -300,6 +307,10 @@ static enum nss_status sss_nss_recv_rep(enum sss_cli_command cmd,
     *_buf = buf;
 
     return NSS_STATUS_SUCCESS;
+
+failed:
+    free(buf);
+    return ret;
 }
 
 /* this function will check command codes match and returned length is ok */
-- 
1.7.3.2

-------------- next part --------------
From f529bcd6bd7368f5d516c64bb762613e0728e434 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 15 Dec 2010 15:16:07 +0100
Subject: [PATCH 4/4] Fix memory leak of library handle in proxy

https://fedorahosted.org/sssd/ticket/733
---
 src/providers/proxy/proxy.h      |    1 +
 src/providers/proxy/proxy_init.c |   43 ++++++++++++++++++++++---------------
 2 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/src/providers/proxy/proxy.h b/src/providers/proxy/proxy.h
index 9faa967..fdf037c 100644
--- a/src/providers/proxy/proxy.h
+++ b/src/providers/proxy/proxy.h
@@ -86,6 +86,7 @@ struct proxy_id_ctx {
     struct be_ctx *be;
     int entry_cache_timeout;
     struct proxy_nss_ops ops;
+    void *handle;
 };
 
 struct proxy_auth_ctx {
diff --git a/src/providers/proxy/proxy_init.c b/src/providers/proxy/proxy_init.c
index 0993fee..d281dea 100644
--- a/src/providers/proxy/proxy_init.c
+++ b/src/providers/proxy/proxy_init.c
@@ -93,7 +93,6 @@ int sssm_proxy_id_init(struct be_ctx *bectx,
     struct proxy_id_ctx *ctx;
     char *libname;
     char *libpath;
-    void *handle;
     int ret;
 
     ctx = talloc_zero(bectx, struct proxy_id_ctx);
@@ -121,86 +120,92 @@ int sssm_proxy_id_init(struct be_ctx *bectx,
         goto done;
     }
 
-    handle = dlopen(libpath, RTLD_NOW);
-    if (!handle) {
+    ctx->handle = dlopen(libpath, RTLD_NOW);
+    if (!ctx->handle) {
         DEBUG(0, ("Unable to load %s module with path, error: %s\n",
                   libpath, dlerror()));
         ret = ELIBACC;
         goto done;
     }
 
-    ctx->ops.getpwnam_r = proxy_dlsym(handle, "_nss_%s_getpwnam_r", libname);
+    ctx->ops.getpwnam_r = proxy_dlsym(ctx->handle, "_nss_%s_getpwnam_r",
+                                      libname);
     if (!ctx->ops.getpwnam_r) {
         DEBUG(0, ("Failed to load NSS fns, error: %s\n", dlerror()));
         ret = ELIBBAD;
         goto done;
     }
 
-    ctx->ops.getpwuid_r = proxy_dlsym(handle, "_nss_%s_getpwuid_r", libname);
+    ctx->ops.getpwuid_r = proxy_dlsym(ctx->handle, "_nss_%s_getpwuid_r",
+                                      libname);
     if (!ctx->ops.getpwuid_r) {
         DEBUG(0, ("Failed to load NSS fns, error: %s\n", dlerror()));
         ret = ELIBBAD;
         goto done;
     }
 
-    ctx->ops.setpwent = proxy_dlsym(handle, "_nss_%s_setpwent", libname);
+    ctx->ops.setpwent = proxy_dlsym(ctx->handle, "_nss_%s_setpwent", libname);
     if (!ctx->ops.setpwent) {
         DEBUG(0, ("Failed to load NSS fns, error: %s\n", dlerror()));
         ret = ELIBBAD;
         goto done;
     }
 
-    ctx->ops.getpwent_r = proxy_dlsym(handle, "_nss_%s_getpwent_r", libname);
+    ctx->ops.getpwent_r = proxy_dlsym(ctx->handle, "_nss_%s_getpwent_r",
+                                      libname);
     if (!ctx->ops.getpwent_r) {
         DEBUG(0, ("Failed to load NSS fns, error: %s\n", dlerror()));
         ret = ELIBBAD;
         goto done;
     }
 
-    ctx->ops.endpwent = proxy_dlsym(handle, "_nss_%s_endpwent", libname);
+    ctx->ops.endpwent = proxy_dlsym(ctx->handle, "_nss_%s_endpwent", libname);
     if (!ctx->ops.endpwent) {
         DEBUG(0, ("Failed to load NSS fns, error: %s\n", dlerror()));
         ret = ELIBBAD;
         goto done;
     }
 
-    ctx->ops.getgrnam_r = proxy_dlsym(handle, "_nss_%s_getgrnam_r", libname);
+    ctx->ops.getgrnam_r = proxy_dlsym(ctx->handle, "_nss_%s_getgrnam_r",
+                                      libname);
     if (!ctx->ops.getgrnam_r) {
         DEBUG(0, ("Failed to load NSS fns, error: %s\n", dlerror()));
         ret = ELIBBAD;
         goto done;
     }
 
-    ctx->ops.getgrgid_r = proxy_dlsym(handle, "_nss_%s_getgrgid_r", libname);
+    ctx->ops.getgrgid_r = proxy_dlsym(ctx->handle, "_nss_%s_getgrgid_r",
+                                      libname);
     if (!ctx->ops.getgrgid_r) {
         DEBUG(0, ("Failed to load NSS fns, error: %s\n", dlerror()));
         ret = ELIBBAD;
         goto done;
     }
 
-    ctx->ops.setgrent = proxy_dlsym(handle, "_nss_%s_setgrent", libname);
+    ctx->ops.setgrent = proxy_dlsym(ctx->handle, "_nss_%s_setgrent", libname);
     if (!ctx->ops.setgrent) {
         DEBUG(0, ("Failed to load NSS fns, error: %s\n", dlerror()));
         ret = ELIBBAD;
         goto done;
     }
 
-    ctx->ops.getgrent_r = proxy_dlsym(handle, "_nss_%s_getgrent_r", libname);
+    ctx->ops.getgrent_r = proxy_dlsym(ctx->handle, "_nss_%s_getgrent_r",
+                                      libname);
     if (!ctx->ops.getgrent_r) {
         DEBUG(0, ("Failed to load NSS fns, error: %s\n", dlerror()));
         ret = ELIBBAD;
         goto done;
     }
 
-    ctx->ops.endgrent = proxy_dlsym(handle, "_nss_%s_endgrent", libname);
+    ctx->ops.endgrent = proxy_dlsym(ctx->handle, "_nss_%s_endgrent", libname);
     if (!ctx->ops.endgrent) {
         DEBUG(0, ("Failed to load NSS fns, error: %s\n", dlerror()));
         ret = ELIBBAD;
         goto done;
     }
 
-    ctx->ops.initgroups_dyn = proxy_dlsym(handle, "_nss_%s_initgroups_dyn",
-                                                  libname);
+    ctx->ops.initgroups_dyn = proxy_dlsym(ctx->handle, "_nss_%s_initgroups_dyn",
+                                          libname);
     if (!ctx->ops.initgroups_dyn) {
         DEBUG(1, ("The '%s' library does not provides the "
                   "_nss_XXX_initgroups_dyn function!\n"
@@ -208,14 +213,15 @@ int sssm_proxy_id_init(struct be_ctx *bectx,
                   "full groups enumeration!\n", libname));
     }
 
-    ctx->ops.setnetgrent = proxy_dlsym(handle, "_nss_%s_setnetgrent", libname);
+    ctx->ops.setnetgrent = proxy_dlsym(ctx->handle, "_nss_%s_setnetgrent",
+                                       libname);
     if (!ctx->ops.setnetgrent) {
         DEBUG(0, ("Failed to load _nss_%s_setnetgrent, error: %s. "
                   "The library does not support netgroups.\n", libname,
                                                                dlerror()));
     }
 
-    ctx->ops.getnetgrent_r = proxy_dlsym(handle, "_nss_%s_getnetgrent_r",
+    ctx->ops.getnetgrent_r = proxy_dlsym(ctx->handle, "_nss_%s_getnetgrent_r",
                                          libname);
     if (!ctx->ops.getgrent_r) {
         DEBUG(0, ("Failed to load _nss_%s_getnetgrent_r, error: %s. "
@@ -223,7 +229,8 @@ int sssm_proxy_id_init(struct be_ctx *bectx,
                                                                dlerror()));
     }
 
-    ctx->ops.endnetgrent = proxy_dlsym(handle, "_nss_%s_endnetgrent", libname);
+    ctx->ops.endnetgrent = proxy_dlsym(ctx->handle, "_nss_%s_endnetgrent",
+                                       libname);
     if (!ctx->ops.endnetgrent) {
         DEBUG(0, ("Failed to load _nss_%s_endnetgrent, error: %s. "
                   "The library does not support netgroups.\n", libname,
-- 
1.7.3.2



More information about the sssd-devel mailing list