On Tue, Sep 10, 2019 at 10:50:30AM +0100, Alex Bennée wrote:
>
> Stefano Garzarella <sgarz...@redhat.com> writes:
>
> > 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;
> > + }
> > +
>
> Seem sensible enough (although gah, I hate these glue bits). Would the
> large amount of goto fail logic be something that could be cleaned up
> with the automatic cleanup functions we recently mentioned in
> CODING_STYLE.rst?
>
As Peter pointed out, maybe we should keep the 'goto fail' and do a
better cleanup, but thanks to pointing that out to me.
> Anyway:
>
> Reviewed-by: Alex Bennée <alex.ben...@linaro.org>
Thanks for the review!
I'm sending a v2 following the Peter's suggestions,
removing the error_report and returing a new ELF_LOAD_TOO_BIG error
value.
Can I keep your R-b, or would you like to have a look at v2?
Thanks,
Stefano
