* [Qemu-devel] [PATCH v2] linux-user: elf: Map empty PT_LOAD segments
@ 2019-05-03 12:20 Giuseppe Musacchio
  2019-05-03 12:20 ` Giuseppe Musacchio
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Giuseppe Musacchio @ 2019-05-03 12:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: riku.voipio, laurent, peter.maydell
Some PT_LOAD segments 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>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/elfload.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index c1a2602..138735b 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2366,11 +2366,19 @@ 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 segments 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] 5+ messages in thread
* [Qemu-devel] [PATCH v2] linux-user: elf: Map empty PT_LOAD segments
  2019-05-03 12:20 [Qemu-devel] [PATCH v2] linux-user: elf: Map empty PT_LOAD segments Giuseppe Musacchio
@ 2019-05-03 12:20 ` Giuseppe Musacchio
  2019-05-09 15:50 ` Laurent Vivier
  2019-05-09 17:02 ` Laurent Vivier
  2 siblings, 0 replies; 5+ messages in thread
From: Giuseppe Musacchio @ 2019-05-03 12:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, riku.voipio, laurent
Some PT_LOAD segments 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>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/elfload.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index c1a2602..138735b 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2366,11 +2366,19 @@ 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 segments 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] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: elf: Map empty PT_LOAD segments
  2019-05-03 12:20 [Qemu-devel] [PATCH v2] linux-user: elf: Map empty PT_LOAD segments Giuseppe Musacchio
  2019-05-03 12:20 ` Giuseppe Musacchio
@ 2019-05-09 15:50 ` Laurent Vivier
  2019-05-09 15:57   ` Peter Maydell
  2019-05-09 17:02 ` Laurent Vivier
  2 siblings, 1 reply; 5+ messages in thread
From: Laurent Vivier @ 2019-05-09 15:50 UTC (permalink / raw)
  To: Giuseppe Musacchio, qemu-devel; +Cc: peter.maydell, riku.voipio
On 03/05/2019 14:20, Giuseppe Musacchio wrote:
> Some PT_LOAD segments 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>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
I don't find the mail from Peter with his R-b.
Did I miss it?
Thanks,
Laurent
> ---
>   linux-user/elfload.c | 18 +++++++++++++-----
>   1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index c1a2602..138735b 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2366,11 +2366,19 @@ 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 segments 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;
> 
^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: elf: Map empty PT_LOAD segments
  2019-05-09 15:50 ` Laurent Vivier
@ 2019-05-09 15:57   ` Peter Maydell
  0 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2019-05-09 15:57 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Giuseppe Musacchio, Riku Voipio, QEMU Developers
On Thu, 9 May 2019 at 16:50, Laurent Vivier <laurent@vivier.eu> wrote:
>
> On 03/05/2019 14:20, Giuseppe Musacchio wrote:
> > Some PT_LOAD segments 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>
> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> I don't find the mail from Peter with his R-b.
> Did I miss it?
Patchew has it:
https://patchew.org/QEMU/20190418153631.rl3lgadzrqqof72p@debian/
thanks
-- PMM
^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2] linux-user: elf: Map empty PT_LOAD segments
  2019-05-03 12:20 [Qemu-devel] [PATCH v2] linux-user: elf: Map empty PT_LOAD segments Giuseppe Musacchio
  2019-05-03 12:20 ` Giuseppe Musacchio
  2019-05-09 15:50 ` Laurent Vivier
@ 2019-05-09 17:02 ` Laurent Vivier
  2 siblings, 0 replies; 5+ messages in thread
From: Laurent Vivier @ 2019-05-09 17:02 UTC (permalink / raw)
  To: Giuseppe Musacchio, qemu-devel; +Cc: peter.maydell, riku.voipio
On 03/05/2019 14:20, Giuseppe Musacchio wrote:
> Some PT_LOAD segments 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>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   linux-user/elfload.c | 18 +++++++++++++-----
>   1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index c1a2602..138735b 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2366,11 +2366,19 @@ 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 segments 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;
> 
Applied to my linux-user branch.
Thanks,
Laurent
^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-05-09 17:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-03 12:20 [Qemu-devel] [PATCH v2] linux-user: elf: Map empty PT_LOAD segments Giuseppe Musacchio
2019-05-03 12:20 ` Giuseppe Musacchio
2019-05-09 15:50 ` Laurent Vivier
2019-05-09 15:57   ` Peter Maydell
2019-05-09 17:02 ` 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).