| Message ID | 1436278368-13449-11-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
Am 07.07.2015 um 16:12 schrieb Paolo Bonzini: > From: "Daniel P. Berrange" <berrange@redhat.com> > > Switch the qcow/qcow2 block driver over to use the generic cipher > API, this allows it to use the pluggable AES implementations, > instead of being hardcoded to use QEMU's built-in impl. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > Message-Id: <1435770638-25715-10-git-send-email-berrange@redhat.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> For whatever reason this breaks migration(or virsh restore) from guests that were created with an older version of QEMU. Thread 1 (Thread 0x3fffb856bd0 (LWP 32226)): #0 0x0000000080354a54 in qcrypto_cipher_free (cipher=0x0) at /home/cborntra/REPOS/qemu/crypto/cipher-builtin.c:357 #1 0x00000000802ca912 in qcow2_close (bs=0x80a0cea0) at /home/cborntra/REPOS/qemu/block/qcow2.c:1477 #2 0x00000000802caa32 in qcow2_invalidate_cache (bs=0x80a0cea0, errp=0x81a3fc58) at /home/cborntra/REPOS/qemu/block/qcow2.c:1509 #3 0x000000008029bac0 in bdrv_invalidate_cache (bs=0x80a0cea0, errp=0x81a3fd08) at /home/cborntra/REPOS/qemu/block.c:3135 #4 0x000000008029bbe6 in bdrv_invalidate_cache_all (errp=0x81a3fdd0) at /home/cborntra/REPOS/qemu/block.c:3160 #5 0x000000008021de50 in process_incoming_migration_co (opaque=0x80acaae0) at /home/cborntra/REPOS/qemu/migration/migration.c:160 #6 0x00000000802ab96a in coroutine_trampoline (i0=0, i1=-2137149984) at /home/cborntra/REPOS/qemu/coroutine-ucontext.c:80 #7 0x000003fffc463ca2 in __makecontext_ret () from /lib64/libc.so.6 Backtrace stopped: previous frame identical to this frame (corrupt stack?) > --- > block/qcow.c | 102 +++++++++++++++++++++++++++++++++++++------------- > block/qcow2-cluster.c | 46 ++++++++++++++++++----- > block/qcow2.c | 95 +++++++++++++++++++++++----------------------- > block/qcow2.h | 13 +++---- > 4 files changed, 165 insertions(+), 91 deletions(-) > > diff --git a/block/qcow.c b/block/qcow.c > index bf5c570..01fba54 100644 > --- a/block/qcow.c > +++ b/block/qcow.c > @@ -26,7 +26,7 @@ > #include "qemu/module.h" > #include <zlib.h> > #include "qapi/qmp/qerror.h" > -#include "crypto/aes.h" > +#include "crypto/cipher.h" > #include "migration/migration.h" > > /**************************************************************/ > @@ -72,10 +72,8 @@ typedef struct BDRVQcowState { > uint8_t *cluster_cache; > uint8_t *cluster_data; > uint64_t cluster_cache_offset; > - uint32_t crypt_method; /* current crypt method, 0 if no key yet */ > + QCryptoCipher *cipher; /* NULL if no key yet */ > uint32_t crypt_method_header; > - AES_KEY aes_encrypt_key; > - AES_KEY aes_decrypt_key; > CoMutex lock; > Error *migration_blocker; > } BDRVQcowState; > @@ -154,6 +152,11 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, > ret = -EINVAL; > goto fail; > } > + if (!qcrypto_cipher_supports(QCRYPTO_CIPHER_ALG_AES_128)) { > + error_setg(errp, "AES cipher not available"); > + ret = -EINVAL; > + goto fail; > + } > s->crypt_method_header = header.crypt_method; > if (s->crypt_method_header) { > bs->encrypted = 1; > @@ -260,6 +263,7 @@ static int qcow_set_key(BlockDriverState *bs, const char *key) > BDRVQcowState *s = bs->opaque; > uint8_t keybuf[16]; > int len, i; > + Error *err; > > memset(keybuf, 0, 16); > len = strlen(key); > @@ -271,38 +275,67 @@ static int qcow_set_key(BlockDriverState *bs, const char *key) > keybuf[i] = key[i]; > } > assert(bs->encrypted); > - s->crypt_method = s->crypt_method_header; > > - if (AES_set_encrypt_key(keybuf, 128, &s->aes_encrypt_key) != 0) > - return -1; > - if (AES_set_decrypt_key(keybuf, 128, &s->aes_decrypt_key) != 0) > + qcrypto_cipher_free(s->cipher); > + s->cipher = qcrypto_cipher_new( > + QCRYPTO_CIPHER_ALG_AES_128, > + QCRYPTO_CIPHER_MODE_CBC, > + keybuf, G_N_ELEMENTS(keybuf), > + &err); > + > + if (!s->cipher) { > + /* XXX would be nice if errors in this method could > + * be properly propagate to the caller. Would need > + * the bdrv_set_key() API signature to be fixed. */ > + error_free(err); > return -1; > + } > return 0; > } > > /* The crypt function is compatible with the linux cryptoloop > algorithm for < 4 GB images. NOTE: out_buf == in_buf is > supported */ > -static void encrypt_sectors(BDRVQcowState *s, int64_t sector_num, > - uint8_t *out_buf, const uint8_t *in_buf, > - int nb_sectors, int enc, > - const AES_KEY *key) > +static int encrypt_sectors(BDRVQcowState *s, int64_t sector_num, > + uint8_t *out_buf, const uint8_t *in_buf, > + int nb_sectors, bool enc, Error **errp) > { > union { > uint64_t ll[2]; > uint8_t b[16]; > } ivec; > int i; > + int ret; > > for(i = 0; i < nb_sectors; i++) { > ivec.ll[0] = cpu_to_le64(sector_num); > ivec.ll[1] = 0; > - AES_cbc_encrypt(in_buf, out_buf, 512, key, > - ivec.b, enc); > + if (qcrypto_cipher_setiv(s->cipher, > + ivec.b, G_N_ELEMENTS(ivec.b), > + errp) < 0) { > + return -1; > + } > + if (enc) { > + ret = qcrypto_cipher_encrypt(s->cipher, > + in_buf, > + out_buf, > + 512, > + errp); > + } else { > + ret = qcrypto_cipher_decrypt(s->cipher, > + in_buf, > + out_buf, > + 512, > + errp); > + } > + if (ret < 0) { > + return -1; > + } > sector_num++; > in_buf += 512; > out_buf += 512; > } > + return 0; > } > > /* 'allocate' is: > @@ -416,15 +449,20 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, > if (bs->encrypted && > (n_end - n_start) < s->cluster_sectors) { > uint64_t start_sect; > - assert(s->crypt_method); > + assert(s->cipher); > start_sect = (offset & ~(s->cluster_size - 1)) >> 9; > memset(s->cluster_data + 512, 0x00, 512); > for(i = 0; i < s->cluster_sectors; i++) { > if (i < n_start || i >= n_end) { > - encrypt_sectors(s, start_sect + i, > - s->cluster_data, > - s->cluster_data + 512, 1, 1, > - &s->aes_encrypt_key); > + Error *err = NULL; > + if (encrypt_sectors(s, start_sect + i, > + s->cluster_data, > + s->cluster_data + 512, 1, > + true, &err) < 0) { > + error_free(err); > + errno = EIO; > + return -1; > + } > if (bdrv_pwrite(bs->file, cluster_offset + i * 512, > s->cluster_data, 512) != 512) > return -1; > @@ -464,7 +502,7 @@ static int64_t coroutine_fn qcow_co_get_block_status(BlockDriverState *bs, > if (!cluster_offset) { > return 0; > } > - if ((cluster_offset & QCOW_OFLAG_COMPRESSED) || s->crypt_method) { > + if ((cluster_offset & QCOW_OFLAG_COMPRESSED) || s->cipher) { > return BDRV_BLOCK_DATA; > } > cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS); > @@ -531,6 +569,7 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, > QEMUIOVector hd_qiov; > uint8_t *buf; > void *orig_buf; > + Error *err = NULL; > > if (qiov->niov > 1) { > buf = orig_buf = qemu_try_blockalign(bs, qiov->size); > @@ -594,10 +633,11 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, > break; > } > if (bs->encrypted) { > - assert(s->crypt_method); > - encrypt_sectors(s, sector_num, buf, buf, > - n, 0, > - &s->aes_decrypt_key); > + assert(s->cipher); > + if (encrypt_sectors(s, sector_num, buf, buf, > + n, false, &err) < 0) { > + goto fail; > + } > } > } > ret = 0; > @@ -618,6 +658,7 @@ done: > return ret; > > fail: > + error_free(err); > ret = -EIO; > goto done; > } > @@ -666,12 +707,17 @@ static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num, > break; > } > if (bs->encrypted) { > - assert(s->crypt_method); > + Error *err = NULL; > + assert(s->cipher); > if (!cluster_data) { > cluster_data = g_malloc0(s->cluster_size); > } > - encrypt_sectors(s, sector_num, cluster_data, buf, > - n, 1, &s->aes_encrypt_key); > + if (encrypt_sectors(s, sector_num, cluster_data, buf, > + n, true, &err) < 0) { > + error_free(err); > + ret = -EIO; > + break; > + } > src_buf = cluster_data; > } else { > src_buf = buf; > @@ -708,6 +754,8 @@ static void qcow_close(BlockDriverState *bs) > { > BDRVQcowState *s = bs->opaque; > > + qcrypto_cipher_free(s->cipher); > + s->cipher = NULL; > g_free(s->l1_table); > qemu_vfree(s->l2_cache); > g_free(s->cluster_cache); > diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c > index 1a5c97a..b43f186 100644 > --- a/block/qcow2-cluster.c > +++ b/block/qcow2-cluster.c > @@ -339,26 +339,47 @@ static int count_contiguous_free_clusters(uint64_t nb_clusters, uint64_t *l2_tab > /* The crypt function is compatible with the linux cryptoloop > algorithm for < 4 GB images. NOTE: out_buf == in_buf is > supported */ > -void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, > - uint8_t *out_buf, const uint8_t *in_buf, > - int nb_sectors, int enc, > - const AES_KEY *key) > +int qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, > + uint8_t *out_buf, const uint8_t *in_buf, > + int nb_sectors, bool enc, > + Error **errp) > { > union { > uint64_t ll[2]; > uint8_t b[16]; > } ivec; > int i; > + int ret; > > for(i = 0; i < nb_sectors; i++) { > ivec.ll[0] = cpu_to_le64(sector_num); > ivec.ll[1] = 0; > - AES_cbc_encrypt(in_buf, out_buf, 512, key, > - ivec.b, enc); > + if (qcrypto_cipher_setiv(s->cipher, > + ivec.b, G_N_ELEMENTS(ivec.b), > + errp) < 0) { > + return -1; > + } > + if (enc) { > + ret = qcrypto_cipher_encrypt(s->cipher, > + in_buf, > + out_buf, > + 512, > + errp); > + } else { > + ret = qcrypto_cipher_decrypt(s->cipher, > + in_buf, > + out_buf, > + 512, > + errp); > + } > + if (ret < 0) { > + return -1; > + } > sector_num++; > in_buf += 512; > out_buf += 512; > } > + return 0; > } > > static int coroutine_fn copy_sectors(BlockDriverState *bs, > @@ -401,10 +422,15 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs, > } > > if (bs->encrypted) { > - assert(s->crypt_method); > - qcow2_encrypt_sectors(s, start_sect + n_start, > - iov.iov_base, iov.iov_base, n, 1, > - &s->aes_encrypt_key); > + Error *err = NULL; > + assert(s->cipher); > + if (qcow2_encrypt_sectors(s, start_sect + n_start, > + iov.iov_base, iov.iov_base, n, > + true, &err) < 0) { > + ret = -EIO; > + error_free(err); > + goto out; > + } > } > > ret = qcow2_pre_write_overlap_check(bs, 0, > diff --git a/block/qcow2.c b/block/qcow2.c > index 85e0731..76c331b 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -698,6 +698,11 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, > ret = -EINVAL; > goto fail; > } > + if (!qcrypto_cipher_supports(QCRYPTO_CIPHER_ALG_AES_128)) { > + error_setg(errp, "AES cipher not available"); > + ret = -EINVAL; > + goto fail; > + } > s->crypt_method_header = header.crypt_method; > if (s->crypt_method_header) { > bs->encrypted = 1; > @@ -1031,6 +1036,7 @@ static int qcow2_set_key(BlockDriverState *bs, const char *key) > BDRVQcowState *s = bs->opaque; > uint8_t keybuf[16]; > int len, i; > + Error *err = NULL; > > memset(keybuf, 0, 16); > len = strlen(key); > @@ -1042,30 +1048,21 @@ static int qcow2_set_key(BlockDriverState *bs, const char *key) > keybuf[i] = key[i]; > } > assert(bs->encrypted); > - s->crypt_method = s->crypt_method_header; > > - if (AES_set_encrypt_key(keybuf, 128, &s->aes_encrypt_key) != 0) > - return -1; > - if (AES_set_decrypt_key(keybuf, 128, &s->aes_decrypt_key) != 0) > + qcrypto_cipher_free(s->cipher); > + s->cipher = qcrypto_cipher_new( > + QCRYPTO_CIPHER_ALG_AES_128, > + QCRYPTO_CIPHER_MODE_CBC, > + keybuf, G_N_ELEMENTS(keybuf), > + &err); > + > + if (!s->cipher) { > + /* XXX would be nice if errors in this method could > + * be properly propagate to the caller. Would need > + * the bdrv_set_key() API signature to be fixed. */ > + error_free(err); > return -1; > -#if 0 > - /* test */ > - { > - uint8_t in[16]; > - uint8_t out[16]; > - uint8_t tmp[16]; > - for(i=0;i<16;i++) > - in[i] = i; > - AES_encrypt(in, tmp, &s->aes_encrypt_key); > - AES_decrypt(tmp, out, &s->aes_decrypt_key); > - for(i = 0; i < 16; i++) > - printf(" %02x", tmp[i]); > - printf("\n"); > - for(i = 0; i < 16; i++) > - printf(" %02x", out[i]); > - printf("\n"); > } > -#endif > return 0; > } > > @@ -1108,7 +1105,7 @@ static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs, > } > > if (cluster_offset != 0 && ret != QCOW2_CLUSTER_COMPRESSED && > - !s->crypt_method) { > + !s->cipher) { > index_in_cluster = sector_num & (s->cluster_sectors - 1); > cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS); > status |= BDRV_BLOCK_OFFSET_VALID | cluster_offset; > @@ -1158,7 +1155,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num, > > /* prepare next request */ > cur_nr_sectors = remaining_sectors; > - if (s->crypt_method) { > + if (s->cipher) { > cur_nr_sectors = MIN(cur_nr_sectors, > QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors); > } > @@ -1230,7 +1227,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num, > } > > if (bs->encrypted) { > - assert(s->crypt_method); > + assert(s->cipher); > > /* > * For encrypted images, read everything into a temporary > @@ -1263,9 +1260,15 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num, > goto fail; > } > if (bs->encrypted) { > - assert(s->crypt_method); > - qcow2_encrypt_sectors(s, sector_num, cluster_data, > - cluster_data, cur_nr_sectors, 0, &s->aes_decrypt_key); > + assert(s->cipher); > + Error *err = NULL; > + if (qcow2_encrypt_sectors(s, sector_num, cluster_data, > + cluster_data, cur_nr_sectors, false, > + &err) < 0) { > + error_free(err); > + ret = -EIO; > + goto fail; > + } > qemu_iovec_from_buf(qiov, bytes_done, > cluster_data, 512 * cur_nr_sectors); > } > @@ -1343,7 +1346,8 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs, > cur_nr_sectors * 512); > > if (bs->encrypted) { > - assert(s->crypt_method); > + Error *err = NULL; > + assert(s->cipher); > if (!cluster_data) { > cluster_data = qemu_try_blockalign(bs->file, > QCOW_MAX_CRYPT_CLUSTERS > @@ -1358,8 +1362,13 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs, > QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size); > qemu_iovec_to_buf(&hd_qiov, 0, cluster_data, hd_qiov.size); > > - qcow2_encrypt_sectors(s, sector_num, cluster_data, > - cluster_data, cur_nr_sectors, 1, &s->aes_encrypt_key); > + if (qcow2_encrypt_sectors(s, sector_num, cluster_data, > + cluster_data, cur_nr_sectors, > + true, &err) < 0) { > + error_free(err); > + ret = -EIO; > + goto fail; > + } > > qemu_iovec_reset(&hd_qiov); > qemu_iovec_add(&hd_qiov, cluster_data, > @@ -1465,6 +1474,9 @@ static void qcow2_close(BlockDriverState *bs) > qcow2_cache_destroy(bs, s->l2_table_cache); > qcow2_cache_destroy(bs, s->refcount_block_cache); > > + qcrypto_cipher_free(s->cipher); > + s->cipher = NULL; > + > g_free(s->unknown_header_fields); > cleanup_unknown_header_ext(bs); > > @@ -1481,9 +1493,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) > { > BDRVQcowState *s = bs->opaque; > int flags = s->flags; > - AES_KEY aes_encrypt_key; > - AES_KEY aes_decrypt_key; > - uint32_t crypt_method = 0; > + QCryptoCipher *cipher = NULL; > QDict *options; > Error *local_err = NULL; > int ret; > @@ -1493,12 +1503,8 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) > * that means we don't have to worry about reopening them here. > */ > > - if (bs->encrypted) { > - assert(s->crypt_method); > - crypt_method = s->crypt_method; > - memcpy(&aes_encrypt_key, &s->aes_encrypt_key, sizeof(aes_encrypt_key)); > - memcpy(&aes_decrypt_key, &s->aes_decrypt_key, sizeof(aes_decrypt_key)); > - } > + cipher = s->cipher; > + s->cipher = NULL; > > qcow2_close(bs); > > @@ -1523,11 +1529,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) > return; > } > > - if (bs->encrypted) { > - s->crypt_method = crypt_method; > - memcpy(&s->aes_encrypt_key, &aes_encrypt_key, sizeof(aes_encrypt_key)); > - memcpy(&s->aes_decrypt_key, &aes_decrypt_key, sizeof(aes_decrypt_key)); > - } > + s->cipher = cipher; > } > > static size_t header_ext_add(char *buf, uint32_t magic, const void *s, > @@ -2728,8 +2730,9 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts, > backing_format = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT); > } else if (!strcmp(desc->name, BLOCK_OPT_ENCRYPT)) { > encrypt = qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT, > - s->crypt_method); > - if (encrypt != !!s->crypt_method) { > + !!s->cipher); > + > + if (encrypt != !!s->cipher) { > fprintf(stderr, "Changing the encryption flag is not " > "supported.\n"); > return -ENOTSUP; > diff --git a/block/qcow2.h b/block/qcow2.h > index 462147c..72e1328 100644 > --- a/block/qcow2.h > +++ b/block/qcow2.h > @@ -25,7 +25,7 @@ > #ifndef BLOCK_QCOW2_H > #define BLOCK_QCOW2_H > > -#include "crypto/aes.h" > +#include "crypto/cipher.h" > #include "block/coroutine.h" > > //#define DEBUG_ALLOC > @@ -253,10 +253,8 @@ typedef struct BDRVQcowState { > > CoMutex lock; > > - uint32_t crypt_method; /* current crypt method, 0 if no key yet */ > + QCryptoCipher *cipher; /* current cipher, NULL if no key yet */ > uint32_t crypt_method_header; > - AES_KEY aes_encrypt_key; > - AES_KEY aes_decrypt_key; > uint64_t snapshots_offset; > int snapshots_size; > unsigned int nb_snapshots; > @@ -536,10 +534,9 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, > int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index); > void qcow2_l2_cache_reset(BlockDriverState *bs); > int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset); > -void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, > - uint8_t *out_buf, const uint8_t *in_buf, > - int nb_sectors, int enc, > - const AES_KEY *key); > +int qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, > + uint8_t *out_buf, const uint8_t *in_buf, > + int nb_sectors, bool enc, Error **errp); > > int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset, > int *num, uint64_t *cluster_offset); >
Forgot some CCs (patch author and migration folks) Am 09.07.2015 um 12:17 schrieb Christian Borntraeger: > Am 07.07.2015 um 16:12 schrieb Paolo Bonzini: >> From: "Daniel P. Berrange" <berrange@redhat.com> >> >> Switch the qcow/qcow2 block driver over to use the generic cipher >> API, this allows it to use the pluggable AES implementations, >> instead of being hardcoded to use QEMU's built-in impl. >> >> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> >> Message-Id: <1435770638-25715-10-git-send-email-berrange@redhat.com> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > For whatever reason this breaks migration(or virsh restore) > from guests that were created with an older version of QEMU. > > > > Thread 1 (Thread 0x3fffb856bd0 (LWP 32226)): > #0 0x0000000080354a54 in qcrypto_cipher_free (cipher=0x0) at /home/cborntra/REPOS/qemu/crypto/cipher-builtin.c:357 > #1 0x00000000802ca912 in qcow2_close (bs=0x80a0cea0) at /home/cborntra/REPOS/qemu/block/qcow2.c:1477 > #2 0x00000000802caa32 in qcow2_invalidate_cache (bs=0x80a0cea0, errp=0x81a3fc58) at /home/cborntra/REPOS/qemu/block/qcow2.c:1509 > #3 0x000000008029bac0 in bdrv_invalidate_cache (bs=0x80a0cea0, errp=0x81a3fd08) at /home/cborntra/REPOS/qemu/block.c:3135 > #4 0x000000008029bbe6 in bdrv_invalidate_cache_all (errp=0x81a3fdd0) at /home/cborntra/REPOS/qemu/block.c:3160 > #5 0x000000008021de50 in process_incoming_migration_co (opaque=0x80acaae0) at /home/cborntra/REPOS/qemu/migration/migration.c:160 > #6 0x00000000802ab96a in coroutine_trampoline (i0=0, i1=-2137149984) at /home/cborntra/REPOS/qemu/coroutine-ucontext.c:80 > #7 0x000003fffc463ca2 in __makecontext_ret () from /lib64/libc.so.6 > Backtrace stopped: previous frame identical to this frame (corrupt stack?) > > >> --- >> block/qcow.c | 102 +++++++++++++++++++++++++++++++++++++------------- >> block/qcow2-cluster.c | 46 ++++++++++++++++++----- >> block/qcow2.c | 95 +++++++++++++++++++++++----------------------- >> block/qcow2.h | 13 +++---- >> 4 files changed, 165 insertions(+), 91 deletions(-) >> >> diff --git a/block/qcow.c b/block/qcow.c >> index bf5c570..01fba54 100644 >> --- a/block/qcow.c >> +++ b/block/qcow.c >> @@ -26,7 +26,7 @@ >> #include "qemu/module.h" >> #include <zlib.h> >> #include "qapi/qmp/qerror.h" >> -#include "crypto/aes.h" >> +#include "crypto/cipher.h" >> #include "migration/migration.h" >> >> /**************************************************************/ >> @@ -72,10 +72,8 @@ typedef struct BDRVQcowState { >> uint8_t *cluster_cache; >> uint8_t *cluster_data; >> uint64_t cluster_cache_offset; >> - uint32_t crypt_method; /* current crypt method, 0 if no key yet */ >> + QCryptoCipher *cipher; /* NULL if no key yet */ >> uint32_t crypt_method_header; >> - AES_KEY aes_encrypt_key; >> - AES_KEY aes_decrypt_key; >> CoMutex lock; >> Error *migration_blocker; >> } BDRVQcowState; >> @@ -154,6 +152,11 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, >> ret = -EINVAL; >> goto fail; >> } >> + if (!qcrypto_cipher_supports(QCRYPTO_CIPHER_ALG_AES_128)) { >> + error_setg(errp, "AES cipher not available"); >> + ret = -EINVAL; >> + goto fail; >> + } >> s->crypt_method_header = header.crypt_method; >> if (s->crypt_method_header) { >> bs->encrypted = 1; >> @@ -260,6 +263,7 @@ static int qcow_set_key(BlockDriverState *bs, const char *key) >> BDRVQcowState *s = bs->opaque; >> uint8_t keybuf[16]; >> int len, i; >> + Error *err; >> >> memset(keybuf, 0, 16); >> len = strlen(key); >> @@ -271,38 +275,67 @@ static int qcow_set_key(BlockDriverState *bs, const char *key) >> keybuf[i] = key[i]; >> } >> assert(bs->encrypted); >> - s->crypt_method = s->crypt_method_header; >> >> - if (AES_set_encrypt_key(keybuf, 128, &s->aes_encrypt_key) != 0) >> - return -1; >> - if (AES_set_decrypt_key(keybuf, 128, &s->aes_decrypt_key) != 0) >> + qcrypto_cipher_free(s->cipher); >> + s->cipher = qcrypto_cipher_new( >> + QCRYPTO_CIPHER_ALG_AES_128, >> + QCRYPTO_CIPHER_MODE_CBC, >> + keybuf, G_N_ELEMENTS(keybuf), >> + &err); >> + >> + if (!s->cipher) { >> + /* XXX would be nice if errors in this method could >> + * be properly propagate to the caller. Would need >> + * the bdrv_set_key() API signature to be fixed. */ >> + error_free(err); >> return -1; >> + } >> return 0; >> } >> >> /* The crypt function is compatible with the linux cryptoloop >> algorithm for < 4 GB images. NOTE: out_buf == in_buf is >> supported */ >> -static void encrypt_sectors(BDRVQcowState *s, int64_t sector_num, >> - uint8_t *out_buf, const uint8_t *in_buf, >> - int nb_sectors, int enc, >> - const AES_KEY *key) >> +static int encrypt_sectors(BDRVQcowState *s, int64_t sector_num, >> + uint8_t *out_buf, const uint8_t *in_buf, >> + int nb_sectors, bool enc, Error **errp) >> { >> union { >> uint64_t ll[2]; >> uint8_t b[16]; >> } ivec; >> int i; >> + int ret; >> >> for(i = 0; i < nb_sectors; i++) { >> ivec.ll[0] = cpu_to_le64(sector_num); >> ivec.ll[1] = 0; >> - AES_cbc_encrypt(in_buf, out_buf, 512, key, >> - ivec.b, enc); >> + if (qcrypto_cipher_setiv(s->cipher, >> + ivec.b, G_N_ELEMENTS(ivec.b), >> + errp) < 0) { >> + return -1; >> + } >> + if (enc) { >> + ret = qcrypto_cipher_encrypt(s->cipher, >> + in_buf, >> + out_buf, >> + 512, >> + errp); >> + } else { >> + ret = qcrypto_cipher_decrypt(s->cipher, >> + in_buf, >> + out_buf, >> + 512, >> + errp); >> + } >> + if (ret < 0) { >> + return -1; >> + } >> sector_num++; >> in_buf += 512; >> out_buf += 512; >> } >> + return 0; >> } >> >> /* 'allocate' is: >> @@ -416,15 +449,20 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, >> if (bs->encrypted && >> (n_end - n_start) < s->cluster_sectors) { >> uint64_t start_sect; >> - assert(s->crypt_method); >> + assert(s->cipher); >> start_sect = (offset & ~(s->cluster_size - 1)) >> 9; >> memset(s->cluster_data + 512, 0x00, 512); >> for(i = 0; i < s->cluster_sectors; i++) { >> if (i < n_start || i >= n_end) { >> - encrypt_sectors(s, start_sect + i, >> - s->cluster_data, >> - s->cluster_data + 512, 1, 1, >> - &s->aes_encrypt_key); >> + Error *err = NULL; >> + if (encrypt_sectors(s, start_sect + i, >> + s->cluster_data, >> + s->cluster_data + 512, 1, >> + true, &err) < 0) { >> + error_free(err); >> + errno = EIO; >> + return -1; >> + } >> if (bdrv_pwrite(bs->file, cluster_offset + i * 512, >> s->cluster_data, 512) != 512) >> return -1; >> @@ -464,7 +502,7 @@ static int64_t coroutine_fn qcow_co_get_block_status(BlockDriverState *bs, >> if (!cluster_offset) { >> return 0; >> } >> - if ((cluster_offset & QCOW_OFLAG_COMPRESSED) || s->crypt_method) { >> + if ((cluster_offset & QCOW_OFLAG_COMPRESSED) || s->cipher) { >> return BDRV_BLOCK_DATA; >> } >> cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS); >> @@ -531,6 +569,7 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, >> QEMUIOVector hd_qiov; >> uint8_t *buf; >> void *orig_buf; >> + Error *err = NULL; >> >> if (qiov->niov > 1) { >> buf = orig_buf = qemu_try_blockalign(bs, qiov->size); >> @@ -594,10 +633,11 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, >> break; >> } >> if (bs->encrypted) { >> - assert(s->crypt_method); >> - encrypt_sectors(s, sector_num, buf, buf, >> - n, 0, >> - &s->aes_decrypt_key); >> + assert(s->cipher); >> + if (encrypt_sectors(s, sector_num, buf, buf, >> + n, false, &err) < 0) { >> + goto fail; >> + } >> } >> } >> ret = 0; >> @@ -618,6 +658,7 @@ done: >> return ret; >> >> fail: >> + error_free(err); >> ret = -EIO; >> goto done; >> } >> @@ -666,12 +707,17 @@ static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num, >> break; >> } >> if (bs->encrypted) { >> - assert(s->crypt_method); >> + Error *err = NULL; >> + assert(s->cipher); >> if (!cluster_data) { >> cluster_data = g_malloc0(s->cluster_size); >> } >> - encrypt_sectors(s, sector_num, cluster_data, buf, >> - n, 1, &s->aes_encrypt_key); >> + if (encrypt_sectors(s, sector_num, cluster_data, buf, >> + n, true, &err) < 0) { >> + error_free(err); >> + ret = -EIO; >> + break; >> + } >> src_buf = cluster_data; >> } else { >> src_buf = buf; >> @@ -708,6 +754,8 @@ static void qcow_close(BlockDriverState *bs) >> { >> BDRVQcowState *s = bs->opaque; >> >> + qcrypto_cipher_free(s->cipher); >> + s->cipher = NULL; >> g_free(s->l1_table); >> qemu_vfree(s->l2_cache); >> g_free(s->cluster_cache); >> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c >> index 1a5c97a..b43f186 100644 >> --- a/block/qcow2-cluster.c >> +++ b/block/qcow2-cluster.c >> @@ -339,26 +339,47 @@ static int count_contiguous_free_clusters(uint64_t nb_clusters, uint64_t *l2_tab >> /* The crypt function is compatible with the linux cryptoloop >> algorithm for < 4 GB images. NOTE: out_buf == in_buf is >> supported */ >> -void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, >> - uint8_t *out_buf, const uint8_t *in_buf, >> - int nb_sectors, int enc, >> - const AES_KEY *key) >> +int qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, >> + uint8_t *out_buf, const uint8_t *in_buf, >> + int nb_sectors, bool enc, >> + Error **errp) >> { >> union { >> uint64_t ll[2]; >> uint8_t b[16]; >> } ivec; >> int i; >> + int ret; >> >> for(i = 0; i < nb_sectors; i++) { >> ivec.ll[0] = cpu_to_le64(sector_num); >> ivec.ll[1] = 0; >> - AES_cbc_encrypt(in_buf, out_buf, 512, key, >> - ivec.b, enc); >> + if (qcrypto_cipher_setiv(s->cipher, >> + ivec.b, G_N_ELEMENTS(ivec.b), >> + errp) < 0) { >> + return -1; >> + } >> + if (enc) { >> + ret = qcrypto_cipher_encrypt(s->cipher, >> + in_buf, >> + out_buf, >> + 512, >> + errp); >> + } else { >> + ret = qcrypto_cipher_decrypt(s->cipher, >> + in_buf, >> + out_buf, >> + 512, >> + errp); >> + } >> + if (ret < 0) { >> + return -1; >> + } >> sector_num++; >> in_buf += 512; >> out_buf += 512; >> } >> + return 0; >> } >> >> static int coroutine_fn copy_sectors(BlockDriverState *bs, >> @@ -401,10 +422,15 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs, >> } >> >> if (bs->encrypted) { >> - assert(s->crypt_method); >> - qcow2_encrypt_sectors(s, start_sect + n_start, >> - iov.iov_base, iov.iov_base, n, 1, >> - &s->aes_encrypt_key); >> + Error *err = NULL; >> + assert(s->cipher); >> + if (qcow2_encrypt_sectors(s, start_sect + n_start, >> + iov.iov_base, iov.iov_base, n, >> + true, &err) < 0) { >> + ret = -EIO; >> + error_free(err); >> + goto out; >> + } >> } >> >> ret = qcow2_pre_write_overlap_check(bs, 0, >> diff --git a/block/qcow2.c b/block/qcow2.c >> index 85e0731..76c331b 100644 >> --- a/block/qcow2.c >> +++ b/block/qcow2.c >> @@ -698,6 +698,11 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, >> ret = -EINVAL; >> goto fail; >> } >> + if (!qcrypto_cipher_supports(QCRYPTO_CIPHER_ALG_AES_128)) { >> + error_setg(errp, "AES cipher not available"); >> + ret = -EINVAL; >> + goto fail; >> + } >> s->crypt_method_header = header.crypt_method; >> if (s->crypt_method_header) { >> bs->encrypted = 1; >> @@ -1031,6 +1036,7 @@ static int qcow2_set_key(BlockDriverState *bs, const char *key) >> BDRVQcowState *s = bs->opaque; >> uint8_t keybuf[16]; >> int len, i; >> + Error *err = NULL; >> >> memset(keybuf, 0, 16); >> len = strlen(key); >> @@ -1042,30 +1048,21 @@ static int qcow2_set_key(BlockDriverState *bs, const char *key) >> keybuf[i] = key[i]; >> } >> assert(bs->encrypted); >> - s->crypt_method = s->crypt_method_header; >> >> - if (AES_set_encrypt_key(keybuf, 128, &s->aes_encrypt_key) != 0) >> - return -1; >> - if (AES_set_decrypt_key(keybuf, 128, &s->aes_decrypt_key) != 0) >> + qcrypto_cipher_free(s->cipher); >> + s->cipher = qcrypto_cipher_new( >> + QCRYPTO_CIPHER_ALG_AES_128, >> + QCRYPTO_CIPHER_MODE_CBC, >> + keybuf, G_N_ELEMENTS(keybuf), >> + &err); >> + >> + if (!s->cipher) { >> + /* XXX would be nice if errors in this method could >> + * be properly propagate to the caller. Would need >> + * the bdrv_set_key() API signature to be fixed. */ >> + error_free(err); >> return -1; >> -#if 0 >> - /* test */ >> - { >> - uint8_t in[16]; >> - uint8_t out[16]; >> - uint8_t tmp[16]; >> - for(i=0;i<16;i++) >> - in[i] = i; >> - AES_encrypt(in, tmp, &s->aes_encrypt_key); >> - AES_decrypt(tmp, out, &s->aes_decrypt_key); >> - for(i = 0; i < 16; i++) >> - printf(" %02x", tmp[i]); >> - printf("\n"); >> - for(i = 0; i < 16; i++) >> - printf(" %02x", out[i]); >> - printf("\n"); >> } >> -#endif >> return 0; >> } >> >> @@ -1108,7 +1105,7 @@ static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs, >> } >> >> if (cluster_offset != 0 && ret != QCOW2_CLUSTER_COMPRESSED && >> - !s->crypt_method) { >> + !s->cipher) { >> index_in_cluster = sector_num & (s->cluster_sectors - 1); >> cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS); >> status |= BDRV_BLOCK_OFFSET_VALID | cluster_offset; >> @@ -1158,7 +1155,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num, >> >> /* prepare next request */ >> cur_nr_sectors = remaining_sectors; >> - if (s->crypt_method) { >> + if (s->cipher) { >> cur_nr_sectors = MIN(cur_nr_sectors, >> QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors); >> } >> @@ -1230,7 +1227,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num, >> } >> >> if (bs->encrypted) { >> - assert(s->crypt_method); >> + assert(s->cipher); >> >> /* >> * For encrypted images, read everything into a temporary >> @@ -1263,9 +1260,15 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num, >> goto fail; >> } >> if (bs->encrypted) { >> - assert(s->crypt_method); >> - qcow2_encrypt_sectors(s, sector_num, cluster_data, >> - cluster_data, cur_nr_sectors, 0, &s->aes_decrypt_key); >> + assert(s->cipher); >> + Error *err = NULL; >> + if (qcow2_encrypt_sectors(s, sector_num, cluster_data, >> + cluster_data, cur_nr_sectors, false, >> + &err) < 0) { >> + error_free(err); >> + ret = -EIO; >> + goto fail; >> + } >> qemu_iovec_from_buf(qiov, bytes_done, >> cluster_data, 512 * cur_nr_sectors); >> } >> @@ -1343,7 +1346,8 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs, >> cur_nr_sectors * 512); >> >> if (bs->encrypted) { >> - assert(s->crypt_method); >> + Error *err = NULL; >> + assert(s->cipher); >> if (!cluster_data) { >> cluster_data = qemu_try_blockalign(bs->file, >> QCOW_MAX_CRYPT_CLUSTERS >> @@ -1358,8 +1362,13 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs, >> QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size); >> qemu_iovec_to_buf(&hd_qiov, 0, cluster_data, hd_qiov.size); >> >> - qcow2_encrypt_sectors(s, sector_num, cluster_data, >> - cluster_data, cur_nr_sectors, 1, &s->aes_encrypt_key); >> + if (qcow2_encrypt_sectors(s, sector_num, cluster_data, >> + cluster_data, cur_nr_sectors, >> + true, &err) < 0) { >> + error_free(err); >> + ret = -EIO; >> + goto fail; >> + } >> >> qemu_iovec_reset(&hd_qiov); >> qemu_iovec_add(&hd_qiov, cluster_data, >> @@ -1465,6 +1474,9 @@ static void qcow2_close(BlockDriverState *bs) >> qcow2_cache_destroy(bs, s->l2_table_cache); >> qcow2_cache_destroy(bs, s->refcount_block_cache); >> >> + qcrypto_cipher_free(s->cipher); >> + s->cipher = NULL; >> + >> g_free(s->unknown_header_fields); >> cleanup_unknown_header_ext(bs); >> >> @@ -1481,9 +1493,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) >> { >> BDRVQcowState *s = bs->opaque; >> int flags = s->flags; >> - AES_KEY aes_encrypt_key; >> - AES_KEY aes_decrypt_key; >> - uint32_t crypt_method = 0; >> + QCryptoCipher *cipher = NULL; >> QDict *options; >> Error *local_err = NULL; >> int ret; >> @@ -1493,12 +1503,8 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) >> * that means we don't have to worry about reopening them here. >> */ >> >> - if (bs->encrypted) { >> - assert(s->crypt_method); >> - crypt_method = s->crypt_method; >> - memcpy(&aes_encrypt_key, &s->aes_encrypt_key, sizeof(aes_encrypt_key)); >> - memcpy(&aes_decrypt_key, &s->aes_decrypt_key, sizeof(aes_decrypt_key)); >> - } >> + cipher = s->cipher; >> + s->cipher = NULL; >> >> qcow2_close(bs); >> >> @@ -1523,11 +1529,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) >> return; >> } >> >> - if (bs->encrypted) { >> - s->crypt_method = crypt_method; >> - memcpy(&s->aes_encrypt_key, &aes_encrypt_key, sizeof(aes_encrypt_key)); >> - memcpy(&s->aes_decrypt_key, &aes_decrypt_key, sizeof(aes_decrypt_key)); >> - } >> + s->cipher = cipher; >> } >> >> static size_t header_ext_add(char *buf, uint32_t magic, const void *s, >> @@ -2728,8 +2730,9 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts, >> backing_format = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT); >> } else if (!strcmp(desc->name, BLOCK_OPT_ENCRYPT)) { >> encrypt = qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT, >> - s->crypt_method); >> - if (encrypt != !!s->crypt_method) { >> + !!s->cipher); >> + >> + if (encrypt != !!s->cipher) { >> fprintf(stderr, "Changing the encryption flag is not " >> "supported.\n"); >> return -ENOTSUP; >> diff --git a/block/qcow2.h b/block/qcow2.h >> index 462147c..72e1328 100644 >> --- a/block/qcow2.h >> +++ b/block/qcow2.h >> @@ -25,7 +25,7 @@ >> #ifndef BLOCK_QCOW2_H >> #define BLOCK_QCOW2_H >> >> -#include "crypto/aes.h" >> +#include "crypto/cipher.h" >> #include "block/coroutine.h" >> >> //#define DEBUG_ALLOC >> @@ -253,10 +253,8 @@ typedef struct BDRVQcowState { >> >> CoMutex lock; >> >> - uint32_t crypt_method; /* current crypt method, 0 if no key yet */ >> + QCryptoCipher *cipher; /* current cipher, NULL if no key yet */ >> uint32_t crypt_method_header; >> - AES_KEY aes_encrypt_key; >> - AES_KEY aes_decrypt_key; >> uint64_t snapshots_offset; >> int snapshots_size; >> unsigned int nb_snapshots; >> @@ -536,10 +534,9 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, >> int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index); >> void qcow2_l2_cache_reset(BlockDriverState *bs); >> int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset); >> -void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, >> - uint8_t *out_buf, const uint8_t *in_buf, >> - int nb_sectors, int enc, >> - const AES_KEY *key); >> +int qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, >> + uint8_t *out_buf, const uint8_t *in_buf, >> + int nb_sectors, bool enc, Error **errp); >> >> int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset, >> int *num, uint64_t *cluster_offset); >> > >
Am 09.07.2015 um 12:53 schrieb Christian Borntraeger: > Forgot some CCs (patch author and migration folks) > > > Am 09.07.2015 um 12:17 schrieb Christian Borntraeger: >> Am 07.07.2015 um 16:12 schrieb Paolo Bonzini: >>> From: "Daniel P. Berrange" <berrange@redhat.com> >>> >>> Switch the qcow/qcow2 block driver over to use the generic cipher >>> API, this allows it to use the pluggable AES implementations, >>> instead of being hardcoded to use QEMU's built-in impl. >>> >>> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> >>> Message-Id: <1435770638-25715-10-git-send-email-berrange@redhat.com> >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> >> For whatever reason this breaks migration(or virsh restore) >> from guests that were created with an older version of QEMU. I should not send emails today :-/ I wanted to say, that this is the stack trace of a crash. >> Thread 1 (Thread 0x3fffb856bd0 (LWP 32226)): >> #0 0x0000000080354a54 in qcrypto_cipher_free (cipher=0x0) at /home/cborntra/REPOS/qemu/crypto/cipher-builtin.c:357 >> #1 0x00000000802ca912 in qcow2_close (bs=0x80a0cea0) at /home/cborntra/REPOS/qemu/block/qcow2.c:1477 >> #2 0x00000000802caa32 in qcow2_invalidate_cache (bs=0x80a0cea0, errp=0x81a3fc58) at /home/cborntra/REPOS/qemu/block/qcow2.c:1509 >> #3 0x000000008029bac0 in bdrv_invalidate_cache (bs=0x80a0cea0, errp=0x81a3fd08) at /home/cborntra/REPOS/qemu/block.c:3135 >> #4 0x000000008029bbe6 in bdrv_invalidate_cache_all (errp=0x81a3fdd0) at /home/cborntra/REPOS/qemu/block.c:3160 >> #5 0x000000008021de50 in process_incoming_migration_co (opaque=0x80acaae0) at /home/cborntra/REPOS/qemu/migration/migration.c:160 >> #6 0x00000000802ab96a in coroutine_trampoline (i0=0, i1=-2137149984) at /home/cborntra/REPOS/qemu/coroutine-ucontext.c:80 >> #7 0x000003fffc463ca2 in __makecontext_ret () from /lib64/libc.so.6 >> Backtrace stopped: previous frame identical to this frame (corrupt stack?) >> >> >>> --- >>> block/qcow.c | 102 +++++++++++++++++++++++++++++++++++++------------- >>> block/qcow2-cluster.c | 46 ++++++++++++++++++----- >>> block/qcow2.c | 95 +++++++++++++++++++++++----------------------- >>> block/qcow2.h | 13 +++---- >>> 4 files changed, 165 insertions(+), 91 deletions(-) >>> >>> diff --git a/block/qcow.c b/block/qcow.c >>> index bf5c570..01fba54 100644 >>> --- a/block/qcow.c >>> +++ b/block/qcow.c >>> @@ -26,7 +26,7 @@ >>> #include "qemu/module.h" >>> #include <zlib.h> >>> #include "qapi/qmp/qerror.h" >>> -#include "crypto/aes.h" >>> +#include "crypto/cipher.h" >>> #include "migration/migration.h" >>> >>> /**************************************************************/ >>> @@ -72,10 +72,8 @@ typedef struct BDRVQcowState { >>> uint8_t *cluster_cache; >>> uint8_t *cluster_data; >>> uint64_t cluster_cache_offset; >>> - uint32_t crypt_method; /* current crypt method, 0 if no key yet */ >>> + QCryptoCipher *cipher; /* NULL if no key yet */ >>> uint32_t crypt_method_header; >>> - AES_KEY aes_encrypt_key; >>> - AES_KEY aes_decrypt_key; >>> CoMutex lock; >>> Error *migration_blocker; >>> } BDRVQcowState; >>> @@ -154,6 +152,11 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, >>> ret = -EINVAL; >>> goto fail; >>> } >>> + if (!qcrypto_cipher_supports(QCRYPTO_CIPHER_ALG_AES_128)) { >>> + error_setg(errp, "AES cipher not available"); >>> + ret = -EINVAL; >>> + goto fail; >>> + } >>> s->crypt_method_header = header.crypt_method; >>> if (s->crypt_method_header) { >>> bs->encrypted = 1; >>> @@ -260,6 +263,7 @@ static int qcow_set_key(BlockDriverState *bs, const char *key) >>> BDRVQcowState *s = bs->opaque; >>> uint8_t keybuf[16]; >>> int len, i; >>> + Error *err; >>> >>> memset(keybuf, 0, 16); >>> len = strlen(key); >>> @@ -271,38 +275,67 @@ static int qcow_set_key(BlockDriverState *bs, const char *key) >>> keybuf[i] = key[i]; >>> } >>> assert(bs->encrypted); >>> - s->crypt_method = s->crypt_method_header; >>> >>> - if (AES_set_encrypt_key(keybuf, 128, &s->aes_encrypt_key) != 0) >>> - return -1; >>> - if (AES_set_decrypt_key(keybuf, 128, &s->aes_decrypt_key) != 0) >>> + qcrypto_cipher_free(s->cipher); >>> + s->cipher = qcrypto_cipher_new( >>> + QCRYPTO_CIPHER_ALG_AES_128, >>> + QCRYPTO_CIPHER_MODE_CBC, >>> + keybuf, G_N_ELEMENTS(keybuf), >>> + &err); >>> + >>> + if (!s->cipher) { >>> + /* XXX would be nice if errors in this method could >>> + * be properly propagate to the caller. Would need >>> + * the bdrv_set_key() API signature to be fixed. */ >>> + error_free(err); >>> return -1; >>> + } >>> return 0; >>> } >>> >>> /* The crypt function is compatible with the linux cryptoloop >>> algorithm for < 4 GB images. NOTE: out_buf == in_buf is >>> supported */ >>> -static void encrypt_sectors(BDRVQcowState *s, int64_t sector_num, >>> - uint8_t *out_buf, const uint8_t *in_buf, >>> - int nb_sectors, int enc, >>> - const AES_KEY *key) >>> +static int encrypt_sectors(BDRVQcowState *s, int64_t sector_num, >>> + uint8_t *out_buf, const uint8_t *in_buf, >>> + int nb_sectors, bool enc, Error **errp) >>> { >>> union { >>> uint64_t ll[2]; >>> uint8_t b[16]; >>> } ivec; >>> int i; >>> + int ret; >>> >>> for(i = 0; i < nb_sectors; i++) { >>> ivec.ll[0] = cpu_to_le64(sector_num); >>> ivec.ll[1] = 0; >>> - AES_cbc_encrypt(in_buf, out_buf, 512, key, >>> - ivec.b, enc); >>> + if (qcrypto_cipher_setiv(s->cipher, >>> + ivec.b, G_N_ELEMENTS(ivec.b), >>> + errp) < 0) { >>> + return -1; >>> + } >>> + if (enc) { >>> + ret = qcrypto_cipher_encrypt(s->cipher, >>> + in_buf, >>> + out_buf, >>> + 512, >>> + errp); >>> + } else { >>> + ret = qcrypto_cipher_decrypt(s->cipher, >>> + in_buf, >>> + out_buf, >>> + 512, >>> + errp); >>> + } >>> + if (ret < 0) { >>> + return -1; >>> + } >>> sector_num++; >>> in_buf += 512; >>> out_buf += 512; >>> } >>> + return 0; >>> } >>> >>> /* 'allocate' is: >>> @@ -416,15 +449,20 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, >>> if (bs->encrypted && >>> (n_end - n_start) < s->cluster_sectors) { >>> uint64_t start_sect; >>> - assert(s->crypt_method); >>> + assert(s->cipher); >>> start_sect = (offset & ~(s->cluster_size - 1)) >> 9; >>> memset(s->cluster_data + 512, 0x00, 512); >>> for(i = 0; i < s->cluster_sectors; i++) { >>> if (i < n_start || i >= n_end) { >>> - encrypt_sectors(s, start_sect + i, >>> - s->cluster_data, >>> - s->cluster_data + 512, 1, 1, >>> - &s->aes_encrypt_key); >>> + Error *err = NULL; >>> + if (encrypt_sectors(s, start_sect + i, >>> + s->cluster_data, >>> + s->cluster_data + 512, 1, >>> + true, &err) < 0) { >>> + error_free(err); >>> + errno = EIO; >>> + return -1; >>> + } >>> if (bdrv_pwrite(bs->file, cluster_offset + i * 512, >>> s->cluster_data, 512) != 512) >>> return -1; >>> @@ -464,7 +502,7 @@ static int64_t coroutine_fn qcow_co_get_block_status(BlockDriverState *bs, >>> if (!cluster_offset) { >>> return 0; >>> } >>> - if ((cluster_offset & QCOW_OFLAG_COMPRESSED) || s->crypt_method) { >>> + if ((cluster_offset & QCOW_OFLAG_COMPRESSED) || s->cipher) { >>> return BDRV_BLOCK_DATA; >>> } >>> cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS); >>> @@ -531,6 +569,7 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, >>> QEMUIOVector hd_qiov; >>> uint8_t *buf; >>> void *orig_buf; >>> + Error *err = NULL; >>> >>> if (qiov->niov > 1) { >>> buf = orig_buf = qemu_try_blockalign(bs, qiov->size); >>> @@ -594,10 +633,11 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, >>> break; >>> } >>> if (bs->encrypted) { >>> - assert(s->crypt_method); >>> - encrypt_sectors(s, sector_num, buf, buf, >>> - n, 0, >>> - &s->aes_decrypt_key); >>> + assert(s->cipher); >>> + if (encrypt_sectors(s, sector_num, buf, buf, >>> + n, false, &err) < 0) { >>> + goto fail; >>> + } >>> } >>> } >>> ret = 0; >>> @@ -618,6 +658,7 @@ done: >>> return ret; >>> >>> fail: >>> + error_free(err); >>> ret = -EIO; >>> goto done; >>> } >>> @@ -666,12 +707,17 @@ static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num, >>> break; >>> } >>> if (bs->encrypted) { >>> - assert(s->crypt_method); >>> + Error *err = NULL; >>> + assert(s->cipher); >>> if (!cluster_data) { >>> cluster_data = g_malloc0(s->cluster_size); >>> } >>> - encrypt_sectors(s, sector_num, cluster_data, buf, >>> - n, 1, &s->aes_encrypt_key); >>> + if (encrypt_sectors(s, sector_num, cluster_data, buf, >>> + n, true, &err) < 0) { >>> + error_free(err); >>> + ret = -EIO; >>> + break; >>> + } >>> src_buf = cluster_data; >>> } else { >>> src_buf = buf; >>> @@ -708,6 +754,8 @@ static void qcow_close(BlockDriverState *bs) >>> { >>> BDRVQcowState *s = bs->opaque; >>> >>> + qcrypto_cipher_free(s->cipher); >>> + s->cipher = NULL; >>> g_free(s->l1_table); >>> qemu_vfree(s->l2_cache); >>> g_free(s->cluster_cache); >>> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c >>> index 1a5c97a..b43f186 100644 >>> --- a/block/qcow2-cluster.c >>> +++ b/block/qcow2-cluster.c >>> @@ -339,26 +339,47 @@ static int count_contiguous_free_clusters(uint64_t nb_clusters, uint64_t *l2_tab >>> /* The crypt function is compatible with the linux cryptoloop >>> algorithm for < 4 GB images. NOTE: out_buf == in_buf is >>> supported */ >>> -void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, >>> - uint8_t *out_buf, const uint8_t *in_buf, >>> - int nb_sectors, int enc, >>> - const AES_KEY *key) >>> +int qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, >>> + uint8_t *out_buf, const uint8_t *in_buf, >>> + int nb_sectors, bool enc, >>> + Error **errp) >>> { >>> union { >>> uint64_t ll[2]; >>> uint8_t b[16]; >>> } ivec; >>> int i; >>> + int ret; >>> >>> for(i = 0; i < nb_sectors; i++) { >>> ivec.ll[0] = cpu_to_le64(sector_num); >>> ivec.ll[1] = 0; >>> - AES_cbc_encrypt(in_buf, out_buf, 512, key, >>> - ivec.b, enc); >>> + if (qcrypto_cipher_setiv(s->cipher, >>> + ivec.b, G_N_ELEMENTS(ivec.b), >>> + errp) < 0) { >>> + return -1; >>> + } >>> + if (enc) { >>> + ret = qcrypto_cipher_encrypt(s->cipher, >>> + in_buf, >>> + out_buf, >>> + 512, >>> + errp); >>> + } else { >>> + ret = qcrypto_cipher_decrypt(s->cipher, >>> + in_buf, >>> + out_buf, >>> + 512, >>> + errp); >>> + } >>> + if (ret < 0) { >>> + return -1; >>> + } >>> sector_num++; >>> in_buf += 512; >>> out_buf += 512; >>> } >>> + return 0; >>> } >>> >>> static int coroutine_fn copy_sectors(BlockDriverState *bs, >>> @@ -401,10 +422,15 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs, >>> } >>> >>> if (bs->encrypted) { >>> - assert(s->crypt_method); >>> - qcow2_encrypt_sectors(s, start_sect + n_start, >>> - iov.iov_base, iov.iov_base, n, 1, >>> - &s->aes_encrypt_key); >>> + Error *err = NULL; >>> + assert(s->cipher); >>> + if (qcow2_encrypt_sectors(s, start_sect + n_start, >>> + iov.iov_base, iov.iov_base, n, >>> + true, &err) < 0) { >>> + ret = -EIO; >>> + error_free(err); >>> + goto out; >>> + } >>> } >>> >>> ret = qcow2_pre_write_overlap_check(bs, 0, >>> diff --git a/block/qcow2.c b/block/qcow2.c >>> index 85e0731..76c331b 100644 >>> --- a/block/qcow2.c >>> +++ b/block/qcow2.c >>> @@ -698,6 +698,11 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, >>> ret = -EINVAL; >>> goto fail; >>> } >>> + if (!qcrypto_cipher_supports(QCRYPTO_CIPHER_ALG_AES_128)) { >>> + error_setg(errp, "AES cipher not available"); >>> + ret = -EINVAL; >>> + goto fail; >>> + } >>> s->crypt_method_header = header.crypt_method; >>> if (s->crypt_method_header) { >>> bs->encrypted = 1; >>> @@ -1031,6 +1036,7 @@ static int qcow2_set_key(BlockDriverState *bs, const char *key) >>> BDRVQcowState *s = bs->opaque; >>> uint8_t keybuf[16]; >>> int len, i; >>> + Error *err = NULL; >>> >>> memset(keybuf, 0, 16); >>> len = strlen(key); >>> @@ -1042,30 +1048,21 @@ static int qcow2_set_key(BlockDriverState *bs, const char *key) >>> keybuf[i] = key[i]; >>> } >>> assert(bs->encrypted); >>> - s->crypt_method = s->crypt_method_header; >>> >>> - if (AES_set_encrypt_key(keybuf, 128, &s->aes_encrypt_key) != 0) >>> - return -1; >>> - if (AES_set_decrypt_key(keybuf, 128, &s->aes_decrypt_key) != 0) >>> + qcrypto_cipher_free(s->cipher); >>> + s->cipher = qcrypto_cipher_new( >>> + QCRYPTO_CIPHER_ALG_AES_128, >>> + QCRYPTO_CIPHER_MODE_CBC, >>> + keybuf, G_N_ELEMENTS(keybuf), >>> + &err); >>> + >>> + if (!s->cipher) { >>> + /* XXX would be nice if errors in this method could >>> + * be properly propagate to the caller. Would need >>> + * the bdrv_set_key() API signature to be fixed. */ >>> + error_free(err); >>> return -1; >>> -#if 0 >>> - /* test */ >>> - { >>> - uint8_t in[16]; >>> - uint8_t out[16]; >>> - uint8_t tmp[16]; >>> - for(i=0;i<16;i++) >>> - in[i] = i; >>> - AES_encrypt(in, tmp, &s->aes_encrypt_key); >>> - AES_decrypt(tmp, out, &s->aes_decrypt_key); >>> - for(i = 0; i < 16; i++) >>> - printf(" %02x", tmp[i]); >>> - printf("\n"); >>> - for(i = 0; i < 16; i++) >>> - printf(" %02x", out[i]); >>> - printf("\n"); >>> } >>> -#endif >>> return 0; >>> } >>> >>> @@ -1108,7 +1105,7 @@ static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs, >>> } >>> >>> if (cluster_offset != 0 && ret != QCOW2_CLUSTER_COMPRESSED && >>> - !s->crypt_method) { >>> + !s->cipher) { >>> index_in_cluster = sector_num & (s->cluster_sectors - 1); >>> cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS); >>> status |= BDRV_BLOCK_OFFSET_VALID | cluster_offset; >>> @@ -1158,7 +1155,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num, >>> >>> /* prepare next request */ >>> cur_nr_sectors = remaining_sectors; >>> - if (s->crypt_method) { >>> + if (s->cipher) { >>> cur_nr_sectors = MIN(cur_nr_sectors, >>> QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors); >>> } >>> @@ -1230,7 +1227,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num, >>> } >>> >>> if (bs->encrypted) { >>> - assert(s->crypt_method); >>> + assert(s->cipher); >>> >>> /* >>> * For encrypted images, read everything into a temporary >>> @@ -1263,9 +1260,15 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num, >>> goto fail; >>> } >>> if (bs->encrypted) { >>> - assert(s->crypt_method); >>> - qcow2_encrypt_sectors(s, sector_num, cluster_data, >>> - cluster_data, cur_nr_sectors, 0, &s->aes_decrypt_key); >>> + assert(s->cipher); >>> + Error *err = NULL; >>> + if (qcow2_encrypt_sectors(s, sector_num, cluster_data, >>> + cluster_data, cur_nr_sectors, false, >>> + &err) < 0) { >>> + error_free(err); >>> + ret = -EIO; >>> + goto fail; >>> + } >>> qemu_iovec_from_buf(qiov, bytes_done, >>> cluster_data, 512 * cur_nr_sectors); >>> } >>> @@ -1343,7 +1346,8 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs, >>> cur_nr_sectors * 512); >>> >>> if (bs->encrypted) { >>> - assert(s->crypt_method); >>> + Error *err = NULL; >>> + assert(s->cipher); >>> if (!cluster_data) { >>> cluster_data = qemu_try_blockalign(bs->file, >>> QCOW_MAX_CRYPT_CLUSTERS >>> @@ -1358,8 +1362,13 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs, >>> QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size); >>> qemu_iovec_to_buf(&hd_qiov, 0, cluster_data, hd_qiov.size); >>> >>> - qcow2_encrypt_sectors(s, sector_num, cluster_data, >>> - cluster_data, cur_nr_sectors, 1, &s->aes_encrypt_key); >>> + if (qcow2_encrypt_sectors(s, sector_num, cluster_data, >>> + cluster_data, cur_nr_sectors, >>> + true, &err) < 0) { >>> + error_free(err); >>> + ret = -EIO; >>> + goto fail; >>> + } >>> >>> qemu_iovec_reset(&hd_qiov); >>> qemu_iovec_add(&hd_qiov, cluster_data, >>> @@ -1465,6 +1474,9 @@ static void qcow2_close(BlockDriverState *bs) >>> qcow2_cache_destroy(bs, s->l2_table_cache); >>> qcow2_cache_destroy(bs, s->refcount_block_cache); >>> >>> + qcrypto_cipher_free(s->cipher); >>> + s->cipher = NULL; >>> + >>> g_free(s->unknown_header_fields); >>> cleanup_unknown_header_ext(bs); >>> >>> @@ -1481,9 +1493,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) >>> { >>> BDRVQcowState *s = bs->opaque; >>> int flags = s->flags; >>> - AES_KEY aes_encrypt_key; >>> - AES_KEY aes_decrypt_key; >>> - uint32_t crypt_method = 0; >>> + QCryptoCipher *cipher = NULL; >>> QDict *options; >>> Error *local_err = NULL; >>> int ret; >>> @@ -1493,12 +1503,8 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) >>> * that means we don't have to worry about reopening them here. >>> */ >>> >>> - if (bs->encrypted) { >>> - assert(s->crypt_method); >>> - crypt_method = s->crypt_method; >>> - memcpy(&aes_encrypt_key, &s->aes_encrypt_key, sizeof(aes_encrypt_key)); >>> - memcpy(&aes_decrypt_key, &s->aes_decrypt_key, sizeof(aes_decrypt_key)); >>> - } >>> + cipher = s->cipher; >>> + s->cipher = NULL; >>> >>> qcow2_close(bs); >>> >>> @@ -1523,11 +1529,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) >>> return; >>> } >>> >>> - if (bs->encrypted) { >>> - s->crypt_method = crypt_method; >>> - memcpy(&s->aes_encrypt_key, &aes_encrypt_key, sizeof(aes_encrypt_key)); >>> - memcpy(&s->aes_decrypt_key, &aes_decrypt_key, sizeof(aes_decrypt_key)); >>> - } >>> + s->cipher = cipher; >>> } >>> >>> static size_t header_ext_add(char *buf, uint32_t magic, const void *s, >>> @@ -2728,8 +2730,9 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts, >>> backing_format = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT); >>> } else if (!strcmp(desc->name, BLOCK_OPT_ENCRYPT)) { >>> encrypt = qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT, >>> - s->crypt_method); >>> - if (encrypt != !!s->crypt_method) { >>> + !!s->cipher); >>> + >>> + if (encrypt != !!s->cipher) { >>> fprintf(stderr, "Changing the encryption flag is not " >>> "supported.\n"); >>> return -ENOTSUP; >>> diff --git a/block/qcow2.h b/block/qcow2.h >>> index 462147c..72e1328 100644 >>> --- a/block/qcow2.h >>> +++ b/block/qcow2.h >>> @@ -25,7 +25,7 @@ >>> #ifndef BLOCK_QCOW2_H >>> #define BLOCK_QCOW2_H >>> >>> -#include "crypto/aes.h" >>> +#include "crypto/cipher.h" >>> #include "block/coroutine.h" >>> >>> //#define DEBUG_ALLOC >>> @@ -253,10 +253,8 @@ typedef struct BDRVQcowState { >>> >>> CoMutex lock; >>> >>> - uint32_t crypt_method; /* current crypt method, 0 if no key yet */ >>> + QCryptoCipher *cipher; /* current cipher, NULL if no key yet */ >>> uint32_t crypt_method_header; >>> - AES_KEY aes_encrypt_key; >>> - AES_KEY aes_decrypt_key; >>> uint64_t snapshots_offset; >>> int snapshots_size; >>> unsigned int nb_snapshots; >>> @@ -536,10 +534,9 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, >>> int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index); >>> void qcow2_l2_cache_reset(BlockDriverState *bs); >>> int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset); >>> -void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, >>> - uint8_t *out_buf, const uint8_t *in_buf, >>> - int nb_sectors, int enc, >>> - const AES_KEY *key); >>> +int qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, >>> + uint8_t *out_buf, const uint8_t *in_buf, >>> + int nb_sectors, bool enc, Error **errp); >>> >>> int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset, >>> int *num, uint64_t *cluster_offset); >>> >> >> > >
On 2015-07-09 12:53, Christian Borntraeger wrote: > Forgot some CCs (patch author and migration folks) > > > Am 09.07.2015 um 12:17 schrieb Christian Borntraeger: > > Am 07.07.2015 um 16:12 schrieb Paolo Bonzini: > >> From: "Daniel P. Berrange" <berrange@redhat.com> > >> > >> Switch the qcow/qcow2 block driver over to use the generic cipher > >> API, this allows it to use the pluggable AES implementations, > >> instead of being hardcoded to use QEMU's built-in impl. > >> > >> Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > >> Message-Id: <1435770638-25715-10-git-send-email-berrange@redhat.com> > >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > > > For whatever reason this breaks migration(or virsh restore) > > from guests that were created with an older version of QEMU. > > > > > > > > Thread 1 (Thread 0x3fffb856bd0 (LWP 32226)): > > #0 0x0000000080354a54 in qcrypto_cipher_free (cipher=0x0) at /home/cborntra/REPOS/qemu/crypto/cipher-builtin.c:357 > > #1 0x00000000802ca912 in qcow2_close (bs=0x80a0cea0) at /home/cborntra/REPOS/qemu/block/qcow2.c:1477 > > #2 0x00000000802caa32 in qcow2_invalidate_cache (bs=0x80a0cea0, errp=0x81a3fc58) at /home/cborntra/REPOS/qemu/block/qcow2.c:1509 > > #3 0x000000008029bac0 in bdrv_invalidate_cache (bs=0x80a0cea0, errp=0x81a3fd08) at /home/cborntra/REPOS/qemu/block.c:3135 > > #4 0x000000008029bbe6 in bdrv_invalidate_cache_all (errp=0x81a3fdd0) at /home/cborntra/REPOS/qemu/block.c:3160 > > #5 0x000000008021de50 in process_incoming_migration_co (opaque=0x80acaae0) at /home/cborntra/REPOS/qemu/migration/migration.c:160 > > #6 0x00000000802ab96a in coroutine_trampoline (i0=0, i1=-2137149984) at /home/cborntra/REPOS/qemu/coroutine-ucontext.c:80 > > #7 0x000003fffc463ca2 in __makecontext_ret () from /lib64/libc.so.6 > > Backtrace stopped: previous frame identical to this frame (corrupt stack?) This is the same kind of backtrace I got on a MIPS host (see my other mail). The reason is that a NULL pointer is dereferenced before testing it is non NULL in qcrypto_cipher_free.
diff --git a/block/qcow.c b/block/qcow.c index bf5c570..01fba54 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -26,7 +26,7 @@ #include "qemu/module.h" #include <zlib.h> #include "qapi/qmp/qerror.h" -#include "crypto/aes.h" +#include "crypto/cipher.h" #include "migration/migration.h" /**************************************************************/ @@ -72,10 +72,8 @@ typedef struct BDRVQcowState { uint8_t *cluster_cache; uint8_t *cluster_data; uint64_t cluster_cache_offset; - uint32_t crypt_method; /* current crypt method, 0 if no key yet */ + QCryptoCipher *cipher; /* NULL if no key yet */ uint32_t crypt_method_header; - AES_KEY aes_encrypt_key; - AES_KEY aes_decrypt_key; CoMutex lock; Error *migration_blocker; } BDRVQcowState; @@ -154,6 +152,11 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags, ret = -EINVAL; goto fail; } + if (!qcrypto_cipher_supports(QCRYPTO_CIPHER_ALG_AES_128)) { + error_setg(errp, "AES cipher not available"); + ret = -EINVAL; + goto fail; + } s->crypt_method_header = header.crypt_method; if (s->crypt_method_header) { bs->encrypted = 1; @@ -260,6 +263,7 @@ static int qcow_set_key(BlockDriverState *bs, const char *key) BDRVQcowState *s = bs->opaque; uint8_t keybuf[16]; int len, i; + Error *err; memset(keybuf, 0, 16); len = strlen(key); @@ -271,38 +275,67 @@ static int qcow_set_key(BlockDriverState *bs, const char *key) keybuf[i] = key[i]; } assert(bs->encrypted); - s->crypt_method = s->crypt_method_header; - if (AES_set_encrypt_key(keybuf, 128, &s->aes_encrypt_key) != 0) - return -1; - if (AES_set_decrypt_key(keybuf, 128, &s->aes_decrypt_key) != 0) + qcrypto_cipher_free(s->cipher); + s->cipher = qcrypto_cipher_new( + QCRYPTO_CIPHER_ALG_AES_128, + QCRYPTO_CIPHER_MODE_CBC, + keybuf, G_N_ELEMENTS(keybuf), + &err); + + if (!s->cipher) { + /* XXX would be nice if errors in this method could + * be properly propagate to the caller. Would need + * the bdrv_set_key() API signature to be fixed. */ + error_free(err); return -1; + } return 0; } /* The crypt function is compatible with the linux cryptoloop algorithm for < 4 GB images. NOTE: out_buf == in_buf is supported */ -static void encrypt_sectors(BDRVQcowState *s, int64_t sector_num, - uint8_t *out_buf, const uint8_t *in_buf, - int nb_sectors, int enc, - const AES_KEY *key) +static int encrypt_sectors(BDRVQcowState *s, int64_t sector_num, + uint8_t *out_buf, const uint8_t *in_buf, + int nb_sectors, bool enc, Error **errp) { union { uint64_t ll[2]; uint8_t b[16]; } ivec; int i; + int ret; for(i = 0; i < nb_sectors; i++) { ivec.ll[0] = cpu_to_le64(sector_num); ivec.ll[1] = 0; - AES_cbc_encrypt(in_buf, out_buf, 512, key, - ivec.b, enc); + if (qcrypto_cipher_setiv(s->cipher, + ivec.b, G_N_ELEMENTS(ivec.b), + errp) < 0) { + return -1; + } + if (enc) { + ret = qcrypto_cipher_encrypt(s->cipher, + in_buf, + out_buf, + 512, + errp); + } else { + ret = qcrypto_cipher_decrypt(s->cipher, + in_buf, + out_buf, + 512, + errp); + } + if (ret < 0) { + return -1; + } sector_num++; in_buf += 512; out_buf += 512; } + return 0; } /* 'allocate' is: @@ -416,15 +449,20 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, if (bs->encrypted && (n_end - n_start) < s->cluster_sectors) { uint64_t start_sect; - assert(s->crypt_method); + assert(s->cipher); start_sect = (offset & ~(s->cluster_size - 1)) >> 9; memset(s->cluster_data + 512, 0x00, 512); for(i = 0; i < s->cluster_sectors; i++) { if (i < n_start || i >= n_end) { - encrypt_sectors(s, start_sect + i, - s->cluster_data, - s->cluster_data + 512, 1, 1, - &s->aes_encrypt_key); + Error *err = NULL; + if (encrypt_sectors(s, start_sect + i, + s->cluster_data, + s->cluster_data + 512, 1, + true, &err) < 0) { + error_free(err); + errno = EIO; + return -1; + } if (bdrv_pwrite(bs->file, cluster_offset + i * 512, s->cluster_data, 512) != 512) return -1; @@ -464,7 +502,7 @@ static int64_t coroutine_fn qcow_co_get_block_status(BlockDriverState *bs, if (!cluster_offset) { return 0; } - if ((cluster_offset & QCOW_OFLAG_COMPRESSED) || s->crypt_method) { + if ((cluster_offset & QCOW_OFLAG_COMPRESSED) || s->cipher) { return BDRV_BLOCK_DATA; } cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS); @@ -531,6 +569,7 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, QEMUIOVector hd_qiov; uint8_t *buf; void *orig_buf; + Error *err = NULL; if (qiov->niov > 1) { buf = orig_buf = qemu_try_blockalign(bs, qiov->size); @@ -594,10 +633,11 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, break; } if (bs->encrypted) { - assert(s->crypt_method); - encrypt_sectors(s, sector_num, buf, buf, - n, 0, - &s->aes_decrypt_key); + assert(s->cipher); + if (encrypt_sectors(s, sector_num, buf, buf, + n, false, &err) < 0) { + goto fail; + } } } ret = 0; @@ -618,6 +658,7 @@ done: return ret; fail: + error_free(err); ret = -EIO; goto done; } @@ -666,12 +707,17 @@ static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num, break; } if (bs->encrypted) { - assert(s->crypt_method); + Error *err = NULL; + assert(s->cipher); if (!cluster_data) { cluster_data = g_malloc0(s->cluster_size); } - encrypt_sectors(s, sector_num, cluster_data, buf, - n, 1, &s->aes_encrypt_key); + if (encrypt_sectors(s, sector_num, cluster_data, buf, + n, true, &err) < 0) { + error_free(err); + ret = -EIO; + break; + } src_buf = cluster_data; } else { src_buf = buf; @@ -708,6 +754,8 @@ static void qcow_close(BlockDriverState *bs) { BDRVQcowState *s = bs->opaque; + qcrypto_cipher_free(s->cipher); + s->cipher = NULL; g_free(s->l1_table); qemu_vfree(s->l2_cache); g_free(s->cluster_cache); diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 1a5c97a..b43f186 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -339,26 +339,47 @@ static int count_contiguous_free_clusters(uint64_t nb_clusters, uint64_t *l2_tab /* The crypt function is compatible with the linux cryptoloop algorithm for < 4 GB images. NOTE: out_buf == in_buf is supported */ -void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, - uint8_t *out_buf, const uint8_t *in_buf, - int nb_sectors, int enc, - const AES_KEY *key) +int qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, + uint8_t *out_buf, const uint8_t *in_buf, + int nb_sectors, bool enc, + Error **errp) { union { uint64_t ll[2]; uint8_t b[16]; } ivec; int i; + int ret; for(i = 0; i < nb_sectors; i++) { ivec.ll[0] = cpu_to_le64(sector_num); ivec.ll[1] = 0; - AES_cbc_encrypt(in_buf, out_buf, 512, key, - ivec.b, enc); + if (qcrypto_cipher_setiv(s->cipher, + ivec.b, G_N_ELEMENTS(ivec.b), + errp) < 0) { + return -1; + } + if (enc) { + ret = qcrypto_cipher_encrypt(s->cipher, + in_buf, + out_buf, + 512, + errp); + } else { + ret = qcrypto_cipher_decrypt(s->cipher, + in_buf, + out_buf, + 512, + errp); + } + if (ret < 0) { + return -1; + } sector_num++; in_buf += 512; out_buf += 512; } + return 0; } static int coroutine_fn copy_sectors(BlockDriverState *bs, @@ -401,10 +422,15 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs, } if (bs->encrypted) { - assert(s->crypt_method); - qcow2_encrypt_sectors(s, start_sect + n_start, - iov.iov_base, iov.iov_base, n, 1, - &s->aes_encrypt_key); + Error *err = NULL; + assert(s->cipher); + if (qcow2_encrypt_sectors(s, start_sect + n_start, + iov.iov_base, iov.iov_base, n, + true, &err) < 0) { + ret = -EIO; + error_free(err); + goto out; + } } ret = qcow2_pre_write_overlap_check(bs, 0, diff --git a/block/qcow2.c b/block/qcow2.c index 85e0731..76c331b 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -698,6 +698,11 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, ret = -EINVAL; goto fail; } + if (!qcrypto_cipher_supports(QCRYPTO_CIPHER_ALG_AES_128)) { + error_setg(errp, "AES cipher not available"); + ret = -EINVAL; + goto fail; + } s->crypt_method_header = header.crypt_method; if (s->crypt_method_header) { bs->encrypted = 1; @@ -1031,6 +1036,7 @@ static int qcow2_set_key(BlockDriverState *bs, const char *key) BDRVQcowState *s = bs->opaque; uint8_t keybuf[16]; int len, i; + Error *err = NULL; memset(keybuf, 0, 16); len = strlen(key); @@ -1042,30 +1048,21 @@ static int qcow2_set_key(BlockDriverState *bs, const char *key) keybuf[i] = key[i]; } assert(bs->encrypted); - s->crypt_method = s->crypt_method_header; - if (AES_set_encrypt_key(keybuf, 128, &s->aes_encrypt_key) != 0) - return -1; - if (AES_set_decrypt_key(keybuf, 128, &s->aes_decrypt_key) != 0) + qcrypto_cipher_free(s->cipher); + s->cipher = qcrypto_cipher_new( + QCRYPTO_CIPHER_ALG_AES_128, + QCRYPTO_CIPHER_MODE_CBC, + keybuf, G_N_ELEMENTS(keybuf), + &err); + + if (!s->cipher) { + /* XXX would be nice if errors in this method could + * be properly propagate to the caller. Would need + * the bdrv_set_key() API signature to be fixed. */ + error_free(err); return -1; -#if 0 - /* test */ - { - uint8_t in[16]; - uint8_t out[16]; - uint8_t tmp[16]; - for(i=0;i<16;i++) - in[i] = i; - AES_encrypt(in, tmp, &s->aes_encrypt_key); - AES_decrypt(tmp, out, &s->aes_decrypt_key); - for(i = 0; i < 16; i++) - printf(" %02x", tmp[i]); - printf("\n"); - for(i = 0; i < 16; i++) - printf(" %02x", out[i]); - printf("\n"); } -#endif return 0; } @@ -1108,7 +1105,7 @@ static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs, } if (cluster_offset != 0 && ret != QCOW2_CLUSTER_COMPRESSED && - !s->crypt_method) { + !s->cipher) { index_in_cluster = sector_num & (s->cluster_sectors - 1); cluster_offset |= (index_in_cluster << BDRV_SECTOR_BITS); status |= BDRV_BLOCK_OFFSET_VALID | cluster_offset; @@ -1158,7 +1155,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num, /* prepare next request */ cur_nr_sectors = remaining_sectors; - if (s->crypt_method) { + if (s->cipher) { cur_nr_sectors = MIN(cur_nr_sectors, QCOW_MAX_CRYPT_CLUSTERS * s->cluster_sectors); } @@ -1230,7 +1227,7 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num, } if (bs->encrypted) { - assert(s->crypt_method); + assert(s->cipher); /* * For encrypted images, read everything into a temporary @@ -1263,9 +1260,15 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num, goto fail; } if (bs->encrypted) { - assert(s->crypt_method); - qcow2_encrypt_sectors(s, sector_num, cluster_data, - cluster_data, cur_nr_sectors, 0, &s->aes_decrypt_key); + assert(s->cipher); + Error *err = NULL; + if (qcow2_encrypt_sectors(s, sector_num, cluster_data, + cluster_data, cur_nr_sectors, false, + &err) < 0) { + error_free(err); + ret = -EIO; + goto fail; + } qemu_iovec_from_buf(qiov, bytes_done, cluster_data, 512 * cur_nr_sectors); } @@ -1343,7 +1346,8 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs, cur_nr_sectors * 512); if (bs->encrypted) { - assert(s->crypt_method); + Error *err = NULL; + assert(s->cipher); if (!cluster_data) { cluster_data = qemu_try_blockalign(bs->file, QCOW_MAX_CRYPT_CLUSTERS @@ -1358,8 +1362,13 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs, QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size); qemu_iovec_to_buf(&hd_qiov, 0, cluster_data, hd_qiov.size); - qcow2_encrypt_sectors(s, sector_num, cluster_data, - cluster_data, cur_nr_sectors, 1, &s->aes_encrypt_key); + if (qcow2_encrypt_sectors(s, sector_num, cluster_data, + cluster_data, cur_nr_sectors, + true, &err) < 0) { + error_free(err); + ret = -EIO; + goto fail; + } qemu_iovec_reset(&hd_qiov); qemu_iovec_add(&hd_qiov, cluster_data, @@ -1465,6 +1474,9 @@ static void qcow2_close(BlockDriverState *bs) qcow2_cache_destroy(bs, s->l2_table_cache); qcow2_cache_destroy(bs, s->refcount_block_cache); + qcrypto_cipher_free(s->cipher); + s->cipher = NULL; + g_free(s->unknown_header_fields); cleanup_unknown_header_ext(bs); @@ -1481,9 +1493,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) { BDRVQcowState *s = bs->opaque; int flags = s->flags; - AES_KEY aes_encrypt_key; - AES_KEY aes_decrypt_key; - uint32_t crypt_method = 0; + QCryptoCipher *cipher = NULL; QDict *options; Error *local_err = NULL; int ret; @@ -1493,12 +1503,8 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) * that means we don't have to worry about reopening them here. */ - if (bs->encrypted) { - assert(s->crypt_method); - crypt_method = s->crypt_method; - memcpy(&aes_encrypt_key, &s->aes_encrypt_key, sizeof(aes_encrypt_key)); - memcpy(&aes_decrypt_key, &s->aes_decrypt_key, sizeof(aes_decrypt_key)); - } + cipher = s->cipher; + s->cipher = NULL; qcow2_close(bs); @@ -1523,11 +1529,7 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp) return; } - if (bs->encrypted) { - s->crypt_method = crypt_method; - memcpy(&s->aes_encrypt_key, &aes_encrypt_key, sizeof(aes_encrypt_key)); - memcpy(&s->aes_decrypt_key, &aes_decrypt_key, sizeof(aes_decrypt_key)); - } + s->cipher = cipher; } static size_t header_ext_add(char *buf, uint32_t magic, const void *s, @@ -2728,8 +2730,9 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts, backing_format = qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT); } else if (!strcmp(desc->name, BLOCK_OPT_ENCRYPT)) { encrypt = qemu_opt_get_bool(opts, BLOCK_OPT_ENCRYPT, - s->crypt_method); - if (encrypt != !!s->crypt_method) { + !!s->cipher); + + if (encrypt != !!s->cipher) { fprintf(stderr, "Changing the encryption flag is not " "supported.\n"); return -ENOTSUP; diff --git a/block/qcow2.h b/block/qcow2.h index 462147c..72e1328 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -25,7 +25,7 @@ #ifndef BLOCK_QCOW2_H #define BLOCK_QCOW2_H -#include "crypto/aes.h" +#include "crypto/cipher.h" #include "block/coroutine.h" //#define DEBUG_ALLOC @@ -253,10 +253,8 @@ typedef struct BDRVQcowState { CoMutex lock; - uint32_t crypt_method; /* current crypt method, 0 if no key yet */ + QCryptoCipher *cipher; /* current cipher, NULL if no key yet */ uint32_t crypt_method_header; - AES_KEY aes_encrypt_key; - AES_KEY aes_decrypt_key; uint64_t snapshots_offset; int snapshots_size; unsigned int nb_snapshots; @@ -536,10 +534,9 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index); void qcow2_l2_cache_reset(BlockDriverState *bs); int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset); -void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, - uint8_t *out_buf, const uint8_t *in_buf, - int nb_sectors, int enc, - const AES_KEY *key); +int qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num, + uint8_t *out_buf, const uint8_t *in_buf, + int nb_sectors, bool enc, Error **errp); int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset, int *num, uint64_t *cluster_offset);