From 89e09afdf6408d090617094e667bcca1b87dd86e Mon Sep 17 00:00:00 2001 From: Milan Broz Date: Wed, 19 Sep 2012 13:58:00 +0200 Subject: [PATCH] Fix some problems found by Coverity static analysis. --- lib/luks1/af.c | 7 ++++++- lib/luks1/keymanage.c | 8 +++++--- lib/setup.c | 17 +++++++++-------- lib/utils_crypt.c | 4 ++++ lib/utils_devpath.c | 2 +- lib/verity/verity_hash.c | 5 ++--- src/cryptsetup.c | 2 +- src/cryptsetup_reencrypt.c | 8 +++++--- 8 files changed, 33 insertions(+), 20 deletions(-) diff --git a/lib/luks1/af.c b/lib/luks1/af.c index 5e7df64..9c50c16 100644 --- a/lib/luks1/af.c +++ b/lib/luks1/af.c @@ -66,9 +66,14 @@ out: static int diffuse(char *src, char *dst, size_t size, const char *hash_name) { - unsigned int digest_size = crypt_hash_size(hash_name); + int hash_size = crypt_hash_size(hash_name); + unsigned int digest_size; unsigned int i, blocks, padding; + if (hash_size <= 0) + return 1; + digest_size = hash_size; + blocks = size / digest_size; padding = size % digest_size; diff --git a/lib/luks1/keymanage.c b/lib/luks1/keymanage.c index 0fcdd98..52df159 100644 --- a/lib/luks1/keymanage.c +++ b/lib/luks1/keymanage.c @@ -784,7 +784,7 @@ int LUKS_set_key(unsigned int keyIndex, r = crypt_random_get(ctx, hdr->keyblock[keyIndex].passwordSalt, LUKS_SALTSIZE, CRYPT_RND_SALT); if (r < 0) - return r; + goto out; r = PBKDF2_HMAC(hdr->hashSpec, password,passwordLen, hdr->keyblock[keyIndex].passwordSalt,LUKS_SALTSIZE, @@ -883,8 +883,10 @@ static int LUKS_open_key(unsigned int keyIndex, assert(vk->keylength == hdr->keyBytes); AFEKSize = AF_split_sectors(vk->keylength, hdr->keyblock[keyIndex].stripes) * SECTOR_SIZE; AfKey = crypt_safe_alloc(AFEKSize); - if (!AfKey) - return -ENOMEM; + if (!AfKey) { + r = -ENOMEM; + goto out; + } r = PBKDF2_HMAC(hdr->hashSpec, password,passwordLen, hdr->keyblock[keyIndex].passwordSalt,LUKS_SALTSIZE, diff --git a/lib/setup.c b/lib/setup.c index d0b25ac..ee9e9ad 100644 --- a/lib/setup.c +++ b/lib/setup.c @@ -625,7 +625,7 @@ static int _crypt_load_verity(struct crypt_device *cd, struct crypt_params_verit if (r < 0) return r; - if (params->flags & CRYPT_VERITY_NO_HEADER) + if (params && params->flags & CRYPT_VERITY_NO_HEADER) return -EINVAL; if (params) @@ -1068,7 +1068,8 @@ static int _crypt_format_verity(struct crypt_device *cd, return -ENOMEM; cd->verity_hdr.flags = params->flags; - cd->verity_hdr.hash_name = strdup(params->hash_name); + if (!(cd->verity_hdr.hash_name = strdup(params->hash_name))) + return -ENOMEM; cd->verity_hdr.data_device = NULL; cd->verity_hdr.data_block_size = params->data_block_size; cd->verity_hdr.hash_block_size = params->hash_block_size; @@ -1076,7 +1077,9 @@ static int _crypt_format_verity(struct crypt_device *cd, cd->verity_hdr.hash_type = params->hash_type; cd->verity_hdr.flags = params->flags; cd->verity_hdr.salt_size = params->salt_size; - cd->verity_hdr.salt = malloc(params->salt_size); + if (!(cd->verity_hdr.salt = malloc(params->salt_size))) + return -ENOMEM; + if (params->salt) memcpy(CONST_CAST(char*)cd->verity_hdr.salt, params->salt, params->salt_size); @@ -1372,7 +1375,7 @@ int crypt_suspend(struct crypt_device *cd, log_dbg("Suspending volume %s.", name); - if (!isLUKS(cd->type)) { + if (!cd || !isLUKS(cd->type)) { log_err(cd, _("This operation is supported only for LUKS device.\n")); r = -EINVAL; goto out; @@ -1384,8 +1387,7 @@ int crypt_suspend(struct crypt_device *cd, return -EINVAL; } - if (!cd) - dm_backend_init(); + dm_backend_init(); r = dm_status_suspended(cd, name); if (r < 0) @@ -1403,8 +1405,7 @@ int crypt_suspend(struct crypt_device *cd, else if (r) log_err(cd, "Error during suspending device %s.\n", name); out: - if (!cd) - dm_backend_exit(); + dm_backend_exit(); return r; } diff --git a/lib/utils_crypt.c b/lib/utils_crypt.c index 204932f..a5d9904 100644 --- a/lib/utils_crypt.c +++ b/lib/utils_crypt.c @@ -94,6 +94,7 @@ void *crypt_safe_alloc(size_t size) alloc->size = size; memset(&alloc->data, 0, size); + /* coverity[leaked_storage] */ return &alloc->data; } @@ -510,10 +511,13 @@ int crypt_string_to_size(struct crypt_device *cd, const char *s, uint64_t *size) break; case 't': case 'T': mult *= mult_base; + /* Fall through */ case 'g': case 'G': mult *= mult_base; + /* Fall through */ case 'm': case 'M': mult *= mult_base; + /* Fall through */ case 'k': case 'K': mult *= mult_base; break; diff --git a/lib/utils_devpath.c b/lib/utils_devpath.c index f22f422..472569e 100644 --- a/lib/utils_devpath.c +++ b/lib/utils_devpath.c @@ -139,7 +139,7 @@ char *crypt_lookup_dev(const char *dev_id) if (snprintf(path, sizeof(path), "/sys/dev/block/%s", dev_id) < 0) return NULL; - len = readlink(path, link, sizeof(link)); + len = readlink(path, link, sizeof(link) - 1); if (len < 0) { /* Without /sys use old scan */ if (stat("/sys/dev/block", &st) < 0) diff --git a/lib/verity/verity_hash.c b/lib/verity/verity_hash.c index d755f88..2da47b2 100644 --- a/lib/verity/verity_hash.c +++ b/lib/verity/verity_hash.c @@ -273,9 +273,8 @@ static int VERITY_create_or_verify_hash(struct crypt_device *cd, s = data_file_blocks >> (i * hash_per_block_bits); s = (s + hash_per_block - 1) / hash_per_block; hash_level_size[i] = s; - if (hash_position + s < hash_position || - (hash_position + s) < 0 || - (hash_position + s) != hash_position + s) { + if ((hash_position + s) < hash_position || + (hash_position + s) < 0) { log_err(cd, _("Device offset overflow.\n")); return -EINVAL; } diff --git a/src/cryptsetup.c b/src/cryptsetup.c index 32aebf6..5927844 100644 --- a/src/cryptsetup.c +++ b/src/cryptsetup.c @@ -54,6 +54,7 @@ static int opt_test_passphrase = 0; static const char **action_argv; static int action_argc; +static const char *null_action_argv[] = {NULL, NULL}; static int action_create(int arg); static int action_remove(int arg); @@ -1125,7 +1126,6 @@ int main(int argc, const char **argv) struct action_type *action; const char *aname; int r; - const char *null_action_argv[] = {NULL}; crypt_set_log_callback(NULL, tool_log, NULL); diff --git a/src/cryptsetup_reencrypt.c b/src/cryptsetup_reencrypt.c index 17d1c88..2dc2a8e 100644 --- a/src/cryptsetup_reencrypt.c +++ b/src/cryptsetup_reencrypt.c @@ -183,8 +183,8 @@ static int device_check(struct reenc_ctx *rc, header_magic set_magic) s = read(devfd, buf, SECTOR_SIZE); if (s < 0 || s != SECTOR_SIZE) { log_err(_("Cannot read device %s.\n"), rc->device); - close(devfd); - return -EIO; + r = -EIO; + goto out; } /* Be sure that we do not process new version of header */ @@ -290,7 +290,9 @@ static int write_log(struct reenc_ctx *rc) 1, rc->device_uuid, rc->reencrypt_direction, rc->device_offset, rc->device_shift); - lseek(rc->log_fd, 0, SEEK_SET); + if (lseek(rc->log_fd, 0, SEEK_SET) == -1) + return -EIO; + r = write(rc->log_fd, rc->log_buf, SECTOR_SIZE); if (r < 0 || r != SECTOR_SIZE) { log_err(_("Cannot write reencryption log file.\n")); -- 2.7.4