[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