Re: [PATCH v2] q800: fix segfault with invalid MacROM

[Thomas Huth]

On 07/01/2022 11.50, Laurent Vivier wrote:
> "qemu-system-m68k -M q800 -bios /dev/null" crashes with a segfault
> in q800_init().
> This happens because the code doesn't check that rom_ptr() returned
> a non-NULL pointer .
>
> To avoid NULL pointer, don't allow 0 sized file and use bios_size with
> rom_ptr().
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/756
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>   hw/m68k/q800.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index e4c7c9b88ad0..55dfe5036f40 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -672,12 +672,13 @@ static void q800_init(MachineState *machine)
>   
>           /* Remove qtest_enabled() check once firmware files are in the tree */
>           if (!qtest_enabled()) {
> -            if (bios_size < 0 || bios_size > MACROM_SIZE) {
> +            if (bios_size <= 0 || bios_size > MACROM_SIZE) {
>                   error_report("could not load MacROM '%s'", bios_name);
>                   exit(1);
>               }
>   
> -            ptr = rom_ptr(MACROM_ADDR, MACROM_SIZE);
> +            ptr = rom_ptr(MACROM_ADDR, bios_size);
> +            assert(ptr != NULL);
>               stl_phys(cs->as, 0, ldl_p(ptr));    /* reset initial SP */
>               stl_phys(cs->as, 4,
>                        MACROM_ADDR + ldl_p(ptr + 4)); /* reset initial PC */

Looks nicer than v1, indeed.

Reviewed-by: Thomas Huth <thuth@redhat.com>