Hi, here are some more fixes for bugs found by Coverity.
bye, Sumit
From 1e82b3a7570af268c107d9f50407515dd51906a4 Mon Sep 17 00:00:00 2001 From: Sumit Bose <[email protected]> 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
From e5952b1c4a3613f65ea97234ac79c1f6aa7dc463 Mon Sep 17 00:00:00 2001 From: Sumit Bose <[email protected]> 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
From 9e3eef02b91ab6c894665c63928a775635b291cb Mon Sep 17 00:00:00 2001 From: Sumit Bose <[email protected]> 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
From f529bcd6bd7368f5d516c64bb762613e0728e434 Mon Sep 17 00:00:00 2001 From: Sumit Bose <[email protected]> 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
_______________________________________________ sssd-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/sssd-devel
