* [PATCH] hw/elf_ops: clear uninitialized segment space
@ 2021-04-14 10:58 Laurent Vivier
2021-04-14 12:16 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 5+ messages in thread
From: Laurent Vivier @ 2021-04-14 10:58 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé,
Laurent Vivier, Stefano Garzarella
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];
+ uint64_t i;
+ for (i = file_size; i < mem_size; i += sizeof(zero)) {
+ res = address_space_write(
+ as ? as : &address_space_memory,
+ addr + i, MEMTXATTRS_UNSPECIFIED,
+ zero, MIN(sizeof(zero), mem_size - i));
+ if (res != MEMTX_OK) {
+ goto fail;
+ }
+ }
+ }
}
}
--
2.30.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] hw/elf_ops: clear uninitialized segment space
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
0 siblings, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-14 12:16 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: Peter Maydell, Stefano Garzarella
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);
> + uint64_t i;
> + for (i = file_size; i < mem_size; i += sizeof(zero)) {
> + res = address_space_write(
> + as ? as : &address_space_memory,
> + addr + i, MEMTXATTRS_UNSPECIFIED,
> + zero, MIN(sizeof(zero), mem_size - i));
> + if (res != MEMTX_OK) {
> + goto fail;
> + }
> + }
> + }
> }
> }
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] hw/elf_ops: clear uninitialized segment space
2021-04-14 12:16 ` Philippe Mathieu-Daudé
@ 2021-04-14 12:41 ` Laurent Vivier
2021-04-15 8:44 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 5+ messages in thread
From: Laurent Vivier @ 2021-04-14 12:41 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel; +Cc: Peter Maydell, Stefano Garzarella
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.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] hw/elf_ops: clear uninitialized segment space
2021-04-14 12:41 ` Laurent Vivier
@ 2021-04-15 8:44 ` Philippe Mathieu-Daudé
2021-04-15 9:57 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-15 8:44 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: Peter Maydell, Stefano Garzarella
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.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] hw/elf_ops: clear uninitialized segment space
2021-04-15 8:44 ` Philippe Mathieu-Daudé
@ 2021-04-15 9:57 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-15 9:57 UTC (permalink / raw)
To: Laurent Vivier, qemu-devel; +Cc: Peter Maydell, Stefano Garzarella
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-04-15 9:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 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).