[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v2 3/3] crypto: use auto cleanup for many stack vari
From: |
Daniel P . Berrangé |
Subject: |
[Qemu-devel] [PATCH v2 3/3] crypto: use auto cleanup for many stack variables |
Date: |
Thu, 25 Jul 2019 09:43:41 +0100 |
Simplify cleanup paths by using glib's auto cleanup macros for stack
variables, allowing several goto jumps / labels to be eliminated.
Signed-off-by: Daniel P. Berrangé <address@hidden>
---
crypto/afsplit.c | 28 +++++-----------
crypto/block-luks.c | 74 +++++++++++++------------------------------
crypto/block.c | 15 +++------
crypto/pbkdf.c | 5 +--
crypto/secret.c | 38 ++++++++++------------
crypto/tlscredsanon.c | 16 ++++------
crypto/tlscredspsk.c | 5 ++-
crypto/tlscredsx509.c | 16 +++-------
8 files changed, 65 insertions(+), 132 deletions(-)
diff --git a/crypto/afsplit.c b/crypto/afsplit.c
index 328d68c96b..b1a5a20899 100644
--- a/crypto/afsplit.c
+++ b/crypto/afsplit.c
@@ -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[] = {
@@ -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;
@@ -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,
@@ -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;
}
@@ -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,
@@ -141,7 +134,7 @@ int qcrypto_afsplit_decode(QCryptoHashAlgorithm hash,
block);
if (qcrypto_afsplit_hash(hash, blocklen, block,
errp) < 0) {
- goto cleanup;
+ return -1;
}
}
@@ -149,10 +142,5 @@ int qcrypto_afsplit_decode(QCryptoHashAlgorithm hash,
in + (i * blocklen),
block,
out);
-
- ret = 0;
-
- cleanup:
- g_free(block);
- return ret;
+ return 0;
}
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index 409ab50f20..c8e0a0caa4 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -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) {
@@ -456,7 +455,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
slot->iterations,
possiblekey, masterkeylen,
errp) < 0) {
- goto cleanup;
+ return -1;
}
/*
@@ -472,7 +471,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
opaque,
errp);
if (rv < 0) {
- goto cleanup;
+ return -1;
}
@@ -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,
@@ -493,7 +492,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
possiblekey, masterkeylen,
errp);
if (!ivgen) {
- goto cleanup;
+ return -1;
}
@@ -512,7 +511,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
splitkey,
splitkeylen,
errp) < 0) {
- goto cleanup;
+ return -1;
}
/*
@@ -525,7 +524,7 @@ qcrypto_block_luks_load_key(QCryptoBlock *block,
splitkey,
masterkey,
errp) < 0) {
- goto cleanup;
+ return -1;
}
@@ -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;
}
@@ -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;
@@ -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) {
@@ -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;
}
@@ -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;
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;
@@ -1311,15 +1297,7 @@ 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;
@@ -1327,17 +1305,9 @@ qcrypto_block_luks_create(QCryptoBlock *block,
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);
diff --git a/crypto/block.c b/crypto/block.c
index ee96759f7d..325752871c 100644
--- a/crypto/block.c
+++ b/crypto/block.c
@@ -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) {
@@ -320,19 +318,19 @@ 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++;
@@ -340,10 +338,7 @@ static int do_qcrypto_block_cipher_encdec(QCryptoCipher
*cipher,
len -= nbytes;
}
- ret = 0;
- cleanup:
- g_free(iv);
- return ret;
+ return 0;
}
diff --git a/crypto/pbkdf.c b/crypto/pbkdf.c
index b7c7c4a59b..3775ddc6c5 100644
--- a/crypto/pbkdf.c
+++ b/crypto/pbkdf.c
@@ -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;
@@ -108,6 +106,5 @@ uint64_t qcrypto_pbkdf2_count_iters(QCryptoHashAlgorithm
hash,
cleanup:
memset(out, 0, nout);
- g_free(out);
return ret;
}
diff --git a/crypto/secret.c b/crypto/secret.c
index a75d50ae0c..daab81575f 100644
--- a/crypto/secret.c
+++ b/crypto/secret.c
@@ -72,10 +72,12 @@ static void qcrypto_secret_decrypt(QCryptoSecret *secret,
size_t *outputlen,
Error **errp)
{
- uint8_t *key = NULL, *ciphertext = NULL, *iv = NULL;
+ g_autofree uint8_t *key = NULL;
+ g_autofree uint8_t *ciphertext = NULL;
+ g_autofree uint8_t *iv = NULL;
size_t keylen, ciphertextlen, ivlen;
- QCryptoCipher *aes = NULL;
- uint8_t *plaintext = NULL;
+ g_autoptr(QCryptoCipher) aes = NULL;
+ g_autofree uint8_t *plaintext = NULL;
*output = NULL;
*outputlen = 0;
@@ -83,27 +85,27 @@ static void qcrypto_secret_decrypt(QCryptoSecret *secret,
if (qcrypto_secret_lookup(secret->keyid,
&key, &keylen,
errp) < 0) {
- goto cleanup;
+ return;
}
if (keylen != 32) {
error_setg(errp, "Key should be 32 bytes in length");
- goto cleanup;
+ return;
}
if (!secret->iv) {
error_setg(errp, "IV is required to decrypt secret");
- goto cleanup;
+ return;
}
iv = qbase64_decode(secret->iv, -1, &ivlen, errp);
if (!iv) {
- goto cleanup;
+ return;
}
if (ivlen != 16) {
error_setg(errp, "IV should be 16 bytes in length not %zu",
ivlen);
- goto cleanup;
+ return;
}
aes = qcrypto_cipher_new(QCRYPTO_CIPHER_ALG_AES_256,
@@ -111,11 +113,11 @@ static void qcrypto_secret_decrypt(QCryptoSecret *secret,
key, keylen,
errp);
if (!aes) {
- goto cleanup;
+ return;
}
if (qcrypto_cipher_setiv(aes, iv, ivlen, errp) < 0) {
- goto cleanup;
+ return;
}
if (secret->format == QCRYPTO_SECRET_FORMAT_BASE64) {
@@ -124,7 +126,7 @@ static void qcrypto_secret_decrypt(QCryptoSecret *secret,
&ciphertextlen,
errp);
if (!ciphertext) {
- goto cleanup;
+ return;
}
plaintext = g_new0(uint8_t, ciphertextlen + 1);
} else {
@@ -136,8 +138,7 @@ static void qcrypto_secret_decrypt(QCryptoSecret *secret,
plaintext,
ciphertextlen,
errp) < 0) {
- plaintext = NULL;
- goto cleanup;
+ return;
}
if (plaintext[ciphertextlen - 1] > 16 ||
@@ -145,9 +146,7 @@ static void qcrypto_secret_decrypt(QCryptoSecret *secret,
error_setg(errp, "Incorrect number of padding bytes (%d) "
"found on decrypted data",
(int)plaintext[ciphertextlen - 1]);
- g_free(plaintext);
- plaintext = NULL;
- goto cleanup;
+ return;
}
/* Even though plaintext may contain arbitrary NUL
@@ -158,12 +157,7 @@ static void qcrypto_secret_decrypt(QCryptoSecret *secret,
*output = plaintext;
*outputlen = ciphertextlen;
-
- cleanup:
- g_free(ciphertext);
- g_free(iv);
- g_free(key);
- qcrypto_cipher_free(aes);
+ plaintext = NULL;
}
diff --git a/crypto/tlscredsanon.c b/crypto/tlscredsanon.c
index d2adc7c131..a235f60146 100644
--- a/crypto/tlscredsanon.c
+++ b/crypto/tlscredsanon.c
@@ -34,9 +34,8 @@ static int
qcrypto_tls_creds_anon_load(QCryptoTLSCredsAnon *creds,
Error **errp)
{
- char *dhparams = NULL;
+ g_autofree char *dhparams = NULL;
int ret;
- int rv = -1;
trace_qcrypto_tls_creds_anon_load(creds,
creds->parent_obj.dir ? creds->parent_obj.dir : "<nodir>");
@@ -45,20 +44,20 @@ qcrypto_tls_creds_anon_load(QCryptoTLSCredsAnon *creds,
if (qcrypto_tls_creds_get_path(&creds->parent_obj,
QCRYPTO_TLS_CREDS_DH_PARAMS,
false, &dhparams, errp) < 0) {
- goto cleanup;
+ return -1;
}
ret = gnutls_anon_allocate_server_credentials(&creds->data.server);
if (ret < 0) {
error_setg(errp, "Cannot allocate credentials: %s",
gnutls_strerror(ret));
- goto cleanup;
+ return -1;
}
if (qcrypto_tls_creds_get_dh_params_file(&creds->parent_obj, dhparams,
&creds->parent_obj.dh_params,
errp) < 0) {
- goto cleanup;
+ return -1;
}
gnutls_anon_set_server_dh_params(creds->data.server,
@@ -68,14 +67,11 @@ qcrypto_tls_creds_anon_load(QCryptoTLSCredsAnon *creds,
if (ret < 0) {
error_setg(errp, "Cannot allocate credentials: %s",
gnutls_strerror(ret));
- goto cleanup;
+ return -1;
}
}
- rv = 0;
- cleanup:
- g_free(dhparams);
- return rv;
+ return 0;
}
diff --git a/crypto/tlscredspsk.c b/crypto/tlscredspsk.c
index 4b6cf636ce..15d12e2448 100644
--- a/crypto/tlscredspsk.c
+++ b/crypto/tlscredspsk.c
@@ -69,7 +69,8 @@ static int
qcrypto_tls_creds_psk_load(QCryptoTLSCredsPSK *creds,
Error **errp)
{
- char *pskfile = NULL, *dhparams = NULL;
+ g_autofree char *pskfile = NULL;
+ g_autofree char *dhparams = NULL;
const char *username;
int ret;
int rv = -1;
@@ -139,8 +140,6 @@ qcrypto_tls_creds_psk_load(QCryptoTLSCredsPSK *creds,
rv = 0;
cleanup:
g_free(key.data);
- g_free(pskfile);
- g_free(dhparams);
return rv;
}
diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 56dcef3673..01fc304e5d 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -378,7 +378,7 @@ qcrypto_tls_creds_load_cert(QCryptoTLSCredsX509 *creds,
{
gnutls_datum_t data;
gnutls_x509_crt_t cert = NULL;
- char *buf = NULL;
+ g_autofree char *buf = NULL;
gsize buflen;
GError *gerr;
int ret = -1;
@@ -420,7 +420,6 @@ qcrypto_tls_creds_load_cert(QCryptoTLSCredsX509 *creds,
gnutls_x509_crt_deinit(cert);
cert = NULL;
}
- g_free(buf);
return cert;
}
@@ -434,9 +433,8 @@ qcrypto_tls_creds_load_ca_cert_list(QCryptoTLSCredsX509
*creds,
Error **errp)
{
gnutls_datum_t data;
- char *buf = NULL;
+ g_autofree char *buf = NULL;
gsize buflen;
- int ret = -1;
GError *gerr = NULL;
*ncerts = 0;
@@ -446,7 +444,7 @@ qcrypto_tls_creds_load_ca_cert_list(QCryptoTLSCredsX509
*creds,
error_setg(errp, "Cannot load CA cert list %s: %s",
certFile, gerr->message);
g_error_free(gerr);
- goto cleanup;
+ return -1;
}
data.data = (unsigned char *)buf;
@@ -457,15 +455,11 @@ qcrypto_tls_creds_load_ca_cert_list(QCryptoTLSCredsX509
*creds,
error_setg(errp,
"Unable to import CA certificate list %s",
certFile);
- goto cleanup;
+ return -1;
}
*ncerts = certMax;
- ret = 0;
-
- cleanup:
- g_free(buf);
- return ret;
+ return 0;
}
--
2.21.0