From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44854) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUzm4-0001Cx-Sn for qemu-devel@nongnu.org; Thu, 27 Aug 2015 12:11:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZUzly-0000hz-ID for qemu-devel@nongnu.org; Thu, 27 Aug 2015 12:11:36 -0400 Received: from mx-out-1.rwth-aachen.de ([134.130.5.186]:29565) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUzly-0000h6-4h for qemu-devel@nongnu.org; Thu, 27 Aug 2015 12:11:30 -0400 From: Stefan Bruens Date: Thu, 27 Aug 2015 18:10:58 +0200 Message-ID: <1715244.NzKheCHJ5E@pebbles.site> In-Reply-To: <74ffe4f3-c653-4e67-a8d0-2bf9cae1c53c@HUB1.rwth-ad.de> References: <74ffe4f3-c653-4e67-a8d0-2bf9cae1c53c@HUB1.rwth-ad.de> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="iso-8859-1" Subject: Re: [Qemu-devel] [PATCH] linux-user: remove MAX_ARG_PAGES limit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On Sunday 23 August 2015 01:47:52 Stefan Br=FCns wrote: > Instead of creating a temporary copy for the whole environment and > the arguments, directly copy everything to the target stack. >=20 > For this to work, we have to change the order of stack creation and > copying the arguments. >=20 > Signed-off-by: Stefan Br=FCns > --- > linux-user/elfload.c | 105 > +++++++++++++++++++++++-------------------------- linux-user/linuxloa= d.c |=20 > 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 *ehd= r) > * 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* scrat= ch, > + abi_ulong p, abi_ulong stack_limit= ) > { > - char *tmp, *tmp1, *pag =3D NULL; > - int len, offset =3D 0; > + char *tmp; > + int len, offset; > + abi_ulong top =3D p; >=20 > if (!p) { > return 0; /* bullet-proofing */ > } > + > + offset =3D ((p - 1) % TARGET_PAGE_SIZE) + 1; > + > while (argc-- > 0) { > tmp =3D argv[argc]; > if (!tmp) { > fprintf(stderr, "VFS: argc is wrong"); > exit(-1); > } > - tmp1 =3D tmp; > - while (*tmp++); > - len =3D tmp - tmp1; > - if (p < len) { /* this shouldn't happen - 128kB */ > + len =3D strlen(tmp) + 1; > + tmp +=3D len; > + > + if (len > (p - stack_limit)) { > return 0; > } > while (len) { > - --p; --tmp; --len; > - if (--offset < 0) { > - offset =3D p % TARGET_PAGE_SIZE; > - pag =3D (char *)page[p/TARGET_PAGE_SIZE]; > - if (!pag) { > - pag =3D g_try_malloc0(TARGET_PAGE_SIZE); > - page[p/TARGET_PAGE_SIZE] =3D pag; > - if (!pag) > - return 0; > - } > - } > - if (len =3D=3D 0 || offset =3D=3D 0) { > - *(pag + offset) =3D *tmp; > + int bytes_to_copy =3D (len > offset) ? offset : len; > + tmp -=3D bytes_to_copy; > + p -=3D bytes_to_copy; > + offset -=3D bytes_to_copy; > + len -=3D bytes_to_copy; > + > + if (bytes_to_copy =3D=3D 1) { > + *(scratch + offset) =3D *tmp; > + } else { > + memcpy_fromfs(scratch + offset, tmp, bytes_to_copy);= > } > - else { > - int bytes_to_copy =3D (len > offset) ? offset : len;= > - tmp -=3D bytes_to_copy; > - p -=3D bytes_to_copy; > - offset -=3D bytes_to_copy; > - len -=3D bytes_to_copy; > - memcpy_fromfs(pag + offset, tmp, bytes_to_copy + 1);= > + > + if (offset =3D=3D 0) { > + memcpy_to_target(p, scratch, top - p); > + top =3D p; > + offset =3D TARGET_PAGE_SIZE; > } > } > } > + if (offset) > + memcpy_to_target(p, scratch + offset, top - p); > + > return p; > } >=20 > -static abi_ulong setup_arg_pages(abi_ulong p, struct linux_binprm *b= prm, > +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 st= ack > size, + but also has a floor of 32 pages. Mimic this behaviour.= */ + > #define MAX_ARG_PAGES 32 >=20 > - /* Create enough stack to hold everything. If we don't use > - it for args, we'll use it for something else. */ > size =3D guest_stack_size; > - if (size < MAX_ARG_PAGES*TARGET_PAGE_SIZE) { > - size =3D MAX_ARG_PAGES*TARGET_PAGE_SIZE; > + if (size < MAX_ARG_PAGES * TARGET_PAGE_SIZE) { > + size =3D MAX_ARG_PAGES * TARGET_PAGE_SIZE; > } > guard =3D 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); >=20 > info->stack_limit =3D error + guard; > - stack_base =3D info->stack_limit + size - MAX_ARG_PAGES*TARGET_P= AGE_SIZE; > - p +=3D stack_base; > - > - for (i =3D 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 +=3D TARGET_PAGE_SIZE; > - } > - return p; > + > + return info->stack_limit + size - sizeof(void*); > } >=20 > /* 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 =3D NULL; > + void **scratch; >=20 > info->start_mmap =3D (abi_ulong)ELF_START_MMAP; > info->mmap =3D 0; > @@ -2211,17 +2204,19 @@ int load_elf_binary(struct linux_binprm *bprm= , > struct image_info *info) when we load the interpreter. */ > elf_ex =3D *(struct elfhdr *)bprm->buf; >=20 > - bprm->p =3D copy_elf_strings(1, &bprm->filename, bprm->page, bpr= m->p); > - bprm->p =3D copy_elf_strings(bprm->envc,bprm->envp,bprm->page,bp= rm->p); > - bprm->p =3D copy_elf_strings(bprm->argc,bprm->argv,bprm->page,bp= rm->p); > + /* Do this so that we can load the interpreter, if need be. We = will > + change some of these later */ > + bprm->p =3D setup_arg_pages(bprm, info); > + > + scratch =3D g_new0(void *, TARGET_PAGE_SIZE); > + bprm->p =3D copy_elf_strings(1, &bprm->filename, scratch, bprm->= p, > info->stack_limit); + bprm->p =3D copy_elf_strings(bprm->envc, bpr= m->envp, > scratch, bprm->p, info->stack_limit); + bprm->p =3D > 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 =3D setup_arg_pages(bprm->p, bprm, info); > + g_free(scratch); >=20 > 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; >=20 > - bprm->p =3D TARGET_PAGE_SIZE*MAX_ARG_PAGES-sizeof(unsigned int);= > - memset(bprm->page, 0, sizeof(bprm->page)); > + bprm->p =3D 0; > bprm->fd =3D fdexec; > bprm->filename =3D (char *)filename; > bprm->argc =3D count(argv); > @@ -172,9 +171,5 @@ int loader_exec(int fdexec, const char *filename,= char > **argv, char **envp, return retval; > } >=20 > - /* Something went wrong, return the inode and free the argument = pages*/ > - for (i=3D0 ; 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; >=20 > /* ??? 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 >=20 > /* 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; > =09int 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, ab= i_long > arg1, } > *q =3D NULL; >=20 > - /* This case will not be caught by the host's execve() i= f its > - page size is bigger than the target's. */ > - if (total_size > MAX_ARG_PAGES * TARGET_PAGE_SIZE) { > - ret =3D -TARGET_E2BIG; > - goto execve_end; > - } > if (!(p =3D lock_user_string(arg1))) > goto execve_efault; > ret =3D get_errno(execve(p, argp, envp)); --=20 Stefan Br=FCns / Bergstra=DFe 21 / 52062 Aachen home: +49 241 53809034 mobile: +49 151 50412019 work: +49 2405 49936-424