On Tue, 10 Sep 2019 at 10:08, Stefano Garzarella <sgarz...@redhat.com> wrote:
>
> This patch fixes a possible integer overflow when we calculate
> the total size of ELF segments loaded.
>
> Reported-by: Coverity (CID 1405299)
> Signed-off-by: Stefano Garzarella <sgarz...@redhat.com>
> ---
> Now we are limited to INT_MAX, should load_elf() returns ssize_t
> to support bigger ELFs?
> ---
> include/hw/elf_ops.h | 6 ++++++
> hw/core/loader.c | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
> index 1496d7e753..46dd3bf413 100644
> --- a/include/hw/elf_ops.h
> +++ b/include/hw/elf_ops.h
> @@ -485,6 +485,12 @@ static int glue(load_elf, SZ)(const char *name, int fd,
> }
> }
>
> + if (mem_size > INT_MAX - total_size) {
> + error_report("ELF total segments size is too big to load "
> + "max is %d)", INT_MAX);
> + goto fail;
> + }
This function doesn't report issues via error_report()
(some callers intentionally have fallback options for
what they try to do with the file), but by returning
a suitable error value in 'ret', so I think we should
continue that approach rather than adding an error_report()
call here.
I agree that accumulating the size in an 'int' is a bit
dubious these days.
thanks
-- PMM
