Re: [Qemu-devel] [PATCH] use an unsigned long for the max_sz parameter in load_image_targphys

[Markus Armbruster]

Mark Langsdorf <address@hidden> writes:

> On 03/09/2012 08:17 AM, Markus Armbruster wrote:
>> Mark Langsdorf <address@hidden> writes:
>> 
>>> On 03/09/2012 03:25 AM, Markus Armbruster wrote:
>>>> get_image_size() returns int.  How does widening size and max_sz here
>>>> improve things?
>>>
>>> If max_sz is greater than 2GB, then:
>>>   int max_sz = 0xc0000000;
>>>   int size =   0x300;
>>>   if (size > max_sz)
>>>       return -1;
>>>
>>> returns -1, even though size is much less than max_sz.
>>>
>>> doing it my way:
>>>   unsigned long max_sz = 0xc0000000;
>>>   unsigned long size =   0x300;
>>>   if (size > max_sz)
>>>       return -1;
>>>
>>> does not return -1.
>> 
>> The current code limits max_sz to INT_MAX.  Passing 0xc0000000 is a bug.
>> 
>> You want to relax the limit.  That's fair.  But I'm afraid your patch
>> doesn't really relax the limit, or rather it relaxes it just by one bit.
>> 
>> Your example shows it works for a case where the higher limit isn't
>> needed.  Let's examine three cases where it is: image sizes 2GiB, 4GiB
>> and 5GiB on a host with 32 bit twos complement int, and 64 bit unsigned
>> size_t, max_sz 0xc0000000.
>
> I'm quite willing to leave the problem of people loading 2GiB images to
> another day. Loading a 300KiB on an ARM machine with 4GiB of memory is
> a problem I'm facing right now, though.

Easiest fix for that is passing a max_sz argument that doesn't overflow
the parameter type :)

[...]