| Message ID | 20180316103408.22295-1-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user: Allocate extra space for brk in PIE executable | expand |
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
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~
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) {
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(-) -- 2.14.3