[PATCH] linux-user: Correctly start brk after executable

Timothy E Baldwin posted 1 patch 3 years, 9 months ago
Test docker-quick@centos7 failed
Test docker-mingw@fedora failed
Test checkpatch failed
Test FreeBSD failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200728224615.326675-1-T.E.Baldwin99@members.leeds.ac.uk
Maintainers: Laurent Vivier <laurent@vivier.eu>
linux-user/elfload.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
[PATCH] linux-user: Correctly start brk after executable
Posted by Timothy E Baldwin 3 years, 9 months ago
info->brk was erroneously set to the end of highest addressed
writable segment which could result it in overlapping the executable.

As per load_elf_binary in fs/binfmt_elf.c in Linux, it should be
set to end of highest addressed segment.

Signed-off-by: Timothy E Baldwin <T.E.Baldwin99@members.leeds.ac.uk>
---
 linux-user/elfload.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 7e7f642332..d5d444f698 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2564,9 +2564,9 @@ static void load_elf_image(const char *image_name, int image_fd,
                 if (vaddr_ef > info->end_data) {
                     info->end_data = vaddr_ef;
                 }
-                if (vaddr_em > info->brk) {
-                    info->brk = vaddr_em;
-                }
+            }
+            if (vaddr_em > info->brk) {
+                info->brk = vaddr_em;
             }
         } else if (eppnt->p_type == PT_INTERP && pinterp_name) {
             char *interp_name;
@@ -2621,7 +2621,6 @@ static void load_elf_image(const char *image_name, int image_fd,
     if (info->end_data == 0) {
         info->start_data = info->end_code;
         info->end_data = info->end_code;
-        info->brk = info->end_code;
     }
 
     if (qemu_log_enabled()) {
-- 
2.25.1

Given an executable with a read-write segment between 2 executable segments
qemu was unmapping most of the execuateble instead of area reserved for brk
at the end of the execuatable.

Re: [PATCH] linux-user: Correctly start brk after executable
Posted by Richard Henderson 3 years, 9 months ago
On 7/28/20 3:46 PM, Timothy E Baldwin wrote:
> info->brk was erroneously set to the end of highest addressed
> writable segment which could result it in overlapping the executable.
> 
> As per load_elf_binary in fs/binfmt_elf.c in Linux, it should be
> set to end of highest addressed segment.
> 
> Signed-off-by: Timothy E Baldwin <T.E.Baldwin99@members.leeds.ac.uk>
> ---
>  linux-user/elfload.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH] linux-user: Correctly start brk after executable
Posted by Laurent Vivier 3 years, 7 months ago
Le 29/07/2020 à 00:46, Timothy E Baldwin a écrit :
> info->brk was erroneously set to the end of highest addressed
> writable segment which could result it in overlapping the executable.
> 
> As per load_elf_binary in fs/binfmt_elf.c in Linux, it should be
> set to end of highest addressed segment.
> 
> Signed-off-by: Timothy E Baldwin <T.E.Baldwin99@members.leeds.ac.uk>
> ---
>  linux-user/elfload.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 7e7f642332..d5d444f698 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2564,9 +2564,9 @@ static void load_elf_image(const char *image_name, int image_fd,
>                  if (vaddr_ef > info->end_data) {
>                      info->end_data = vaddr_ef;
>                  }
> -                if (vaddr_em > info->brk) {
> -                    info->brk = vaddr_em;
> -                }
> +            }
> +            if (vaddr_em > info->brk) {
> +                info->brk = vaddr_em;
>              }
>          } else if (eppnt->p_type == PT_INTERP && pinterp_name) {
>              char *interp_name;
> @@ -2621,7 +2621,6 @@ static void load_elf_image(const char *image_name, int image_fd,
>      if (info->end_data == 0) {
>          info->start_data = info->end_code;
>          info->end_data = info->end_code;
> -        info->brk = info->end_code;
>      }
>  
>      if (qemu_log_enabled()) {
> 

Applied to my linux-user-for-5.2 branch.

Thanks,
Laurent