Re: [Qemu-block] [PATCH 1/7] qcow2: Generalize validate_table_offset() into qcow2_validate_table()

[Eric Blake]

On 03/01/2018 10:27 AM, Alberto Garcia wrote:
> This function checks that the offset and size of a table are valid.
>
> While the offset checks are fine, the size check is too generic, since
> it only verifies that the total size in bytes fits in a 64-bit
> integer. In practice all tables used in qcow2 have much smaller size
> limits, so the size needs to be checked again for each table using its
> actual limit.
>
> This patch generalizes this function by allowing the caller to specify
> the maximum size for that table. In addition to that it allows passing
> an Error variable.
>
> The function is also renamed and made public since we're going to use
> it in other parts of the code.
>
> Signed-off-by: Alberto Garcia <address@hidden>
> ---

> +int qcow2_validate_table(BlockDriverState *bs, uint64_t offset,
> +                         uint64_t entries, size_t entry_len,
> +                         int64_t max_size_bytes, const char *table_name,
> +                         Error **errp)
>   {
>       BDRVQcow2State *s = bs->opaque;
> -    uint64_t size;
> +
> +    if (entries > max_size_bytes / entry_len) {
> +        error_setg(errp, "%s too large", table_name);
> +        return -EFBIG;
> +    }

EFBIG "File too large".  Would EOVERFLOW "Value too large for defined 
data type" make any more sense?  But that's bikeshedding; I'm okay with 
your choice.


>   
>       /* read the level 1 table */
> -    if (header.l1_size > QCOW_MAX_L1_SIZE / sizeof(uint64_t)) {
> -        error_setg(errp, "Active L1 table too large");
> -        ret = -EFBIG;
> +    ret = qcow2_validate_table(bs, header.l1_table_offset,
> +                               header.l1_size, sizeof(uint64_t),
> +                               QCOW_MAX_L1_SIZE, "Active L1 table", errp);
> +    if (ret < 0) {

At any rate, it looks like we were already using EFBIG.

I also like that you consolidated more checking into the common function.

Reviewed-by: Eric Blake <address@hidden>

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