Re: [PATCH] hw/elf_ops: clear uninitialized segment space

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

On 4/14/21 2:41 PM, Laurent Vivier wrote:
> Le 14/04/2021 à 14:16, Philippe Mathieu-Daudé a écrit :
>> On 4/14/21 12:58 PM, Laurent Vivier wrote:
>>> When the mem_size of the segment is bigger than the file_size,
>>> and if this space doesn't overlap another segment, it needs
>>> to be cleared.
>>>
>>> This bug is very similar to the one we had for linux-user,
>>> 22d113b52f41 ("linux-user: Fix loading of BSS segments"),
>>> where .bss section is encoded as an extension of the the data
>>> one by setting the segment p_memsz > p_filesz.
>>>
>>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>>> ---
>>>  include/hw/elf_ops.h | 17 +++++++++++++++++
>>>  1 file changed, 17 insertions(+)
>>>
>>> diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
>>> index 6ee458e7bc3c..e3dcee3ee349 100644
>>> --- a/include/hw/elf_ops.h
>>> +++ b/include/hw/elf_ops.h
>>> @@ -562,6 +562,23 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>>>                      if (res != MEMTX_OK) {
>>>                          goto fail;
>>>                      }
>>> +                    /*
>>> +                     * We need to zero'ify the space that is not copied
>>> +                     * from file
>>> +                     */
>>> +                    if (file_size < mem_size) {
>>> +                        static uint8_t zero[4096];
>>
>> Given it is unlikely, maybe better use:
>>
>>               g_autofree uint8_t *zero = g_new0(uint8_t, 4096);
> 
> I don't know what is the best solution but this seems to introduce a lot of complexity only to have
> a page of 0s.

Less complex alternative is to use dma_memory_set():

   dma_memory_set(as, file_size, 0, mem_size - file_size);

Actually we should extract address_space_set() from it, keeping
the dma_barrier() call in dma_memory_set(), and use address_space_set()
here.

What do you think?

Phil.