Re: [PATCH v3 3/3] hw/core/loader: clear uninitialized ROM space

[Philippe Mathieu-Daudé]

On 4/29/21 4:13 PM, Laurent Vivier wrote:
> As for "hw/elf_ops: clear uninitialized segment space" we need to
> clear the uninitialized space when the ELF is set in ROM.
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  hw/core/loader.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index d3e5f3b423f6..8146fdcbb7a0 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -1146,9 +1146,13 @@ static void rom_reset(void *unused)
>          if (rom->mr) {
>              void *host = memory_region_get_ram_ptr(rom->mr);
>              memcpy(host, rom->data, rom->datasize);
> +            memset(host + rom->datasize, 0, rom->romsize - rom->datasize);
>          } else {
>              address_space_write_rom(rom->as, rom->addr, 
> MEMTXATTRS_UNSPECIFIED,
>                                      rom->data, rom->datasize);
> +            address_space_set(rom->as, rom->addr + rom->datasize, 0,
> +                              rom->romsize - rom->datasize,
> +                              MEMTXATTRS_UNSPECIFIED);
>          }
>          if (rom->isrom) {
>              /* rom needs to be written only once */
> 

This is consistent with the comment from commit d60fa42e8ba
("Save memory allocation in the elf loader"):

    /* datasize is the amount of memory allocated in "data". If datasize
is less
     * than romsize, it means that the area from datasize to romsize is
filled
     * with zeros.
     */

Therefore:

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

However depending on the underlying media, there might be cases
where we want to fill with -1 instead. Just to keep in mind, if
one day it bites us.

Regards,

Phil.