* [PATCH v2 0/2] hw/elf_ops: clear uninitialized segment space @ 2021-04-15 10:04 Philippe Mathieu-Daudé 2021-04-15 10:04 ` [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() Philippe Mathieu-Daudé 2021-04-15 10:04 ` [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé 0 siblings, 2 replies; 12+ messages in thread From: Philippe Mathieu-Daudé @ 2021-04-15 10:04 UTC (permalink / raw) To: Stefano Garzarella, Laurent Vivier, qemu-devel Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini Respining Laurent's patch, but extracting the address_space_set() helper. Laurent Vivier (1): hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé (1): exec/memory: Extract address_space_set() from dma_memory_set() include/exec/memory.h | 16 ++++++++++++++++ include/hw/elf_ops.h | 13 +++++++++++++ softmmu/dma-helpers.c | 16 +--------------- softmmu/physmem.c | 19 +++++++++++++++++++ 4 files changed, 49 insertions(+), 15 deletions(-) -- 2.26.3 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() 2021-04-15 10:04 [PATCH v2 0/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé @ 2021-04-15 10:04 ` Philippe Mathieu-Daudé 2021-04-15 10:46 ` Laurent Vivier ` (2 more replies) 2021-04-15 10:04 ` [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé 1 sibling, 3 replies; 12+ messages in thread From: Philippe Mathieu-Daudé @ 2021-04-15 10:04 UTC (permalink / raw) To: Stefano Garzarella, Laurent Vivier, qemu-devel Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini dma_memory_set() does a DMA barrier, set the address space with a constant value. The constant value filling code is not specific to DMA and can be used for AddressSpace. Extract it as a new helper: address_space_set(). Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/exec/memory.h | 16 ++++++++++++++++ softmmu/dma-helpers.c | 16 +--------------- softmmu/physmem.c | 19 +++++++++++++++++++ 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index 5728a681b27..192139af58e 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -2568,6 +2568,22 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr, } } +/** + * address_space_set: Fill address space with a constant byte. + * + * Return a MemTxResult indicating whether the operation succeeded + * or failed (eg unassigned memory, device rejected the transaction, + * IOMMU fault). + * + * @as: #AddressSpace to be accessed + * @addr: address within that address space + * @c: constant byte to fill the memory + * @len: the number of bytes to fill with the constant byte + * @attrs: memory transaction attributes + */ +MemTxResult address_space_set(AddressSpace *as, hwaddr addr, + uint8_t c, hwaddr len, MemTxAttrs attrs); + #ifdef NEED_CPU_H /* enum device_endian to MemOp. */ static inline MemOp devend_memop(enum device_endian end) diff --git a/softmmu/dma-helpers.c b/softmmu/dma-helpers.c index 7d766a5e89a..8e1e7ad5320 100644 --- a/softmmu/dma-helpers.c +++ b/softmmu/dma-helpers.c @@ -23,21 +23,7 @@ MemTxResult dma_memory_set(AddressSpace *as, dma_addr_t addr, { dma_barrier(as, DMA_DIRECTION_FROM_DEVICE); -#define FILLBUF_SIZE 512 - uint8_t fillbuf[FILLBUF_SIZE]; - int l; - MemTxResult error = MEMTX_OK; - - memset(fillbuf, c, FILLBUF_SIZE); - while (len > 0) { - l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE; - error |= address_space_write(as, addr, MEMTXATTRS_UNSPECIFIED, - fillbuf, l); - len -= l; - addr += l; - } - - return error; + return address_space_set(as, addr, c, len, MEMTXATTRS_UNSPECIFIED); } void qemu_sglist_init(QEMUSGList *qsg, DeviceState *dev, int alloc_hint, diff --git a/softmmu/physmem.c b/softmmu/physmem.c index 85034d9c11e..c9117527ae7 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -2891,6 +2891,25 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, } } +MemTxResult address_space_set(AddressSpace *as, hwaddr addr, + uint8_t c, hwaddr len, MemTxAttrs attrs) +{ +#define FILLBUF_SIZE 512 + uint8_t fillbuf[FILLBUF_SIZE]; + int l; + MemTxResult error = MEMTX_OK; + + memset(fillbuf, c, FILLBUF_SIZE); + while (len > 0) { + l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE; + error |= address_space_write(as, addr, attrs, fillbuf, l); + len -= l; + addr += l; + } + + return error; +} + void cpu_physical_memory_rw(hwaddr addr, void *buf, hwaddr len, bool is_write) { -- 2.26.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() 2021-04-15 10:04 ` [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() Philippe Mathieu-Daudé @ 2021-04-15 10:46 ` Laurent Vivier 2021-04-15 11:00 ` Stefano Garzarella 2021-04-15 22:32 ` Richard Henderson 2 siblings, 0 replies; 12+ messages in thread From: Laurent Vivier @ 2021-04-15 10:46 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Stefano Garzarella, qemu-devel Cc: Peter Maydell, Paolo Bonzini Le 15/04/2021 à 12:04, Philippe Mathieu-Daudé a écrit : > dma_memory_set() does a DMA barrier, set the address space with > a constant value. The constant value filling code is not specific > to DMA and can be used for AddressSpace. Extract it as a new > helper: address_space_set(). > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > include/exec/memory.h | 16 ++++++++++++++++ > softmmu/dma-helpers.c | 16 +--------------- > softmmu/physmem.c | 19 +++++++++++++++++++ > 3 files changed, 36 insertions(+), 15 deletions(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 5728a681b27..192139af58e 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -2568,6 +2568,22 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr, > } > } > > +/** > + * address_space_set: Fill address space with a constant byte. > + * > + * Return a MemTxResult indicating whether the operation succeeded > + * or failed (eg unassigned memory, device rejected the transaction, > + * IOMMU fault). > + * > + * @as: #AddressSpace to be accessed > + * @addr: address within that address space > + * @c: constant byte to fill the memory > + * @len: the number of bytes to fill with the constant byte > + * @attrs: memory transaction attributes > + */ > +MemTxResult address_space_set(AddressSpace *as, hwaddr addr, > + uint8_t c, hwaddr len, MemTxAttrs attrs); > + > #ifdef NEED_CPU_H > /* enum device_endian to MemOp. */ > static inline MemOp devend_memop(enum device_endian end) > diff --git a/softmmu/dma-helpers.c b/softmmu/dma-helpers.c > index 7d766a5e89a..8e1e7ad5320 100644 > --- a/softmmu/dma-helpers.c > +++ b/softmmu/dma-helpers.c > @@ -23,21 +23,7 @@ MemTxResult dma_memory_set(AddressSpace *as, dma_addr_t addr, > { > dma_barrier(as, DMA_DIRECTION_FROM_DEVICE); > > -#define FILLBUF_SIZE 512 > - uint8_t fillbuf[FILLBUF_SIZE]; > - int l; > - MemTxResult error = MEMTX_OK; > - > - memset(fillbuf, c, FILLBUF_SIZE); > - while (len > 0) { > - l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE; > - error |= address_space_write(as, addr, MEMTXATTRS_UNSPECIFIED, > - fillbuf, l); > - len -= l; > - addr += l; > - } > - > - return error; > + return address_space_set(as, addr, c, len, MEMTXATTRS_UNSPECIFIED); > } > > void qemu_sglist_init(QEMUSGList *qsg, DeviceState *dev, int alloc_hint, > diff --git a/softmmu/physmem.c b/softmmu/physmem.c > index 85034d9c11e..c9117527ae7 100644 > --- a/softmmu/physmem.c > +++ b/softmmu/physmem.c > @@ -2891,6 +2891,25 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, > } > } > > +MemTxResult address_space_set(AddressSpace *as, hwaddr addr, > + uint8_t c, hwaddr len, MemTxAttrs attrs) > +{ > +#define FILLBUF_SIZE 512 > + uint8_t fillbuf[FILLBUF_SIZE]; > + int l; > + MemTxResult error = MEMTX_OK; > + > + memset(fillbuf, c, FILLBUF_SIZE); > + while (len > 0) { > + l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE; > + error |= address_space_write(as, addr, attrs, fillbuf, l); > + len -= l; > + addr += l; > + } > + > + return error; > +} > + > void cpu_physical_memory_rw(hwaddr addr, void *buf, > hwaddr len, bool is_write) > { > Reviewed-by: Laurent Vivier <laurent@vivier.eu> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() 2021-04-15 10:04 ` [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() Philippe Mathieu-Daudé 2021-04-15 10:46 ` Laurent Vivier @ 2021-04-15 11:00 ` Stefano Garzarella 2021-04-15 11:05 ` Peter Maydell 2021-04-15 22:32 ` Richard Henderson 2 siblings, 1 reply; 12+ messages in thread From: Stefano Garzarella @ 2021-04-15 11:00 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Peter Maydell, Paolo Bonzini, Laurent Vivier, qemu-devel On Thu, Apr 15, 2021 at 12:04:08PM +0200, Philippe Mathieu-Daudé wrote: >dma_memory_set() does a DMA barrier, set the address space with >a constant value. The constant value filling code is not specific >to DMA and can be used for AddressSpace. Extract it as a new >helper: address_space_set(). > >Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >--- > include/exec/memory.h | 16 ++++++++++++++++ > softmmu/dma-helpers.c | 16 +--------------- > softmmu/physmem.c | 19 +++++++++++++++++++ > 3 files changed, 36 insertions(+), 15 deletions(-) > >diff --git a/include/exec/memory.h b/include/exec/memory.h >index 5728a681b27..192139af58e 100644 >--- a/include/exec/memory.h >+++ b/include/exec/memory.h >@@ -2568,6 +2568,22 @@ address_space_write_cached(MemoryRegionCache *cache, hwaddr addr, > } > } > >+/** >+ * address_space_set: Fill address space with a constant byte. >+ * >+ * Return a MemTxResult indicating whether the operation succeeded >+ * or failed (eg unassigned memory, device rejected the transaction, >+ * IOMMU fault). >+ * >+ * @as: #AddressSpace to be accessed >+ * @addr: address within that address space >+ * @c: constant byte to fill the memory >+ * @len: the number of bytes to fill with the constant byte >+ * @attrs: memory transaction attributes >+ */ >+MemTxResult address_space_set(AddressSpace *as, hwaddr addr, >+ uint8_t c, hwaddr len, MemTxAttrs attrs); >+ > #ifdef NEED_CPU_H > /* enum device_endian to MemOp. */ > static inline MemOp devend_memop(enum device_endian end) >diff --git a/softmmu/dma-helpers.c b/softmmu/dma-helpers.c >index 7d766a5e89a..8e1e7ad5320 100644 >--- a/softmmu/dma-helpers.c >+++ b/softmmu/dma-helpers.c >@@ -23,21 +23,7 @@ MemTxResult dma_memory_set(AddressSpace *as, dma_addr_t addr, > { > dma_barrier(as, DMA_DIRECTION_FROM_DEVICE); > >-#define FILLBUF_SIZE 512 >- uint8_t fillbuf[FILLBUF_SIZE]; >- int l; >- MemTxResult error = MEMTX_OK; >- >- memset(fillbuf, c, FILLBUF_SIZE); >- while (len > 0) { >- l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE; >- error |= address_space_write(as, addr, MEMTXATTRS_UNSPECIFIED, >- fillbuf, l); >- len -= l; >- addr += l; >- } >- >- return error; >+ return address_space_set(as, addr, c, len, MEMTXATTRS_UNSPECIFIED); > } > > void qemu_sglist_init(QEMUSGList *qsg, DeviceState *dev, int alloc_hint, >diff --git a/softmmu/physmem.c b/softmmu/physmem.c >index 85034d9c11e..c9117527ae7 100644 >--- a/softmmu/physmem.c >+++ b/softmmu/physmem.c >@@ -2891,6 +2891,25 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs, > } > } > >+MemTxResult address_space_set(AddressSpace *as, hwaddr addr, >+ uint8_t c, hwaddr len, MemTxAttrs attrs) >+{ >+#define FILLBUF_SIZE 512 >+ uint8_t fillbuf[FILLBUF_SIZE]; >+ int l; >+ MemTxResult error = MEMTX_OK; >+ >+ memset(fillbuf, c, FILLBUF_SIZE); >+ while (len > 0) { What about return immediately if there is an error? I mean: while (len > 0 && result == MEMTX_OK) { I don't have a strong opinion on that, so in both cases: Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> >+ l = len < FILLBUF_SIZE ? len : FILLBUF_SIZE; >+ error |= address_space_write(as, addr, attrs, fillbuf, l); >+ len -= l; >+ addr += l; >+ } >+ >+ return error; >+} >+ > void cpu_physical_memory_rw(hwaddr addr, void *buf, > hwaddr len, bool is_write) > { >-- >2.26.3 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() 2021-04-15 11:00 ` Stefano Garzarella @ 2021-04-15 11:05 ` Peter Maydell 2021-04-15 11:18 ` Stefano Garzarella 0 siblings, 1 reply; 12+ messages in thread From: Peter Maydell @ 2021-04-15 11:05 UTC (permalink / raw) To: Stefano Garzarella Cc: Paolo Bonzini, Philippe Mathieu-Daudé, Laurent Vivier, QEMU Developers On Thu, 15 Apr 2021 at 12:00, Stefano Garzarella <sgarzare@redhat.com> wrote: > > On Thu, Apr 15, 2021 at 12:04:08PM +0200, Philippe Mathieu-Daudé wrote: > >dma_memory_set() does a DMA barrier, set the address space with > >a constant value. The constant value filling code is not specific > >to DMA and can be used for AddressSpace. Extract it as a new > >helper: address_space_set(). > > > >+MemTxResult address_space_set(AddressSpace *as, hwaddr addr, > >+ uint8_t c, hwaddr len, MemTxAttrs attrs) > >+{ > >+#define FILLBUF_SIZE 512 > >+ uint8_t fillbuf[FILLBUF_SIZE]; > >+ int l; > >+ MemTxResult error = MEMTX_OK; > >+ > >+ memset(fillbuf, c, FILLBUF_SIZE); > >+ while (len > 0) { > > What about return immediately if there is an error? > I mean: > while (len > 0 && result == MEMTX_OK) { I think that (a) we're just moving code here so we don't want to also change semantics; (b) there's a comment in memattrs.h that says * A zero (MEMTX_OK) response means success; anything else is a failure * of some kind. The memory subsystem will bitwise-OR together results * if it is synthesizing an operation from multiple smaller accesses. so in this function "keep going but merge errors" is in keeping with that principle. thanks -- PMM ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() 2021-04-15 11:05 ` Peter Maydell @ 2021-04-15 11:18 ` Stefano Garzarella 0 siblings, 0 replies; 12+ messages in thread From: Stefano Garzarella @ 2021-04-15 11:18 UTC (permalink / raw) To: Peter Maydell Cc: Paolo Bonzini, Philippe Mathieu-Daudé, Laurent Vivier, QEMU Developers On Thu, Apr 15, 2021 at 12:05:07PM +0100, Peter Maydell wrote: >On Thu, 15 Apr 2021 at 12:00, Stefano Garzarella <sgarzare@redhat.com> wrote: >> >> On Thu, Apr 15, 2021 at 12:04:08PM +0200, Philippe Mathieu-Daudé wrote: >> >dma_memory_set() does a DMA barrier, set the address space with >> >a constant value. The constant value filling code is not specific >> >to DMA and can be used for AddressSpace. Extract it as a new >> >helper: address_space_set(). > >> > >> >+MemTxResult address_space_set(AddressSpace *as, hwaddr addr, >> >+ uint8_t c, hwaddr len, MemTxAttrs attrs) >> >+{ >> >+#define FILLBUF_SIZE 512 >> >+ uint8_t fillbuf[FILLBUF_SIZE]; >> >+ int l; >> >+ MemTxResult error = MEMTX_OK; >> >+ >> >+ memset(fillbuf, c, FILLBUF_SIZE); >> >+ while (len > 0) { >> >> What about return immediately if there is an error? >> I mean: >> while (len > 0 && result == MEMTX_OK) { > >I think that (a) we're just moving code here so we don't want to also >change semantics; (b) there's a comment in memattrs.h that says > * A zero (MEMTX_OK) response means success; anything else is a failure > * of some kind. The memory subsystem will bitwise-OR together results > * if it is synthesizing an operation from multiple smaller accesses. > >so in this function "keep going but merge errors" is in keeping with that >principle. Got it, thanks for the explanation! :-) Stefano ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() 2021-04-15 10:04 ` [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() Philippe Mathieu-Daudé 2021-04-15 10:46 ` Laurent Vivier 2021-04-15 11:00 ` Stefano Garzarella @ 2021-04-15 22:32 ` Richard Henderson 2 siblings, 0 replies; 12+ messages in thread From: Richard Henderson @ 2021-04-15 22:32 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Stefano Garzarella, Laurent Vivier, qemu-devel Cc: Peter Maydell, Paolo Bonzini On 4/15/21 3:04 AM, Philippe Mathieu-Daudé wrote: > dma_memory_set() does a DMA barrier, set the address space with > a constant value. The constant value filling code is not specific > to DMA and can be used for AddressSpace. Extract it as a new > helper: address_space_set(). > > Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com> > --- > include/exec/memory.h | 16 ++++++++++++++++ > softmmu/dma-helpers.c | 16 +--------------- > softmmu/physmem.c | 19 +++++++++++++++++++ > 3 files changed, 36 insertions(+), 15 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space 2021-04-15 10:04 [PATCH v2 0/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé 2021-04-15 10:04 ` [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() Philippe Mathieu-Daudé @ 2021-04-15 10:04 ` Philippe Mathieu-Daudé 2021-04-15 11:02 ` Stefano Garzarella ` (3 more replies) 1 sibling, 4 replies; 12+ messages in thread From: Philippe Mathieu-Daudé @ 2021-04-15 10:04 UTC (permalink / raw) To: Stefano Garzarella, Laurent Vivier, qemu-devel Cc: Peter Maydell, Philippe Mathieu-Daudé, Paolo Bonzini From: Laurent Vivier <laurent@vivier.eu> 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> Message-Id: <20210414105838.205019-1-laurent@vivier.eu> [PMD: Use recently added address_space_set()] Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/hw/elf_ops.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h index 6ee458e7bc3..29f4c43e231 100644 --- a/include/hw/elf_ops.h +++ b/include/hw/elf_ops.h @@ -562,6 +562,19 @@ 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) { + res = address_space_set(as ? as : &address_space_memory, + addr + file_size, 0, + mem_size - file_size, + MEMTXATTRS_UNSPECIFIED); + if (res != MEMTX_OK) { + goto fail; + } + } } } -- 2.26.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space 2021-04-15 10:04 ` [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé @ 2021-04-15 11:02 ` Stefano Garzarella 2021-04-15 13:26 ` Laurent Vivier ` (2 subsequent siblings) 3 siblings, 0 replies; 12+ messages in thread From: Stefano Garzarella @ 2021-04-15 11:02 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Peter Maydell, Paolo Bonzini, Laurent Vivier, qemu-devel On Thu, Apr 15, 2021 at 12:04:09PM +0200, Philippe Mathieu-Daudé wrote: >From: Laurent Vivier <laurent@vivier.eu> > >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> >Message-Id: <20210414105838.205019-1-laurent@vivier.eu> >[PMD: Use recently added address_space_set()] >Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >--- > include/hw/elf_ops.h | 13 +++++++++++++ > 1 file changed, 13 insertions(+) Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > >diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h >index 6ee458e7bc3..29f4c43e231 100644 >--- a/include/hw/elf_ops.h >+++ b/include/hw/elf_ops.h >@@ -562,6 +562,19 @@ 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) { >+ res = address_space_set(as ? as : &address_space_memory, >+ addr + file_size, 0, >+ mem_size - file_size, >+ MEMTXATTRS_UNSPECIFIED); >+ if (res != MEMTX_OK) { >+ goto fail; >+ } >+ } > } > } > >-- >2.26.3 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space 2021-04-15 10:04 ` [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé 2021-04-15 11:02 ` Stefano Garzarella @ 2021-04-15 13:26 ` Laurent Vivier 2021-04-15 22:33 ` Richard Henderson 2021-04-17 15:36 ` Laurent Vivier 3 siblings, 0 replies; 12+ messages in thread From: Laurent Vivier @ 2021-04-15 13:26 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Stefano Garzarella, qemu-devel Cc: Peter Maydell, Paolo Bonzini Le 15/04/2021 à 12:04, Philippe Mathieu-Daudé a écrit : > From: Laurent Vivier <laurent@vivier.eu> > > 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> > Message-Id: <20210414105838.205019-1-laurent@vivier.eu> > [PMD: Use recently added address_space_set()] I agree that is a better way to do. thanks, Laurent > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > include/hw/elf_ops.h | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h > index 6ee458e7bc3..29f4c43e231 100644 > --- a/include/hw/elf_ops.h > +++ b/include/hw/elf_ops.h > @@ -562,6 +562,19 @@ 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) { > + res = address_space_set(as ? as : &address_space_memory, > + addr + file_size, 0, > + mem_size - file_size, > + MEMTXATTRS_UNSPECIFIED); > + if (res != MEMTX_OK) { > + goto fail; > + } > + } > } > } > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space 2021-04-15 10:04 ` [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé 2021-04-15 11:02 ` Stefano Garzarella 2021-04-15 13:26 ` Laurent Vivier @ 2021-04-15 22:33 ` Richard Henderson 2021-04-17 15:36 ` Laurent Vivier 3 siblings, 0 replies; 12+ messages in thread From: Richard Henderson @ 2021-04-15 22:33 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Stefano Garzarella, Laurent Vivier, qemu-devel Cc: Peter Maydell, Paolo Bonzini On 4/15/21 3:04 AM, Philippe Mathieu-Daudé wrote: > From: Laurent Vivier<laurent@vivier.eu> > > 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> > Message-Id:<20210414105838.205019-1-laurent@vivier.eu> > [PMD: Use recently added address_space_set()] > Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com> > --- > include/hw/elf_ops.h | 13 +++++++++++++ > 1 file changed, 13 insertions(+) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space 2021-04-15 10:04 ` [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé ` (2 preceding siblings ...) 2021-04-15 22:33 ` Richard Henderson @ 2021-04-17 15:36 ` Laurent Vivier 3 siblings, 0 replies; 12+ messages in thread From: Laurent Vivier @ 2021-04-17 15:36 UTC (permalink / raw) To: Philippe Mathieu-Daudé, Stefano Garzarella, qemu-devel Cc: Peter Maydell, Paolo Bonzini Le 15/04/2021 à 12:04, Philippe Mathieu-Daudé a écrit : > From: Laurent Vivier <laurent@vivier.eu> > > 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> > Message-Id: <20210414105838.205019-1-laurent@vivier.eu> > [PMD: Use recently added address_space_set()] > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > include/hw/elf_ops.h | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h > index 6ee458e7bc3..29f4c43e231 100644 > --- a/include/hw/elf_ops.h > +++ b/include/hw/elf_ops.h > @@ -562,6 +562,19 @@ 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) { > + res = address_space_set(as ? as : &address_space_memory, > + addr + file_size, 0, > + mem_size - file_size, > + MEMTXATTRS_UNSPECIFIED); > + if (res != MEMTX_OK) { > + goto fail; > + } > + } > } > } > > It seems we need also the same kind of operation with the other branch of the if (rom_add_elf_program()), but I'm not sure where to do it: 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 */ Thanks, Laurent ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-04-17 15:37 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-15 10:04 [PATCH v2 0/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé 2021-04-15 10:04 ` [PATCH v2 1/2] exec/memory: Extract address_space_set() from dma_memory_set() Philippe Mathieu-Daudé 2021-04-15 10:46 ` Laurent Vivier 2021-04-15 11:00 ` Stefano Garzarella 2021-04-15 11:05 ` Peter Maydell 2021-04-15 11:18 ` Stefano Garzarella 2021-04-15 22:32 ` Richard Henderson 2021-04-15 10:04 ` [PATCH v2 2/2] hw/elf_ops: clear uninitialized segment space Philippe Mathieu-Daudé 2021-04-15 11:02 ` Stefano Garzarella 2021-04-15 13:26 ` Laurent Vivier 2021-04-15 22:33 ` Richard Henderson 2021-04-17 15:36 ` Laurent Vivier
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).