Bug 1217351

l1_size is already checked against 512 * 1024 * 1024. See where is is assigned:

static int vmdk_add_extent(BlockDriverState *bs,
                           BlockDriverState *file, bool flat, int64_t sectors,
                           int64_t l1_offset, int64_t l1_backup_offset,
                           uint32_t l1_size,
                           int l2_size, uint64_t cluster_sectors,
                           VmdkExtent **new_extent,
                           Error **errp)
{
    ...

    if (l1_size > 512 * 1024 * 1024) {
        /* Although with big capacity and small l1_entry_sectors, we can get a
         * big l1_size, we don't want unbounded value to allocate the table.
         * Limit it to 512M, which is 16PB for default cluster and L2 table
         * size */
        error_setg(errp, "L1 size too big");
        return -EFBIG;
    }

    ...
    extent->l1_size = l1_size;

Did you try the disk image?  It demonstrates the overflow happening.

Yes, Rich, you're right, the image does cause an overflow.

In fact, that's the only possible value to overflow (extent->l1_size = 0x20000000):

l1_size = extent->l1_size * sizeof(long) => 0x80000000;

g_try_malloc returns NULL because l1_size is interpreted as negative during type casting from 'int' to 'gsize', which yields a very large value. Eventually, by coincidence, we get expected behavior:

qemu-img: Could not open '/tmp/afl6.img': Could not open '/tmp/afl6.img': Cannot allocate memory

Values larger than 0x20000000 will be refused as the code in comment #2 shows.

Values smaller than 0x20000000 will not overflow l1_size.

Because it only happens to work, this is still a valid bug report and we should fix the overflow.

Thanks!

Fam

Making this bug public since it has been reported upstream and
since it is not exploitable after all.

Patch is:
https://lists.nongnu.org/archive/html/qemu-devel/2015-05/msg00413.html

Fix included in qemu-kvm-1.5.3-91.el7

1. Test this bug on qemu-img-1.5.3-90.el7.x86_64
# qemu-img info afl6.img 
qemu-img: Could not open 'afl6.img': Could not open 'afl6.img': Cannot allocate memory

2. Upgrade qemu-img to qemu-img-1.5.3-97.el7.x86_64.

# qemu-img info afl6.img 
tcmalloc: large alloc 2147483648 bytes == 0x7f8252fd6000 @  0x7f824e99ac73 0x7f824e9badb2 0x7f824e49f5ff 0x7f825017b03f 0x7f825017cb42 0x7f825017c318 0x7f825017cf08 0x7f825014c254 0x7f82501516fb 0x7f82501a228d 0x7f82501a26b6 0x7f824ca39af5 0x7f8250148f8d (nil)
qemu-img: Could not open 'afl6.img': Could not read l1 table from extent 'afl6.img': Invalid argument

Fam, is this expected for this bug fix?

Thanks,
Mazhang.

Yes this is correct.  Without the patch, a calculation in qemu-img
overflows causing it to allocate a negative amount of memory and
you see the error "Cannot allocate memory".  With the patch the
allocation is correct and it fails a bit later with "Invalid argument".

The test image is (deliberately) faulty so it cannot be opened, so
the only question is which error you get.

(In reply to Richard W.M. Jones from comment #9)
> Yes this is correct.  Without the patch, a calculation in qemu-img
> overflows causing it to allocate a negative amount of memory and
> you see the error "Cannot allocate memory".  With the patch the
> allocation is correct and it fails a bit later with "Invalid argument".
> 
> The test image is (deliberately) faulty so it cannot be opened, so
> the only question is which error you get.

Thanks for your explanation, let's set this bug verified.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHBA-2015-2213.html