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" <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); >> > >