Commit 603fbd07 authored by Maxim Levitsky's avatar Maxim Levitsky Committed by Max Reitz
Browse files

block/qcow2: refactor encryption code



* Change the qcow2_co_{encrypt|decrypt} to just receive full host and
  guest offsets and use this function directly instead of calling
  do_perform_cow_encrypt (which is removed by that patch).

* Adjust qcow2_co_encdec to take full host and guest offsets as well.

* Document the qcow2_co_{encrypt|decrypt} arguments
  to prevent the bug fixed in former commit from hopefully
  happening again.

Signed-off-by: default avatarMaxim Levitsky <mlevitsk@redhat.com>
Message-id: 20190915203655.21638-3-mlevitsk@redhat.com
Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
[mreitz: Let perform_cow() return the error value returned by
         qcow2_co_encrypt(), as proposed by Vladimir]
Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
parent 38e7d54b
Loading
Loading
Loading
Loading
+13 −28
Original line number Diff line number Diff line
@@ -462,28 +462,6 @@ static int coroutine_fn do_perform_cow_read(BlockDriverState *bs,
    return 0;
}

static bool coroutine_fn do_perform_cow_encrypt(BlockDriverState *bs,
                                                uint64_t src_cluster_offset,
                                                uint64_t cluster_offset,
                                                unsigned offset_in_cluster,
                                                uint8_t *buffer,
                                                unsigned bytes)
{
    if (bytes && bs->encrypted) {
        BDRVQcow2State *s = bs->opaque;
        assert(QEMU_IS_ALIGNED(offset_in_cluster, BDRV_SECTOR_SIZE));
        assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE));
        assert(s->crypto);
        if (qcow2_co_encrypt(bs,
                start_of_cluster(s, cluster_offset + offset_in_cluster),
                src_cluster_offset + offset_in_cluster,
                buffer, bytes) < 0) {
            return false;
        }
    }
    return true;
}

static int coroutine_fn do_perform_cow_write(BlockDriverState *bs,
                                             uint64_t cluster_offset,
                                             unsigned offset_in_cluster,
@@ -891,12 +869,19 @@ static int perform_cow(BlockDriverState *bs, QCowL2Meta *m)

    /* Encrypt the data if necessary before writing it */
    if (bs->encrypted) {
        if (!do_perform_cow_encrypt(bs, m->offset, m->alloc_offset,
                                    start->offset, start_buffer,
                                    start->nb_bytes) ||
            !do_perform_cow_encrypt(bs, m->offset, m->alloc_offset,
                                    end->offset, end_buffer, end->nb_bytes)) {
            ret = -EIO;
        ret = qcow2_co_encrypt(bs,
                               m->alloc_offset + start->offset,
                               m->offset + start->offset,
                               start_buffer, start->nb_bytes);
        if (ret < 0) {
            goto fail;
        }

        ret = qcow2_co_encrypt(bs,
                               m->alloc_offset + end->offset,
                               m->offset + end->offset,
                               end_buffer, end->nb_bytes);
        if (ret < 0) {
            goto fail;
        }
    }
+49 −14
Original line number Diff line number Diff line
@@ -234,35 +234,70 @@ static int qcow2_encdec_pool_func(void *opaque)
}

static int coroutine_fn
qcow2_co_encdec(BlockDriverState *bs, uint64_t file_cluster_offset,
                  uint64_t offset, void *buf, size_t len, Qcow2EncDecFunc func)
qcow2_co_encdec(BlockDriverState *bs, uint64_t host_offset,
                uint64_t guest_offset, void *buf, size_t len,
                Qcow2EncDecFunc func)
{
    BDRVQcow2State *s = bs->opaque;
    Qcow2EncDecData arg = {
        .block = s->crypto,
        .offset = s->crypt_physical_offset ?
                      file_cluster_offset + offset_into_cluster(s, offset) :
                      offset,
        .offset = s->crypt_physical_offset ? host_offset : guest_offset,
        .buf = buf,
        .len = len,
        .func = func,
    };

    return qcow2_co_process(bs, qcow2_encdec_pool_func, &arg);
    assert(QEMU_IS_ALIGNED(guest_offset, BDRV_SECTOR_SIZE));
    assert(QEMU_IS_ALIGNED(host_offset, BDRV_SECTOR_SIZE));
    assert(QEMU_IS_ALIGNED(len, BDRV_SECTOR_SIZE));
    assert(s->crypto);

    return len == 0 ? 0 : qcow2_co_process(bs, qcow2_encdec_pool_func, &arg);
}

/*
 * qcow2_co_encrypt()
 *
 * Encrypts one or more contiguous aligned sectors
 *
 * @host_offset - underlying storage offset of the first sector of the
 * data to be encrypted
 *
 * @guest_offset - guest (virtual) offset of the first sector of the
 * data to be encrypted
 *
 * @buf - buffer with the data to encrypt, that after encryption
 *        will be written to the underlying storage device at
 *        @host_offset
 *
 * @len - length of the buffer (must be a BDRV_SECTOR_SIZE multiple)
 *
 * Depending on the encryption method, @host_offset and/or @guest_offset
 * may be used for generating the initialization vector for
 * encryption.
 *
 * Note that while the whole range must be aligned on sectors, it
 * does not have to be aligned on clusters and can also cross cluster
 * boundaries
 */
int coroutine_fn
qcow2_co_encrypt(BlockDriverState *bs, uint64_t file_cluster_offset,
                 uint64_t offset, void *buf, size_t len)
qcow2_co_encrypt(BlockDriverState *bs, uint64_t host_offset,
                 uint64_t guest_offset, void *buf, size_t len)
{
    return qcow2_co_encdec(bs, file_cluster_offset, offset, buf, len,
    return qcow2_co_encdec(bs, host_offset, guest_offset, buf, len,
                           qcrypto_block_encrypt);
}

/*
 * qcow2_co_decrypt()
 *
 * Decrypts one or more contiguous aligned sectors
 * Similar to qcow2_co_encrypt
 */
int coroutine_fn
qcow2_co_decrypt(BlockDriverState *bs, uint64_t file_cluster_offset,
                 uint64_t offset, void *buf, size_t len)
qcow2_co_decrypt(BlockDriverState *bs, uint64_t host_offset,
                 uint64_t guest_offset, void *buf, size_t len)
{
    return qcow2_co_encdec(bs, file_cluster_offset, offset, buf, len,
    return qcow2_co_encdec(bs, host_offset, guest_offset, buf, len,
                           qcrypto_block_decrypt);
}
+3 −2
Original line number Diff line number Diff line
@@ -2069,7 +2069,8 @@ static coroutine_fn int qcow2_co_preadv_part(BlockDriverState *bs,

                assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE));
                assert(QEMU_IS_ALIGNED(cur_bytes, BDRV_SECTOR_SIZE));
                if (qcow2_co_decrypt(bs, cluster_offset, offset,
                if (qcow2_co_decrypt(bs, cluster_offset + offset_in_cluster,
                                     offset,
                                     cluster_data, cur_bytes) < 0) {
                    ret = -EIO;
                    goto fail;
@@ -2288,7 +2289,7 @@ static coroutine_fn int qcow2_co_pwritev_part(
            qemu_iovec_to_buf(qiov, qiov_offset + bytes_done,
                              cluster_data, cur_bytes);

            if (qcow2_co_encrypt(bs, cluster_offset, offset,
            if (qcow2_co_encrypt(bs, cluster_offset + offset_in_cluster, offset,
                                 cluster_data, cur_bytes) < 0) {
                ret = -EIO;
                goto out_unlocked;
+4 −4
Original line number Diff line number Diff line
@@ -758,10 +758,10 @@ ssize_t coroutine_fn
qcow2_co_decompress(BlockDriverState *bs, void *dest, size_t dest_size,
                    const void *src, size_t src_size);
int coroutine_fn
qcow2_co_encrypt(BlockDriverState *bs, uint64_t file_cluster_offset,
                 uint64_t offset, void *buf, size_t len);
qcow2_co_encrypt(BlockDriverState *bs, uint64_t host_offset,
                 uint64_t guest_offset, void *buf, size_t len);
int coroutine_fn
qcow2_co_decrypt(BlockDriverState *bs, uint64_t file_cluster_offset,
                 uint64_t offset, void *buf, size_t len);
qcow2_co_decrypt(BlockDriverState *bs, uint64_t host_offset,
                 uint64_t guest_offset, void *buf, size_t len);

#endif