* [Qemu-devel] [PATCH] linux-user: elf: Map empty PT_LOAD sections
@ 2019-04-18 15:42 Giuseppe Musacchio
2019-05-03 10:29 ` Giuseppe Musacchio
2019-05-03 11:48 ` Peter Maydell
0 siblings, 2 replies; 3+ messages in thread
From: Giuseppe Musacchio @ 2019-04-18 15:42 UTC (permalink / raw)
To: Riku Voipio; +Cc: Laurent Vivier, qemu-devel
Some PT_LOAD sections may be completely zeroed out and their p_filesize
is zero, in that case the loader should just allocate a page that's at
least p_memsz bytes large (plus eventual alignment padding).
Calling zero_bss does this job for us, all we have to do is make sure we
don't try to mmap a zero-length page.
Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
---
linux-user/elfload.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index c1a2602..e9a0951 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2366,11 +2366,17 @@ static void load_elf_image(const char *image_name, int image_fd,
vaddr_ps = TARGET_ELF_PAGESTART(vaddr);
vaddr_len = TARGET_ELF_PAGELENGTH(eppnt->p_filesz + vaddr_po);
- error = target_mmap(vaddr_ps, vaddr_len,
- elf_prot, MAP_PRIVATE | MAP_FIXED,
- image_fd, eppnt->p_offset - vaddr_po);
- if (error == -1) {
- goto exit_perror;
+ /* Some sections may be completely empty without any backing file
+ * segment, in that case just let zero_bss allocate an empty buffer
+ * for it. */
+ if (eppnt->p_filesz != 0) {
+ error = target_mmap(vaddr_ps, vaddr_len, elf_prot,
+ MAP_PRIVATE | MAP_FIXED,
+ image_fd, eppnt->p_offset - vaddr_po);
+
+ if (error == -1) {
+ goto exit_perror;
+ }
}
vaddr_ef = vaddr + eppnt->p_filesz;
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: elf: Map empty PT_LOAD sections
2019-04-18 15:42 [Qemu-devel] [PATCH] linux-user: elf: Map empty PT_LOAD sections Giuseppe Musacchio
@ 2019-05-03 10:29 ` Giuseppe Musacchio
2019-05-03 11:48 ` Peter Maydell
1 sibling, 0 replies; 3+ messages in thread
From: Giuseppe Musacchio @ 2019-05-03 10:29 UTC (permalink / raw)
To: Riku Voipio; +Cc: Laurent Vivier, qemu-devel
Friendly inactivity ping.
On Thu, 18 Apr 2019 at 17:42, Giuseppe Musacchio <thatlemon@gmail.com> wrote:
>
> Some PT_LOAD sections may be completely zeroed out and their p_filesize
> is zero, in that case the loader should just allocate a page that's at
> least p_memsz bytes large (plus eventual alignment padding).
>
> Calling zero_bss does this job for us, all we have to do is make sure we
> don't try to mmap a zero-length page.
>
> Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
> ---
> linux-user/elfload.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index c1a2602..e9a0951 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2366,11 +2366,17 @@ static void load_elf_image(const char *image_name, int image_fd,
> vaddr_ps = TARGET_ELF_PAGESTART(vaddr);
> vaddr_len = TARGET_ELF_PAGELENGTH(eppnt->p_filesz + vaddr_po);
>
> - error = target_mmap(vaddr_ps, vaddr_len,
> - elf_prot, MAP_PRIVATE | MAP_FIXED,
> - image_fd, eppnt->p_offset - vaddr_po);
> - if (error == -1) {
> - goto exit_perror;
> + /* Some sections may be completely empty without any backing file
> + * segment, in that case just let zero_bss allocate an empty buffer
> + * for it. */
> + if (eppnt->p_filesz != 0) {
> + error = target_mmap(vaddr_ps, vaddr_len, elf_prot,
> + MAP_PRIVATE | MAP_FIXED,
> + image_fd, eppnt->p_offset - vaddr_po);
> +
> + if (error == -1) {
> + goto exit_perror;
> + }
> }
>
> vaddr_ef = vaddr + eppnt->p_filesz;
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] linux-user: elf: Map empty PT_LOAD sections
2019-04-18 15:42 [Qemu-devel] [PATCH] linux-user: elf: Map empty PT_LOAD sections Giuseppe Musacchio
2019-05-03 10:29 ` Giuseppe Musacchio
@ 2019-05-03 11:48 ` Peter Maydell
1 sibling, 0 replies; 3+ messages in thread
From: Peter Maydell @ 2019-05-03 11:48 UTC (permalink / raw)
To: Giuseppe Musacchio; +Cc: Riku Voipio, Laurent Vivier, QEMU Developers
On Thu, 18 Apr 2019 at 17:46, Giuseppe Musacchio <thatlemon@gmail.com> wrote:
>
> Some PT_LOAD sections may be completely zeroed out and their p_filesize
> is zero, in that case the loader should just allocate a page that's at
> least p_memsz bytes large (plus eventual alignment padding).
Thanks for this patch -- codewise it looks good, so I just have
a couple of minor tweaks I think we should make to the comment
and commit message:
The things we're iterating through here are segments, not
sections (an ELF section is a different sort of data structure):
could you fix the text in your commit message and comment, please?
> Calling zero_bss does this job for us, all we have to do is make sure we
> don't try to mmap a zero-length page.
>
> Signed-off-by: Giuseppe Musacchio <thatlemon@gmail.com>
> ---
> linux-user/elfload.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index c1a2602..e9a0951 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2366,11 +2366,17 @@ static void load_elf_image(const char *image_name, int image_fd,
> vaddr_ps = TARGET_ELF_PAGESTART(vaddr);
> vaddr_len = TARGET_ELF_PAGELENGTH(eppnt->p_filesz + vaddr_po);
>
> - error = target_mmap(vaddr_ps, vaddr_len,
> - elf_prot, MAP_PRIVATE | MAP_FIXED,
> - image_fd, eppnt->p_offset - vaddr_po);
> - if (error == -1) {
> - goto exit_perror;
> + /* Some sections may be completely empty without any backing file
> + * segment, in that case just let zero_bss allocate an empty buffer
> + * for it. */
Our coding style says that multi-line comments should be
/*
* like this, with the start and
* end markers on lines of their own
*/
(if you run your patch through scripts/checkpatch.pl it ought
to detect this sort of minor coding style nit).
> + if (eppnt->p_filesz != 0) {
> + error = target_mmap(vaddr_ps, vaddr_len, elf_prot,
> + MAP_PRIVATE | MAP_FIXED,
> + image_fd, eppnt->p_offset - vaddr_po);
> +
> + if (error == -1) {
> + goto exit_perror;
> + }
> }
>
> vaddr_ef = vaddr + eppnt->p_filesz;
Other than those minor things:
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
so if you fix them and send out a v2 patch you can include
my Reviewed-by: tag in the commit message (under your
signed-off-by) in that patch email.
thanks
-- PMM
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-05-03 11:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-18 15:42 [Qemu-devel] [PATCH] linux-user: elf: Map empty PT_LOAD sections Giuseppe Musacchio
2019-05-03 10:29 ` Giuseppe Musacchio
2019-05-03 11:48 ` Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.