Re: [Qemu-block] [PATCH v3 3/7] block: fix data type casting for crypto payload offset

[Eric Blake]

On 09/12/2017 06:28 AM, Daniel P. Berrange wrote:
> The crypto APIs report the offset of the data payload as an uint64_t
> type, but the block driver is casting to size_t or ssize_t which will
> potentially truncate.
> 
> Most of the block APIs use int64_t for offsets meanwhile, so even if
> using uint64_t in the crypto block driver we are still at risk of
> truncation.
> 
> Change the block crypto driver to use uint64_t, but add asserts that
> the value is less than INT64_MAX.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  block/crypto.c | 13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH v3 3/7] block: fix data type casting for crypto payload offset

[Max Reitz]

On 2017-09-12 13:28, Daniel P. Berrange wrote:
> The crypto APIs report the offset of the data payload as an uint64_t
> type, but the block driver is casting to size_t or ssize_t which will
> potentially truncate.
> 
> Most of the block APIs use int64_t for offsets meanwhile, so even if
> using uint64_t in the crypto block driver we are still at risk of
> truncation.
> 
> Change the block crypto driver to use uint64_t, but add asserts that
> the value is less than INT64_MAX.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  block/crypto.c | 13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

[Qemu-block] [PATCH v3 3/7] block: fix data type casting for crypto payload offset

[Daniel P. Berrange]

The crypto APIs report the offset of the data payload as an uint64_t
type, but the block driver is casting to size_t or ssize_t which will
potentially truncate.

Most of the block APIs use int64_t for offsets meanwhile, so even if
using uint64_t in the crypto block driver we are still at risk of
truncation.

Change the block crypto driver to use uint64_t, but add asserts that
the value is less than INT64_MAX.

Signed-off-by: Daniel P. Berrange 
---
 block/crypto.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index cc8afe0e0d..d68cbac2ac 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -364,8 +364,9 @@ static int block_crypto_truncate(BlockDriverState *bs, 
int64_t offset,
  PreallocMode prealloc, Error **errp)
 {
 BlockCrypto *crypto = bs->opaque;
-size_t payload_offset =
+uint64_t payload_offset =
 qcrypto_block_get_payload_offset(crypto->block);
+assert(payload_offset < (INT64_MAX - offset));
 
 offset += payload_offset;
 
@@ -391,8 +392,9 @@ block_crypto_co_readv(BlockDriverState *bs, int64_t 
sector_num,
 uint8_t *cipher_data = NULL;
 QEMUIOVector hd_qiov;
 int ret = 0;
-size_t payload_offset =
+uint64_t payload_offset =
 qcrypto_block_get_payload_offset(crypto->block) / 512;
+assert(payload_offset < (INT64_MAX / 512));
 
 qemu_iovec_init(_qiov, qiov->niov);
 
@@ -458,8 +460,9 @@ block_crypto_co_writev(BlockDriverState *bs, int64_t 
sector_num,
 uint8_t *cipher_data = NULL;
 QEMUIOVector hd_qiov;
 int ret = 0;
-size_t payload_offset =
+uint64_t payload_offset =
 qcrypto_block_get_payload_offset(crypto->block) / 512;
+assert(payload_offset < (INT64_MAX / 512));
 
 qemu_iovec_init(_qiov, qiov->niov);
 
@@ -520,7 +523,9 @@ static int64_t block_crypto_getlength(BlockDriverState *bs)
 BlockCrypto *crypto = bs->opaque;
 int64_t len = bdrv_getlength(bs->file->bs);
 
-ssize_t offset = qcrypto_block_get_payload_offset(crypto->block);
+uint64_t offset = qcrypto_block_get_payload_offset(crypto->block);
+assert(offset < INT64_MAX);
+assert(offset < len);
 
 len -= offset;
 
-- 
2.13.5