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

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

On 11/6/24 09:42, Paolo Bonzini wrote:
> Check for overflow to avoid that fseek() receives a sign-extended value.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   include/qemu/osdep.h | 4 ++++
>   hw/core/eif.c        | 4 ++++
>   2 files changed, 8 insertions(+)
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index fe7c3c5f673..fdff07fd992 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -297,6 +297,10 @@ void QEMU_ERROR("code path is reachable")
>   #error building with G_DISABLE_ASSERT is not supported
>   #endif
>   
> +#ifndef OFF_MAX
> +#define OFF_MAX (sizeof (off_t) == 8 ? INT64_MAX : INT32_MAX)
> +#endif
> +
>   #ifndef O_LARGEFILE
>   #define O_LARGEFILE 0
>   #endif
> diff --git a/hw/core/eif.c b/hw/core/eif.c
> index 7f3b2edc9a7..cbcd80de58b 100644
> --- a/hw/core/eif.c
> +++ b/hw/core/eif.c
> @@ -115,6 +115,10 @@ static bool read_eif_header(FILE *f, EifHeader *header, uint32_t *crc,
>   
>       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.

> +            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>