Re: [PATCH] eif: cope with huge section offsets

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 11/6/24 09:49, Paolo Bonzini wrote:
> On Wed, Nov 6, 2024 at 6:47 PM Pierrick Bouvier
> <pierrick.bouvier@linaro.org> wrote:
> 
>>>        for (int i = 0; i < MAX_SECTIONS; ++i) {
>>>            header->section_offsets[i] = be64_to_cpu(header->section_offsets[i]);
>>> +        if (header->section_offsets[i] > OFF_MAX) {
>>
>> Maybe we could add a comment that sections_offsets is unsigned, as it
>> can be confusing to read value > INT_MAX without more context.
> 
> It does sound like OFF_MAX is related to section_offsets[], but it's
> actually related to off_t.  So the comparison is with the maximum
> value of off_t, which is signed.
> 
> The problem would happen even if section_offsets[] was signed (for
> example off_t could be 32-bit).
> 

I'm a bit confused.
It works because section_offsets[i] is unsigned. If it was signed, and 
sizeof(off_t) is 8, we can never satisfy "(int64) > INT64_MAX".

> Paolo
> 
>>> +            error_setg(errp, "Invalid EIF image. Section offset out of bounds");
>>> +            return false;
>>> +        }
>>>        }
>>>
>>>        for (int i = 0; i < MAX_SECTIONS; ++i) {
>>
>> Else,
>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>
>