Re: [Qemu-devel] [PATCH v3 1/3] qemu-io: fix cvtnum lval types

[Kevin Wolf <kwolf@redhat.com>]

Am 04.11.2015 um 01:17 hat John Snow geschrieben:
> cvtnum() returns int64_t: we should not be storing this
> result inside of an int.
> 
> In a few cases, we need an extra sprinkling of error handling
> where we expect to pass this number on towards a function that
> expects something smaller than int64_t.
> 
> Reported-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  qemu-io-cmds.c | 88 +++++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 53 insertions(+), 35 deletions(-)

> v3:
> - pulled a lot of loose yarn, now missing my sweater
>   (Updated patch 1 even further, reported-by Kevin)

I'm afraid you'll have to start using up another sweater.

> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 6e5d1e4..f04c1db 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -294,7 +294,7 @@ static void qemu_io_free(void *p)
>      qemu_vfree(p);
>  }
>  
> -static void dump_buffer(const void *buffer, int64_t offset, int len)
> +static void dump_buffer(const void *buffer, int64_t offset, int64_t len)
>  {
>      int i, j;
>      const uint8_t *p;

One more line of context:

    for (i = 0, p = buffer; i < len; i += 16) {

For len > INT_MAX, this is an endless loop. The same way, i + j a few
lines below can wrap around.

> @@ -393,8 +393,8 @@ fail:
>      return buf;
>  }
>  
> -static int do_read(BlockBackend *blk, char *buf, int64_t offset, int count,
> -                   int *total)
> +static int do_read(BlockBackend *blk, char *buf, int64_t offset, int64_t count,
> +                   int64_t *total)
>  {
>      int ret;

Again, one more line of context:

    ret = blk_read(blk, offset >> 9, (uint8_t *)buf, count >> 9);

count is silently truncated if it's larger than INT_MAX << 9. I think we
should return an error (ERANGE? EINVAL? EFBIG?) instead.

Same for do_write, do_pread, do_pwrite, co_write_zeroes_entry,
do_write_compressed, do_load_vmstate, do_save_vmstate.

Kevin