From 9fe7a83675fd197fef7823ede3d8bc5a617ac198 Mon Sep 17 00:00:00 2001 From: Mark Riddoch Date: Wed, 21 Aug 2013 17:26:55 +0200 Subject: [PATCH] Fixes for resource leaks foudn with Coverity. Bug 173, 174, 175, 176 and 177 --- server/core/adminusers.c | 20 ++++++++++++++------ server/core/dcb.c | 2 ++ server/core/gw_utils.c | 3 +++ server/core/secrets.c | 10 ++++++++++ server/core/server.c | 11 +++++++++-- 5 files changed, 38 insertions(+), 8 deletions(-) diff --git a/server/core/adminusers.c b/server/core/adminusers.c index 07850bfc1..d82a38e99 100644 --- a/server/core/adminusers.c +++ b/server/core/adminusers.c @@ -122,7 +122,10 @@ char uname[80], passwd[80]; if ((fp = fopen(fname, "r")) == NULL) return NULL; if ((rval = users_alloc()) == NULL) + { + fclose(fp); return NULL; + } while (fscanf(fp, "%[^:]:%s\n", uname, passwd) == 2) { users_add(rval, uname, passwd); @@ -274,13 +277,15 @@ char* admin_remove_user( * Scan passwd and copy all but matching lines to temp file. */ if (fgetpos(fp, &rpos) != 0) { - int err = errno; - skygw_log_write( LOGFILE_ERROR, + int err = errno; + skygw_log_write( LOGFILE_ERROR, "Unable to process passwd file %s : errno %d.\n" "Removing user from file failed, and must be done manually.", fname, err); - return ADMIN_ERR_PWDFILEACCESS; + fclose(fp_tmp); + unlink(fname_tmp); + return ADMIN_ERR_PWDFILEACCESS; } while (fscanf(fp, "%[^:]:%s\n", fusr, fpwd) == 2) @@ -303,6 +308,8 @@ char* admin_remove_user( "done manually.", fname, err); + fclose(fp_tmp); + unlink(fname_tmp); return ADMIN_ERR_PWDFILEACCESS; } } @@ -311,14 +318,15 @@ char* admin_remove_user( * Replace original passwd file with new. */ if (rename(fname_tmp, fname)) { - int err = errno; - skygw_log_write( LOGFILE_ERROR, + int err = errno; + skygw_log_write( LOGFILE_ERROR, "Unable to rename new passwd file %s : errno %d.\n" "Rename it to %s manually.", fname_tmp, err, fname); - return ADMIN_ERR_PWDFILEACCESS; + unlink(fname_tmp); + return ADMIN_ERR_PWDFILEACCESS; } fclose(fp_tmp); diff --git a/server/core/dcb.c b/server/core/dcb.c index a84079727..cef229146 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -376,6 +376,7 @@ int eno = 0; if (n < 0) { + gwbuf_free(buffer); if ((errno == EAGAIN) || (errno == EWOULDBLOCK)) { return n; @@ -387,6 +388,7 @@ int eno = 0; } else if (n == 0) { + gwbuf_free(buffer); return n; } diff --git a/server/core/gw_utils.c b/server/core/gw_utils.c index f9ab33b82..104a890e3 100644 --- a/server/core/gw_utils.c +++ b/server/core/gw_utils.c @@ -106,9 +106,11 @@ int gw_read_gwbuff(DCB *dcb, GWBUF **head, int b) { if (n < 0) { if ((errno == EAGAIN) || (errno == EWOULDBLOCK)) { fprintf(stderr, "Client connection %i: continue for %i, %s\n", dcb->fd, errno, strerror(errno)); + gwbuf_free(buffer); return 1; } else { fprintf(stderr, "Client connection %i error: %i, %s\n", dcb->fd, errno, strerror(errno));; + gwbuf_free(buffer); (dcb->func).close(dcb); return 1; } @@ -117,6 +119,7 @@ int gw_read_gwbuff(DCB *dcb, GWBUF **head, int b) { if (n == 0) { // socket closed fprintf(stderr, "Client connection %i closed: %i, %s\n", dcb->fd, errno, strerror(errno)); + gwbuf_free(buffer); (dcb->func).close(dcb); return 1; } diff --git a/server/core/secrets.c b/server/core/secrets.c index 8498f131e..0b34e4abb 100644 --- a/server/core/secrets.c +++ b/server/core/secrets.c @@ -106,12 +106,14 @@ int fd; /* read all data from file */ if (read(fd, keys, sizeof(MAXKEYS)) != sizeof(MAXKEYS)) { + free(keys); skygw_log_write( LOGFILE_ERROR, "secrets_readKeys, failed reading from secret file [%s]. Error %i, %s\n", secret_file, errno, strerror(errno)); return NULL; } /* Close the file */ if (close(fd) < 0) { + free(keys); skygw_log_write( LOGFILE_ERROR, "secrets_readKeys, failed closing the secret file [%s]. Error %i, %s\n", secret_file, errno, strerror(errno)); return NULL; } @@ -188,14 +190,22 @@ int enlen; return strdup(crypt); /* If the input is not a HEX string return the input - it probably was not encrypted */ for (ptr = crypt; *ptr; ptr++) + { if (!isxdigit(*ptr)) + { + free(keys); return strdup(crypt); + } + } enlen = strlen(crypt) / 2; gw_hex2bin(encrypted, crypt, strlen(crypt)); if ((plain = (unsigned char *)malloc(80)) == NULL) + { + free(keys); return NULL; + } AES_set_decrypt_key(keys->enckey, 8 * MAXSCALE_KEYLEN, &aeskey); diff --git a/server/core/server.c b/server/core/server.c index 2d7b3cf1f..d59a7ca62 100644 --- a/server/core/server.c +++ b/server/core/server.c @@ -182,6 +182,7 @@ void dprintAllServers(DCB *dcb) { SERVER *ptr; +char *stat; spinlock_acquire(&server_spin); ptr = allServers; @@ -189,7 +190,9 @@ SERVER *ptr; { dcb_printf(dcb, "Server %p\n", ptr); dcb_printf(dcb, "\tServer: %s\n", ptr->name); - dcb_printf(dcb, "\tStatus: %s\n", server_status(ptr)); + stat = server_status(ptr); + dcb_printf(dcb, "\tStatus: %s\n", stat); + free(stat); dcb_printf(dcb, "\tProtocol: %s\n", ptr->protocol); dcb_printf(dcb, "\tPort: %d\n", ptr->port); dcb_printf(dcb, "\tNumber of connections: %d\n", ptr->stats.n_connections); @@ -208,9 +211,13 @@ SERVER *ptr; void dprintServer(DCB *dcb, SERVER *server) { +char *stat; + dcb_printf(dcb, "Server %p\n", server); dcb_printf(dcb, "\tServer: %s\n", server->name); - dcb_printf(dcb, "\tStatus: %s\n", server_status(server)); + stat = server_status(server); + dcb_printf(dcb, "\tStatus: %s\n", stat); + free(stat); dcb_printf(dcb, "\tProtocol: %s\n", server->protocol); dcb_printf(dcb, "\tPort: %d\n", server->port); dcb_printf(dcb, "\tNumber of connections: %d\n", server->stats.n_connections);