| Message ID | 20200213171646.15876-1-berto@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qcow2: Fix alignment checks in encrypted images | expand |
On Thu, Feb 13, 2020 at 06:16:46PM +0100, Alberto Garcia wrote: > I/O requests to encrypted media should be aligned to the sector size > used by the underlying encryption method, not to BDRV_SECTOR_SIZE. > Fortunately this doesn't break anything at the moment because > both existing QCRYPTO_BLOCK_*_SECTOR_SIZE have the same value as > BDRV_SECTOR_SIZE. > > The checks in qcow2_co_preadv_encrypted() are also unnecessary because > they are repeated immediately afterwards in qcow2_co_encdec(). > > Signed-off-by: Alberto Garcia <berto@igalia.com> > --- > block/qcow2-threads.c | 12 ++++++++---- > block/qcow2.c | 2 -- > 2 files changed, 8 insertions(+), 6 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel
Am 13.02.2020 um 18:16 hat Alberto Garcia geschrieben: > I/O requests to encrypted media should be aligned to the sector size > used by the underlying encryption method, not to BDRV_SECTOR_SIZE. > Fortunately this doesn't break anything at the moment because > both existing QCRYPTO_BLOCK_*_SECTOR_SIZE have the same value as > BDRV_SECTOR_SIZE. > > The checks in qcow2_co_preadv_encrypted() are also unnecessary because > they are repeated immediately afterwards in qcow2_co_encdec(). > > Signed-off-by: Alberto Garcia <berto@igalia.com> Thanks, applied to the block branch. Kevin
diff --git a/block/qcow2-threads.c b/block/qcow2-threads.c index 8f5a0d1ebe..77bb578cdf 100644 --- a/block/qcow2-threads.c +++ b/block/qcow2-threads.c @@ -246,12 +246,15 @@ qcow2_co_encdec(BlockDriverState *bs, uint64_t host_offset, .len = len, .func = func, }; + uint64_t sector_size; - 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); + sector_size = qcrypto_block_get_sector_size(s->crypto); + assert(QEMU_IS_ALIGNED(guest_offset, sector_size)); + assert(QEMU_IS_ALIGNED(host_offset, sector_size)); + assert(QEMU_IS_ALIGNED(len, sector_size)); + return len == 0 ? 0 : qcow2_co_process(bs, qcow2_encdec_pool_func, &arg); } @@ -270,7 +273,8 @@ qcow2_co_encdec(BlockDriverState *bs, uint64_t host_offset, * will be written to the underlying storage device at * @host_offset * - * @len - length of the buffer (must be a BDRV_SECTOR_SIZE multiple) + * @len - length of the buffer (must be a multiple of the encryption + * sector size) * * Depending on the encryption method, @host_offset and/or @guest_offset * may be used for generating the initialization vector for diff --git a/block/qcow2.c b/block/qcow2.c index ef96606f8d..8dcee5efec 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2068,8 +2068,6 @@ qcow2_co_preadv_encrypted(BlockDriverState *bs, goto fail; } - assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)); - assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE)); if (qcow2_co_decrypt(bs, file_cluster_offset + offset_into_cluster(s, offset), offset, buf, bytes) < 0)
I/O requests to encrypted media should be aligned to the sector size used by the underlying encryption method, not to BDRV_SECTOR_SIZE. Fortunately this doesn't break anything at the moment because both existing QCRYPTO_BLOCK_*_SECTOR_SIZE have the same value as BDRV_SECTOR_SIZE. The checks in qcow2_co_preadv_encrypted() are also unnecessary because they are repeated immediately afterwards in qcow2_co_encdec(). Signed-off-by: Alberto Garcia <berto@igalia.com> --- block/qcow2-threads.c | 12 ++++++++---- block/qcow2.c | 2 -- 2 files changed, 8 insertions(+), 6 deletions(-)