Am 07.07.2015 um 16:12 schrieb Paolo Bonzini: > From: "Daniel P. Berrange" <berra...@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 <berra...@redhat.com> > Message-Id: <1435770638-25715-10-git-send-email-berra...@redhat.com> > Signed-off-by: Paolo Bonzini <pbonz...@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); >