Skip to content

Commit

Permalink
crypto: use auto cleanup for many stack variables
Browse files Browse the repository at this point in the history 
Simplify cleanup paths by using glib's auto cleanup macros for stack
variables, allowing several goto jumps / labels to be eliminated.

Reviewed-by: Marc-André Lureau <[email protected]>
Reviewed-by: Stefan Hajnoczi <[email protected]>
Signed-off-by: Daniel P. Berrangé <[email protected]>
  • Loading branch information
@berrange
berrange committed Aug 22, 2019
1 parent 133cf1e commit 57b9f11
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 133 deletions.
28 changes: 8 additions & 20 deletions crypto/afsplit.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ static int qcrypto_afsplit_hash(QCryptoHashAlgorithm hash,
}

for (i = 0; i < hashcount; i++) {
uint8_t *out = NULL;
g_autofree uint8_t *out = NULL;
size_t outlen = 0;
uint32_t iv = cpu_to_be32(i);
struct iovec in[] = {
Expand All @@ -79,7 +79,6 @@ static int qcrypto_afsplit_hash(QCryptoHashAlgorithm hash,
assert(outlen == digestlen);
memcpy(block + (i * digestlen), out,
(i == (hashcount - 1)) ? finallen : digestlen);
g_free(out);
}

return 0;
Expand All @@ -93,13 +92,12 @@ int qcrypto_afsplit_encode(QCryptoHashAlgorithm hash,
uint8_t *out,
Error **errp)
{
uint8_t *block = g_new0(uint8_t, blocklen);
g_autofree uint8_t *block = g_new0(uint8_t, blocklen);
size_t i;
int ret = -1;

for (i = 0; i < (stripes - 1); i++) {
if (qcrypto_random_bytes(out + (i * blocklen), blocklen, errp) < 0) {
goto cleanup;
return -1;
}

qcrypto_afsplit_xor(blocklen,
Expand All @@ -108,18 +106,14 @@ int qcrypto_afsplit_encode(QCryptoHashAlgorithm hash,
block);
if (qcrypto_afsplit_hash(hash, blocklen, block,
errp) < 0) {
goto cleanup;
return -1;
}
}
qcrypto_afsplit_xor(blocklen,
in,
block,
out + (i * blocklen));
ret = 0;

cleanup:
g_free(block);
return ret;
return 0;
}


Expand All @@ -130,9 +124,8 @@ int qcrypto_afsplit_decode(QCryptoHashAlgorithm hash,
uint8_t *out,
Error **errp)
{
uint8_t *block = g_new0(uint8_t, blocklen);
g_autofree uint8_t *block = g_new0(uint8_t, blocklen);
size_t i;
int ret = -1;

for (i = 0; i < (stripes - 1); i++) {
qcrypto_afsplit_xor(blocklen,
Expand All @@ -141,18 +134,13 @@ int qcrypto_afsplit_decode(QCryptoHashAlgorithm hash,
block);
if (qcrypto_afsplit_hash(hash, blocklen, block,
errp) < 0) {
goto cleanup;
return -1;
}
}

qcrypto_afsplit_xor(blocklen,
in + (i * blocklen),
block,
out);

ret = 0;

cleanup:
g_free(block);
return ret;
return 0;
}
74 changes: 22 additions & 52 deletions crypto/block-luks.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,14 +425,13 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
Error **errp)
{
QCryptoBlockLUKS *luks = block->opaque;
uint8_t *splitkey;
g_autofree uint8_t *splitkey = NULL;
size_t splitkeylen;
uint8_t *possiblekey;
int ret = -1;
g_autofree uint8_t *possiblekey = NULL;
ssize_t rv;
QCryptoCipher *cipher = NULL;
g_autoptr(QCryptoCipher) cipher = NULL;
uint8_t keydigest[QCRYPTO_BLOCK_LUKS_DIGEST_LEN];
QCryptoIVGen *ivgen = NULL;
g_autoptr(QCryptoIVGen) ivgen = NULL;
size_t niv;

if (slot->active != QCRYPTO_BLOCK_LUKS_KEY_SLOT_ENABLED) {
Expand All @@ -456,7 +455,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
slot->iterations,
possiblekey, masterkeylen,
errp) < 0) {
goto cleanup;
return -1;
}

/*
Expand All @@ -472,7 +471,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
opaque,
errp);
if (rv < 0) {
goto cleanup;
return -1;
}


Expand All @@ -482,7 +481,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
possiblekey, masterkeylen,
errp);
if (!cipher) {
goto cleanup;
return -1;
}

niv = qcrypto_cipher_get_iv_len(cipheralg,
Expand All @@ -493,7 +492,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
possiblekey, masterkeylen,
errp);
if (!ivgen) {
goto cleanup;
return -1;
}


Expand All @@ -512,7 +511,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
splitkey,
splitkeylen,
errp) < 0) {
goto cleanup;
return -1;
}

/*
Expand All @@ -525,7 +524,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
splitkey,
masterkey,
errp) < 0) {
goto cleanup;
return -1;
}


Expand All @@ -544,26 +543,18 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
luks->header.master_key_iterations,
keydigest, G_N_ELEMENTS(keydigest),
errp) < 0) {
goto cleanup;
return -1;
}

if (memcmp(keydigest, luks->header.master_key_digest,
QCRYPTO_BLOCK_LUKS_DIGEST_LEN) == 0) {
/* Success, we got the right master key */
ret = 1;
goto cleanup;
return 1;
}

/* Fail, user's password was not valid for this key slot,
* tell caller to try another slot */
ret = 0;

cleanup:
qcrypto_ivgen_free(ivgen);
qcrypto_cipher_free(cipher);
g_free(splitkey);
g_free(possiblekey);
return ret;
return 0;
}


Expand Down Expand Up @@ -644,7 +635,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
int ret = 0;
size_t i;
ssize_t rv;
uint8_t *masterkey = NULL;
g_autofree uint8_t *masterkey = NULL;
size_t masterkeylen;
char *ivgen_name, *ivhash_name;
QCryptoCipherMode ciphermode;
Expand All @@ -653,7 +644,7 @@ qcrypto_block_luks_open(QCryptoBlock *block,
QCryptoCipherAlgorithm ivcipheralg;
QCryptoHashAlgorithm hash;
QCryptoHashAlgorithm ivhash;
char *password = NULL;
g_autofree char *password = NULL;

if (!(flags & QCRYPTO_BLOCK_OPEN_NO_IO)) {
if (!options->u.luks.key_secret) {
Expand Down Expand Up @@ -856,17 +847,12 @@ qcrypto_block_luks_open(QCryptoBlock *block,
luks->ivgen_hash_alg = ivhash;
luks->hash_alg = hash;

g_free(masterkey);
g_free(password);

return 0;

fail:
g_free(masterkey);
qcrypto_block_free_cipher(block);
qcrypto_ivgen_free(block->ivgen);
g_free(luks);
g_free(password);
return ret;
}

Expand All @@ -891,20 +877,20 @@ qcrypto_block_luks_create(QCryptoBlock *block,
QCryptoBlockLUKS *luks;
QCryptoBlockCreateOptionsLUKS luks_opts;
Error *local_err = NULL;
uint8_t *masterkey = NULL;
uint8_t *slotkey = NULL;
uint8_t *splitkey = NULL;
g_autofree uint8_t *masterkey = NULL;
g_autofree uint8_t *slotkey = NULL;
g_autofree uint8_t *splitkey = NULL;
size_t splitkeylen = 0;
size_t i;
QCryptoCipher *cipher = NULL;
QCryptoIVGen *ivgen = NULL;
char *password;
g_autoptr(QCryptoCipher) cipher = NULL;
g_autoptr(QCryptoIVGen) ivgen = NULL;
g_autofree char *password = NULL;
const char *cipher_alg;
const char *cipher_mode;
const char *ivgen_alg;
const char *ivgen_hash_alg = NULL;
const char *hash_alg;
char *cipher_mode_spec = NULL;
g_autofree char *cipher_mode_spec = NULL;
QCryptoCipherAlgorithm ivcipheralg = 0;
uint64_t iters;

Expand Down Expand Up @@ -1311,33 +1297,17 @@ qcrypto_block_luks_create(QCryptoBlock *block,
luks->hash_alg = luks_opts.hash_alg;

memset(masterkey, 0, luks->header.key_bytes);
g_free(masterkey);
memset(slotkey, 0, luks->header.key_bytes);
g_free(slotkey);
g_free(splitkey);
g_free(password);
g_free(cipher_mode_spec);

qcrypto_ivgen_free(ivgen);
qcrypto_cipher_free(cipher);

return 0;

error:
if (masterkey) {
memset(masterkey, 0, luks->header.key_bytes);
}
g_free(masterkey);
if (slotkey) {
memset(slotkey, 0, luks->header.key_bytes);
}
g_free(slotkey);
g_free(splitkey);
g_free(password);
g_free(cipher_mode_spec);

qcrypto_ivgen_free(ivgen);
qcrypto_cipher_free(cipher);

qcrypto_block_free_cipher(block);
qcrypto_ivgen_free(block->ivgen);
Expand Down
15 changes: 5 additions & 10 deletions crypto/block.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,15 +299,13 @@ static int do_qcrypto_block_cipher_encdec(QCryptoCipher *cipher,
QCryptoCipherEncDecFunc func,
Error **errp)
{
uint8_t *iv;
g_autofree uint8_t *iv = niv ? g_new0(uint8_t, niv) : NULL;
int ret = -1;
uint64_t startsector = offset / sectorsize;

assert(QEMU_IS_ALIGNED(offset, sectorsize));
assert(QEMU_IS_ALIGNED(len, sectorsize));

iv = niv ? g_new0(uint8_t, niv) : NULL;

while (len > 0) {
size_t nbytes;
if (niv) {
Expand All @@ -320,30 +318,27 @@ static int do_qcrypto_block_cipher_encdec(QCryptoCipher *cipher,
}

if (ret < 0) {
goto cleanup;
return -1;
}

if (qcrypto_cipher_setiv(cipher,
iv, niv,
errp) < 0) {
goto cleanup;
return -1;
}
}

nbytes = len > sectorsize ? sectorsize : len;
if (func(cipher, buf, buf, nbytes, errp) < 0) {
goto cleanup;
return -1;
}

startsector++;
buf += nbytes;
len -= nbytes;
}

ret = 0;
cleanup:
g_free(iv);
return ret;
return 0;
}


Expand Down
5 changes: 1 addition & 4 deletions crypto/pbkdf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,10 @@ uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,
Error **errp)
{
uint64_t ret = -1;
uint8_t *out;
g_autofree uint8_t *out = g_new(uint8_t, nout);
uint64_t iterations = (1 << 15);
unsigned long long delta_ms, start_ms, end_ms;

out = g_new(uint8_t, nout);

while (1) {
if (qcrypto_pbkdf2_get_thread_cpu(&start_ms, errp) < 0) {
goto cleanup;
Expand Down Expand Up @@ -108,6 +106,5 @@ uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm hash,

cleanup:
memset(out, 0, nout);
g_free(out);
return ret;
}
Loading

0 comments on commit 57b9f11

Please sign in to comment.