The last attempt to fix linux-user to be able to run static armhf
binaries fixed armhf, but broke arm64, ppc64el and s390x.

This patch takes another approach to fix the issue.
Instead of pre-allocatig a 16/32 MiB memory region for heap, the loader
tracks the highest address of both executable and interpreter.

Then it compares both values and chooses the highest address from the
executable as brk starting point, if the starting address of the
interpreter leaves at least 16 MiB for the heap.

If not, qemu will choose the highest address of the interpreter
as brk start.

I've tested this patch on arm64 and hppa (both with chroot), and with a
static armhf binary.  It applies on top of git head, where the previous
patch was reverted.

Signed-off-by: Helge Deller <deller@gmx.de>

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 861ec07abc..710ba03862 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -3107,27 +3107,6 @@ static void load_elf_image(const char *image_name, int image_fd,
     }

     if (pinterp_name != NULL) {
-        /*
-         * This is the main executable.
-         *
-         * Reserve extra space for brk.
-         * We hold on to this space while placing the interpreter
-         * and the stack, lest they be placed immediately after
-         * the data segment and block allocation from the brk.
-         *
-         * 16MB is chosen as "large enough" without being so large as
-         * to allow the result to not fit with a 32-bit guest on a
-         * 32-bit host. However some 64 bit guests (e.g. s390x)
-         * attempt to place their heap further ahead and currently
-         * nothing stops them smashing into QEMUs address space.
-         */
-#if TARGET_LONG_BITS == 64
-        info->reserve_brk = 32 * MiB;
-#else
-        info->reserve_brk = 16 * MiB;
-#endif
-        hiaddr += info->reserve_brk;
-
         if (ehdr->e_type == ET_EXEC) {
             /*
              * Make sure that the low address does not conflict with
@@ -3194,7 +3173,8 @@ static void load_elf_image(const char *image_name, int image_fd,
     info->end_code = 0;
     info->start_data = -1;
     info->end_data = 0;
-    info->brk = 0;
+    /* put possible start for brk behind all sections of this ELF file. */
+    info->brk = load_bias + TARGET_PAGE_ALIGN(hiaddr);
     info->elf_flags = ehdr->e_flags;

     prot_exec = PROT_EXEC;
@@ -3288,9 +3268,6 @@ static void load_elf_image(const char *image_name, int image_fd,
                     info->end_data = vaddr_ef;
                 }
             }
-            if (vaddr_em > info->brk) {
-                info->brk = vaddr_em;
-            }
 #ifdef TARGET_MIPS
         } else if (eppnt->p_type == PT_MIPS_ABIFLAGS) {
             Mips_elf_abiflags_v0 abiflags;
@@ -3618,6 +3595,15 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info)

     if (elf_interpreter) {
         load_elf_interp(elf_interpreter, &interp_info, bprm->buf);
+        /*
+         * Use brk address of interpreter if it was loaded above the
+         * executable and leaves less than 16 MB for heap.
+         * This happens e.g. with static binaries on armhf.
+         */
+        if (interp_info.brk > info->brk &&
+            info->load_bias + interp_info.load_addr - info->brk < 16 * MiB)  {
+            info->brk = interp_info.brk;
+        }

         /* If the program interpreter is one of these two, then assume
            an iBCS2 image.  Otherwise assume a native linux image.  */
@@ -3672,17 +3658,6 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info)
     bprm->core_dump = &elf_core_dump;
 #endif

-    /*
-     * If we reserved extra space for brk, release it now.
-     * The implementation of do_brk in syscalls.c expects to be able
-     * to mmap pages in this space.
-     */
-    if (info->reserve_brk) {
-        abi_ulong start_brk = HOST_PAGE_ALIGN(info->brk);
-        abi_ulong end_brk = HOST_PAGE_ALIGN(info->brk + info->reserve_brk);
-        target_munmap(start_brk, end_brk - start_brk);
-    }
-
     return 0;
 }

24.07.2023 20:48, Helge Deller wrote:
> The last attempt to fix linux-user to be able to run static armhf
> binaries fixed armhf, but broke arm64, ppc64el and s390x.

Hello Delge!

I haven't looked at this version yet (was busy today).
Have you seen https://bugs.debian.org/1041859 ?
It is.. telling.
I'll try to reproduce some of them locally.

/mjt

On 7/24/23 19:48, Helge Deller wrote:
> The last attempt to fix linux-user to be able to run static armhf
> binaries fixed armhf, but broke arm64, ppc64el and s390x.
>
> This patch takes another approach to fix the issue.
> Instead of pre-allocatig a 16/32 MiB memory region for heap, the loader
> tracks the highest address of both executable and interpreter.
>
> Then it compares both values and chooses the highest address from the
> executable as brk starting point, if the starting address of the
> interpreter leaves at least 16 MiB for the heap.
>
> If not, qemu will choose the highest address of the interpreter
> as brk start.
>
> I've tested this patch on arm64 and hppa (both with chroot), and with a
> static armhf binary.  It applies on top of git head, where the previous
> patch was reverted.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 861ec07abc..710ba03862 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -3107,27 +3107,6 @@ static void load_elf_image(const char *image_name, int image_fd,
>       }
>
>       if (pinterp_name != NULL) {
> -        /*
> -         * This is the main executable.
> -         *
> -         * Reserve extra space for brk.
> -         * We hold on to this space while placing the interpreter
> -         * and the stack, lest they be placed immediately after
> -         * the data segment and block allocation from the brk.
> -         *
> -         * 16MB is chosen as "large enough" without being so large as
> -         * to allow the result to not fit with a 32-bit guest on a
> -         * 32-bit host. However some 64 bit guests (e.g. s390x)
> -         * attempt to place their heap further ahead and currently
> -         * nothing stops them smashing into QEMUs address space.
> -         */
> -#if TARGET_LONG_BITS == 64
> -        info->reserve_brk = 32 * MiB;
> -#else
> -        info->reserve_brk = 16 * MiB;
> -#endif
> -        hiaddr += info->reserve_brk;
> -
>           if (ehdr->e_type == ET_EXEC) {
>               /*
>                * Make sure that the low address does not conflict with
> @@ -3194,7 +3173,8 @@ static void load_elf_image(const char *image_name, int image_fd,
>       info->end_code = 0;
>       info->start_data = -1;
>       info->end_data = 0;
> -    info->brk = 0;
> +    /* put possible start for brk behind all sections of this ELF file. */
> +    info->brk = load_bias + TARGET_PAGE_ALIGN(hiaddr);
>       info->elf_flags = ehdr->e_flags;
>
>       prot_exec = PROT_EXEC;
> @@ -3288,9 +3268,6 @@ static void load_elf_image(const char *image_name, int image_fd,
>                       info->end_data = vaddr_ef;
>                   }
>               }
> -            if (vaddr_em > info->brk) {
> -                info->brk = vaddr_em;
> -            }
>   #ifdef TARGET_MIPS
>           } else if (eppnt->p_type == PT_MIPS_ABIFLAGS) {
>               Mips_elf_abiflags_v0 abiflags;
> @@ -3618,6 +3595,15 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info)
>
>       if (elf_interpreter) {
>           load_elf_interp(elf_interpreter, &interp_info, bprm->buf);
> +        /*
> +         * Use brk address of interpreter if it was loaded above the
> +         * executable and leaves less than 16 MB for heap.
> +         * This happens e.g. with static binaries on armhf.
> +         */
> +        if (interp_info.brk > info->brk &&
> +            info->load_bias + interp_info.load_addr - info->brk < 16 * MiB)  {

^^^
"info" is wrong here. should be interp_info.
I'll recheck and then send a v4.

Helge


> +            info->brk = interp_info.brk;
> +        }
>
>           /* If the program interpreter is one of these two, then assume
>              an iBCS2 image.  Otherwise assume a native linux image.  */
> @@ -3672,17 +3658,6 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info)
>       bprm->core_dump = &elf_core_dump;
>   #endif
>
> -    /*
> -     * If we reserved extra space for brk, release it now.
> -     * The implementation of do_brk in syscalls.c expects to be able
> -     * to mmap pages in this space.
> -     */
> -    if (info->reserve_brk) {
> -        abi_ulong start_brk = HOST_PAGE_ALIGN(info->brk);
> -        abi_ulong end_brk = HOST_PAGE_ALIGN(info->brk + info->reserve_brk);
> -        target_munmap(start_brk, end_brk - start_brk);
> -    }
> -
>       return 0;
>   }
>
>