[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] linux-user: remove MAX_ARG_PAGES limit
From: |
Stefan Bruens |
Subject: |
Re: [Qemu-devel] [PATCH] linux-user: remove MAX_ARG_PAGES limit |
Date: |
Thu, 27 Aug 2015 18:10:58 +0200 |
User-agent: |
KMail/4.14.9 (Linux/3.16.7-24-desktop; KDE/4.14.9; x86_64; ; ) |
On Sunday 23 August 2015 01:47:52 Stefan Brüns wrote:
> Instead of creating a temporary copy for the whole environment and
> the arguments, directly copy everything to the target stack.
>
> For this to work, we have to change the order of stack creation and
> copying the arguments.
>
> Signed-off-by: Stefan Brüns <address@hidden>
> ---
> linux-user/elfload.c | 105
> +++++++++++++++++++++++-------------------------- linux-user/linuxload.c |
> 7 +---
> linux-user/qemu.h | 7 ----
> linux-user/syscall.c | 6 ---
> 4 files changed, 51 insertions(+), 74 deletions(-)
Any comments are very appreciated.
Kind regards,
Stefan
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 1788368..62feaf7 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -1365,66 +1365,69 @@ static bool elf_check_ehdr(struct elfhdr *ehdr)
> * to be put directly into the top of new user memory.
> *
> */
> -static abi_ulong copy_elf_strings(int argc,char ** argv, void **page,
> - abi_ulong p)
> +static abi_ulong copy_elf_strings(int argc,char ** argv, char* scratch,
> + abi_ulong p, abi_ulong stack_limit)
> {
> - char *tmp, *tmp1, *pag = NULL;
> - int len, offset = 0;
> + char *tmp;
> + int len, offset;
> + abi_ulong top = p;
>
> if (!p) {
> return 0; /* bullet-proofing */
> }
> +
> + offset = ((p - 1) % TARGET_PAGE_SIZE) + 1;
> +
> while (argc-- > 0) {
> tmp = argv[argc];
> if (!tmp) {
> fprintf(stderr, "VFS: argc is wrong");
> exit(-1);
> }
> - tmp1 = tmp;
> - while (*tmp++);
> - len = tmp - tmp1;
> - if (p < len) { /* this shouldn't happen - 128kB */
> + len = strlen(tmp) + 1;
> + tmp += len;
> +
> + if (len > (p - stack_limit)) {
> return 0;
> }
> while (len) {
> - --p; --tmp; --len;
> - if (--offset < 0) {
> - offset = p % TARGET_PAGE_SIZE;
> - pag = (char *)page[p/TARGET_PAGE_SIZE];
> - if (!pag) {
> - pag = g_try_malloc0(TARGET_PAGE_SIZE);
> - page[p/TARGET_PAGE_SIZE] = pag;
> - if (!pag)
> - return 0;
> - }
> - }
> - if (len == 0 || offset == 0) {
> - *(pag + offset) = *tmp;
> + int bytes_to_copy = (len > offset) ? offset : len;
> + tmp -= bytes_to_copy;
> + p -= bytes_to_copy;
> + offset -= bytes_to_copy;
> + len -= bytes_to_copy;
> +
> + if (bytes_to_copy == 1) {
> + *(scratch + offset) = *tmp;
> + } else {
> + memcpy_fromfs(scratch + offset, tmp, bytes_to_copy);
> }
> - else {
> - int bytes_to_copy = (len > offset) ? offset : len;
> - tmp -= bytes_to_copy;
> - p -= bytes_to_copy;
> - offset -= bytes_to_copy;
> - len -= bytes_to_copy;
> - memcpy_fromfs(pag + offset, tmp, bytes_to_copy + 1);
> +
> + if (offset == 0) {
> + memcpy_to_target(p, scratch, top - p);
> + top = p;
> + offset = TARGET_PAGE_SIZE;
> }
> }
> }
> + if (offset)
> + memcpy_to_target(p, scratch + offset, top - p);
> +
> return p;
> }
>
> -static abi_ulong setup_arg_pages(abi_ulong p, struct linux_binprm *bprm,
> +static abi_ulong setup_arg_pages(struct linux_binprm *bprm,
> struct image_info *info)
> {
> - abi_ulong stack_base, size, error, guard;
> - int i;
> + abi_ulong size, error, guard;
> +
> + /* Linux kernel limits argument/environment space to 1/4th of stack
> size, + but also has a floor of 32 pages. Mimic this behaviour. */ +
> #define MAX_ARG_PAGES 32
>
> - /* Create enough stack to hold everything. If we don't use
> - it for args, we'll use it for something else. */
> size = guest_stack_size;
> - if (size < MAX_ARG_PAGES*TARGET_PAGE_SIZE) {
> - size = MAX_ARG_PAGES*TARGET_PAGE_SIZE;
> + if (size < MAX_ARG_PAGES * TARGET_PAGE_SIZE) {
> + size = MAX_ARG_PAGES * TARGET_PAGE_SIZE;
> }
> guard = TARGET_PAGE_SIZE;
> if (guard < qemu_real_host_page_size) {
> @@ -1442,19 +1445,8 @@ static abi_ulong setup_arg_pages(abi_ulong p, struct
> linux_binprm *bprm, target_mprotect(error, guard, PROT_NONE);
>
> info->stack_limit = error + guard;
> - stack_base = info->stack_limit + size - MAX_ARG_PAGES*TARGET_PAGE_SIZE;
> - p += stack_base;
> -
> - for (i = 0 ; i < MAX_ARG_PAGES ; i++) {
> - if (bprm->page[i]) {
> - info->rss++;
> - /* FIXME - check return value of memcpy_to_target() for failure
> */ - memcpy_to_target(stack_base, bprm->page[i],
> TARGET_PAGE_SIZE); - g_free(bprm->page[i]);
> - }
> - stack_base += TARGET_PAGE_SIZE;
> - }
> - return p;
> +
> + return info->stack_limit + size - sizeof(void*);
> }
>
> /* Map and zero the bss. We need to explicitly zero any fractional pages
> @@ -2198,6 +2190,7 @@ int load_elf_binary(struct linux_binprm *bprm, struct
> image_info *info) struct image_info interp_info;
> struct elfhdr elf_ex;
> char *elf_interpreter = NULL;
> + void **scratch;
>
> info->start_mmap = (abi_ulong)ELF_START_MMAP;
> info->mmap = 0;
> @@ -2211,17 +2204,19 @@ int load_elf_binary(struct linux_binprm *bprm,
> struct image_info *info) when we load the interpreter. */
> elf_ex = *(struct elfhdr *)bprm->buf;
>
> - bprm->p = copy_elf_strings(1, &bprm->filename, bprm->page, bprm->p);
> - bprm->p = copy_elf_strings(bprm->envc,bprm->envp,bprm->page,bprm->p);
> - bprm->p = copy_elf_strings(bprm->argc,bprm->argv,bprm->page,bprm->p);
> + /* Do this so that we can load the interpreter, if need be. We will
> + change some of these later */
> + bprm->p = setup_arg_pages(bprm, info);
> +
> + scratch = g_new0(void *, TARGET_PAGE_SIZE);
> + bprm->p = copy_elf_strings(1, &bprm->filename, scratch, bprm->p,
> info->stack_limit); + bprm->p = copy_elf_strings(bprm->envc, bprm->envp,
> scratch, bprm->p, info->stack_limit); + bprm->p =
> copy_elf_strings(bprm->argc, bprm->argv, scratch, bprm->p,
> info->stack_limit); if (!bprm->p) {
> fprintf(stderr, "%s: %s\n", bprm->filename, strerror(E2BIG));
> exit(-1);
> }
> -
> - /* Do this so that we can load the interpreter, if need be. We will
> - change some of these later */
> - bprm->p = setup_arg_pages(bprm->p, bprm, info);
> + g_free(scratch);
>
> if (elf_interpreter) {
> load_elf_interp(elf_interpreter, &interp_info, bprm->buf);
> diff --git a/linux-user/linuxload.c b/linux-user/linuxload.c
> index 506e837..09df934 100644
> --- a/linux-user/linuxload.c
> +++ b/linux-user/linuxload.c
> @@ -137,8 +137,7 @@ int loader_exec(int fdexec, const char *filename, char
> **argv, char **envp, int retval;
> int i;
>
> - bprm->p = TARGET_PAGE_SIZE*MAX_ARG_PAGES-sizeof(unsigned int);
> - memset(bprm->page, 0, sizeof(bprm->page));
> + bprm->p = 0;
> bprm->fd = fdexec;
> bprm->filename = (char *)filename;
> bprm->argc = count(argv);
> @@ -172,9 +171,5 @@ int loader_exec(int fdexec, const char *filename, char
> **argv, char **envp, return retval;
> }
>
> - /* Something went wrong, return the inode and free the argument pages*/
> - for (i=0 ; i<MAX_ARG_PAGES ; i++) {
> - g_free(bprm->page[i]);
> - }
> return(retval);
> }
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index 8012cc2..04c315b 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -145,12 +145,6 @@ extern const char *qemu_uname_release;
> extern unsigned long mmap_min_addr;
>
> /* ??? See if we can avoid exposing so much of the loader internals. */
> -/*
> - * MAX_ARG_PAGES defines the number of pages allocated for arguments
> - * and envelope for the new program. 32 should suffice, this gives
> - * a maximum env+arg of 128kB w/4KB pages!
> - */
> -#define MAX_ARG_PAGES 33
>
> /* Read a good amount of data initially, to hopefully get all the
> program headers loaded. */
> @@ -162,7 +156,6 @@ extern unsigned long mmap_min_addr;
> */
> struct linux_binprm {
> char buf[BPRM_BUF_SIZE] __attribute__((aligned));
> - void *page[MAX_ARG_PAGES];
> abi_ulong p;
> int fd;
> int e_uid, e_gid;
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index f62c698..7d4e60e 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5799,12 +5799,6 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
> arg1, }
> *q = NULL;
>
> - /* This case will not be caught by the host's execve() if its
> - page size is bigger than the target's. */
> - if (total_size > MAX_ARG_PAGES * TARGET_PAGE_SIZE) {
> - ret = -TARGET_E2BIG;
> - goto execve_end;
> - }
> if (!(p = lock_user_string(arg1)))
> goto execve_efault;
> ret = get_errno(execve(p, argp, envp));
--
Stefan Brüns / Bergstraße 21 / 52062 Aachen
home: +49 241 53809034 mobile: +49 151 50412019
work: +49 2405 49936-424
Re: [Qemu-devel] [PATCH] linux-user: remove MAX_ARG_PAGES, allow dynamic growth of env/argv space, Stefan Bruens, 2015/08/22