Re: [Qemu-devel] [PATCH 02/10] hw/ppc/ppc405_boards: Don't use load_image()

[Eric Blake]

On 11/30/18 9:17 AM, Peter Maydell wrote:
> The load_image() function is deprecated, as it does not let the
> caller specify how large the buffer to read the file into is.
> Instead use load_image_size().
>
> Signed-off-by: Peter Maydell <address@hidden>
> ---
>   hw/ppc/ppc405_boards.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> index 3be3fe4432b..1b0a0a8ba3a 100644
> --- a/hw/ppc/ppc405_boards.c
> +++ b/hw/ppc/ppc405_boards.c
> @@ -219,9 +219,11 @@ static void ref405ep_init(MachineState *machine)
>               bios_name = BIOS_FILENAME;
>           filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>           if (filename) {
> -            bios_size = load_image(filename, memory_region_get_ram_ptr(bios));
> +            bios_size = load_image_size(filename,
> +                                        memory_region_get_ram_ptr(bios),
> +                                        BIOS_SIZE);
>               g_free(filename);
> -            if (bios_size < 0 || bios_size > BIOS_SIZE) {

That old code is so wrong - "if we already overflowed the destination, 
possibly allowing for RCE in the meantime which might not even return to 
executing this code, THEN check and report the overflow".

> +            if (bios_size < 0) {
>                   error_report("Could not load PowerPC BIOS '%s'", bios_name);
>                   exit(1);
>               }

MUCH safer, even if silent truncation happens.
Reviewed-by: Eric Blake <address@hidden>

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org