From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: Laurent Vivier <laurent@vivier.eu>, qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
Stefano Garzarella <sgarzare@redhat.com>
Subject: Re: [PATCH] hw/elf_ops: clear uninitialized segment space
Date: Thu, 15 Apr 2021 11:57:44 +0200 [thread overview]
Message-ID: <934a3f68-8800-4774-0326-6881edff94e4@redhat.com> (raw)
In-Reply-To: <97b4d188-9fb6-3ba3-183a-82f926ab016c@amsat.org>
On 4/15/21 10:44 AM, Philippe Mathieu-Daudé wrote:
> 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?
I'll post a patch.
prev parent reply other threads:[~2021-04-15 9:59 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-14 10:58 [PATCH] hw/elf_ops: clear uninitialized segment space Laurent Vivier
2021-04-14 12:16 ` Philippe Mathieu-Daudé
2021-04-14 12:41 ` Laurent Vivier
2021-04-15 8:44 ` Philippe Mathieu-Daudé
2021-04-15 9:57 ` Philippe Mathieu-Daudé [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=934a3f68-8800-4774-0326-6881edff94e4@redhat.com \
--to=philmd@redhat.com \
--cc=laurent@vivier.eu \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=sgarzare@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).