[Qemu-devel] [PATCH] linux-user: Allocate extra space for brk in PIE executable

[Qemu-devel] [PATCH] linux-user: Allocate extra space for brk in PIE executable

[Richard Henderson]

Limit this to 16M; there does not appear to be any special
support for this in the kernel itself, at least for i686.

Fixes: https://bugs.launchpad.net/bugs/1749393
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---

Commentary in the launchpad bug suggests 128M gap for x86_64, but that's
somewhat irrelevant to the given i686 test case.  There's certainly nothing
in the referenced kernel patch that does any more than we had been doing
without this patch.

I'm not sure what other limits on extra_size might we want to impose.
With -R set to something less than the full address space we could easily
wind up asking for more space than is available.


r~
---
 linux-user/elfload.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 9d10a5f592..e51d441fb9 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2055,7 +2055,15 @@ static void load_elf_image(const char *image_name, int image_fd,
            image is pre-linked, LOADDR will be non-zero.  Since we do
            not supply MAP_FIXED here we'll use that address if and
            only if it remains available.  */
-        load_addr = target_mmap(loaddr, hiaddr - loaddr, PROT_NONE,
+        abi_ulong total_size = hiaddr - loaddr;
+        if (pinterp_name != NULL) {
+            /* This is the main executable.
+             * Hack to reserve some extra space for brk.
+             */
+            abi_ulong extra_size = 16 * 1024 * 1024;
+            load_addr = mmap_find_vma(loaddr, total_size + extra_size);
+        }
+        load_addr = target_mmap(load_addr, total_size, PROT_NONE,
                                 MAP_PRIVATE | MAP_ANON | MAP_NORESERVE,
                                 -1, 0);
         if (load_addr == -1) {
-- 
2.14.3

Re: [Qemu-devel] [PATCH] linux-user: Allocate extra space for brk in PIE executable

[Peter Maydell]

On 16 March 2018 at 10:34, Richard Henderson
<richard.henderson@linaro.org> wrote:
> Limit this to 16M; there does not appear to be any special
> support for this in the kernel itself, at least for i686.
>
> Fixes: https://bugs.launchpad.net/bugs/1749393
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>
> Commentary in the launchpad bug suggests 128M gap for x86_64, but that's
> somewhat irrelevant to the given i686 test case.  There's certainly nothing
> in the referenced kernel patch that does any more than we had been doing
> without this patch.

I think the 128MB is enforced by mmap_base() in arch/x86/mm/mmap.c:
since x86-64 sots HAVE_ARCH_UNMAPPED_AREA_TOPDOWN, mmap_base is the
highest address in memory where mmap is permitted, and mmap_base()
enforces that it goes at least 128MB below the bottom of the stack
(accounting for rlimit stack size requirements also). Since
binfmt_elf() loads ELF segments via mmap this means that they won't
go too close to the stack. (The commit a87938b2e246 ensures the
gap is honoured by using the full binary size when it does the first
mapping so that mmap picks an address that is sufficiently before the
end of the mmap region for everything to fit.)
The kernel also uses ELF_ET_DYN_BASE to ensure that PIE programs
themselves get loaded clear of the ELF interpreter, which we
don't have any equivalent of (so you can see that different values
of -R result in either the interpreter or the executable getting
loaded at lower addresses.)

PS: do you know what the intention of the
        if (reserved_va) {
            mmap_next_start = reserved_va;
        }
code in linux-user/main.c is? It seems a bit odd to say "ok,
we have reserved a big region. we will start trying to mmap
outside it.", especially when that region covers the full
4G that the guest can access...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] linux-user: Allocate extra space for brk in PIE executable

[Richard Henderson]

On 03/16/2018 07:01 PM, Peter Maydell wrote:
> PS: do you know what the intention of the
>         if (reserved_va) {
>             mmap_next_start = reserved_va;
>         }
> code in linux-user/main.c is? It seems a bit odd to say "ok,
> we have reserved a big region. we will start trying to mmap
> outside it.", especially when that region covers the full
> 4G that the guest can access...

My guess is that it probably should have been

  mmap_next_start = MIN(TASK_UNMAPPED_BASE, reserved_va);

I can't think of any other reason it should be modified at all.


r~