From: "Daniel P. Berrangé" <berrange@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Subject: [Qemu-devel] [PATCH v2 3/3] crypto: use auto cleanup for many stack variables
Date: Thu, 25 Jul 2019 09:43:41 +0100 [thread overview]
Message-ID: <20190725084341.8287-4-berrange@redhat.com> (raw)
In-Reply-To: <20190725084341.8287-1-berrange@redhat.com>
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é <berrange@redhat.com>
---
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
next prev parent reply other threads:[~2019-07-25 8:44 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-25 8:43 [Qemu-devel] [PATCH v2 0/3] require newer glib2 to enable autofree'ing of stack variables exiting scope Daniel P. Berrangé
2019-07-25 8:43 ` [Qemu-devel] [PATCH v2 1/3] glib: bump min required glib library version to 2.48 Daniel P. Berrangé
2019-07-25 8:43 ` [Qemu-devel] [PATCH v2 2/3] crypto: define cleanup functions for use with g_autoptr Daniel P. Berrangé
2019-07-25 8:43 ` Daniel P. Berrangé [this message]
2019-07-29 14:58 ` [Qemu-devel] [PATCH v2 3/3] crypto: use auto cleanup for many stack variables Stefan Hajnoczi
2019-07-31 12:59 ` [Qemu-devel] [PATCH v2 0/3] require newer glib2 to enable autofree'ing of stack variables exiting scope Marc-André Lureau
2019-07-31 14:04 ` Alex Bennée
2019-07-31 14:08 ` Eric Blake
2019-07-31 14:10 ` Daniel P. Berrangé
2019-07-31 14:33 ` Alex Bennée
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190725084341.8287-4-berrange@redhat.com \
--to=berrange@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.