From 73c8686e91350bb6a16ed918886fade28735a1ed Mon Sep 17 00:00:00 2001 From: John Baldwin Date: Thu, 19 Apr 2018 16:00:34 +0000 Subject: [PATCH] Simplify the code to allocate stack for auxv, argv[], and environment vectors. Remove auxarg_size as it was only used once right after a confusing assignment in each of the variants of exec_copyout_strings(). Reviewed by: emaste MFC after: 1 month Differential Revision: https://reviews.freebsd.org/D15123 --- sys/amd64/linux/linux_sysvec.c | 32 +++++++++---------------- sys/amd64/linux32/linux32_sysvec.c | 32 +++++++++---------------- sys/compat/freebsd32/freebsd32_misc.c | 34 +++++++++------------------ sys/i386/linux/linux_sysvec.c | 30 +++++++++-------------- sys/kern/kern_exec.c | 34 +++++++++------------------ sys/sys/imgact.h | 1 - 6 files changed, 55 insertions(+), 108 deletions(-) diff --git a/sys/amd64/linux/linux_sysvec.c b/sys/amd64/linux/linux_sysvec.c index 16a01b6e79e4..649a4323eafb 100644 --- a/sys/amd64/linux/linux_sysvec.c +++ b/sys/amd64/linux/linux_sysvec.c @@ -331,32 +331,22 @@ linux_copyout_strings(struct image_params *imgp) roundup(sizeof(canary), sizeof(char *)); copyout(canary, (void *)imgp->canary, sizeof(canary)); - /* If we have a valid auxargs ptr, prepare some room on the stack. */ + vectp = (char **)destp; if (imgp->auxargs) { /* - * 'AT_COUNT*2' is size for the ELF Auxargs data. This is for - * lower compatibility. + * Allocate room on the stack for the ELF auxargs + * array. It has LINUX_AT_COUNT entries. */ - imgp->auxarg_size = (imgp->auxarg_size) ? imgp->auxarg_size : - (LINUX_AT_COUNT * 2); - - /* - * The '+ 2' is for the null pointers at the end of each of - * the arg and env vector sets,and imgp->auxarg_size is room - * for argument of Runtime loader. - */ - vectp = (char **)(destp - (imgp->args->argc + - imgp->args->envc + 2 + imgp->auxarg_size) * sizeof(char *)); - - } else { - /* - * The '+ 2' is for the null pointers at the end of each of - * the arg and env vector sets - */ - vectp = (char **)(destp - (imgp->args->argc + - imgp->args->envc + 2) * sizeof(char *)); + vectp -= howmany(LINUX_AT_COUNT * sizeof(Elf64_Auxinfo), + sizeof(*vectp)); } + /* + * Allocate room for the argv[] and env vectors including the + * terminating NULL pointers. + */ + vectp -= imgp->args->argc + 1 + imgp->args->envc + 1; + /* vectp also becomes our initial stack base. */ stack_base = (register_t *)vectp; diff --git a/sys/amd64/linux32/linux32_sysvec.c b/sys/amd64/linux32/linux32_sysvec.c index 4741cf77bc79..c8d922a207f6 100644 --- a/sys/amd64/linux32/linux32_sysvec.c +++ b/sys/amd64/linux32/linux32_sysvec.c @@ -793,32 +793,22 @@ linux_copyout_strings(struct image_params *imgp) roundup(sizeof(canary), sizeof(char *)); copyout(canary, (void *)imgp->canary, sizeof(canary)); - /* If we have a valid auxargs ptr, prepare some room on the stack. */ + vectp = (uint32_t *)destp; if (imgp->auxargs) { /* - * 'AT_COUNT*2' is size for the ELF Auxargs data. This is for - * lower compatibility. + * Allocate room on the stack for the ELF auxargs + * array. It has LINUX_AT_COUNT entries. */ - imgp->auxarg_size = (imgp->auxarg_size) ? imgp->auxarg_size : - (LINUX_AT_COUNT * 2); - /* - * The '+ 2' is for the null pointers at the end of each of - * the arg and env vector sets,and imgp->auxarg_size is room - * for argument of Runtime loader. - */ - vectp = (u_int32_t *) (destp - (imgp->args->argc + - imgp->args->envc + 2 + imgp->auxarg_size) * - sizeof(u_int32_t)); - - } else { - /* - * The '+ 2' is for the null pointers at the end of each of - * the arg and env vector sets - */ - vectp = (u_int32_t *)(destp - (imgp->args->argc + - imgp->args->envc + 2) * sizeof(u_int32_t)); + vectp -= howmany(LINUX_AT_COUNT * sizeof(Elf32_Auxinfo), + sizeof(*vectp)); } + /* + * Allocate room for the argv[] and env vectors including the + * terminating NULL pointers. + */ + vectp -= imgp->args->argc + 1 + imgp->args->envc + 1; + /* vectp also becomes our initial stack base. */ stack_base = vectp; diff --git a/sys/compat/freebsd32/freebsd32_misc.c b/sys/compat/freebsd32/freebsd32_misc.c index 5cda21be68f8..35b5164d6be9 100644 --- a/sys/compat/freebsd32/freebsd32_misc.c +++ b/sys/compat/freebsd32/freebsd32_misc.c @@ -3180,34 +3180,22 @@ freebsd32_copyout_strings(struct image_params *imgp) destp -= ARG_MAX - imgp->args->stringspace; destp = rounddown2(destp, sizeof(uint32_t)); - /* - * If we have a valid auxargs ptr, prepare some room - * on the stack. - */ + vectp = (uint32_t *)destp; if (imgp->auxargs) { /* - * 'AT_COUNT*2' is size for the ELF Auxargs data. This is for - * lower compatibility. + * Allocate room on the stack for the ELF auxargs + * array. It has up to AT_COUNT entries. */ - imgp->auxarg_size = (imgp->auxarg_size) ? imgp->auxarg_size - : (AT_COUNT * 2); - /* - * The '+ 2' is for the null pointers at the end of each of - * the arg and env vector sets,and imgp->auxarg_size is room - * for argument of Runtime loader. - */ - vectp = (u_int32_t *) (destp - (imgp->args->argc + - imgp->args->envc + 2 + imgp->auxarg_size + execpath_len) * - sizeof(u_int32_t)); - } else { - /* - * The '+ 2' is for the null pointers at the end of each of - * the arg and env vector sets - */ - vectp = (u_int32_t *)(destp - (imgp->args->argc + - imgp->args->envc + 2) * sizeof(u_int32_t)); + vectp -= howmany(AT_COUNT * sizeof(Elf32_Auxinfo), + sizeof(*vectp)); } + /* + * Allocate room for the argv[] and env vectors including the + * terminating NULL pointers. + */ + vectp -= imgp->args->argc + 1 + imgp->args->envc + 1; + /* * vectp also becomes our initial stack base */ diff --git a/sys/i386/linux/linux_sysvec.c b/sys/i386/linux/linux_sysvec.c index ecf9e2c6314b..43625a122f68 100644 --- a/sys/i386/linux/linux_sysvec.c +++ b/sys/i386/linux/linux_sysvec.c @@ -309,30 +309,22 @@ linux_copyout_strings(struct image_params *imgp) roundup(sizeof(canary), sizeof(char *)); copyout(canary, (void *)imgp->canary, sizeof(canary)); - /* If we have a valid auxargs ptr, prepare some room on the stack. */ + vectp = (char **)destp; if (imgp->auxargs) { /* - * 'AT_COUNT*2' is size for the ELF Auxargs data. This is for - * lower compatibility. + * Allocate room on the stack for the ELF auxargs + * array. It has LINUX_AT_COUNT entries. */ - imgp->auxarg_size = (imgp->auxarg_size) ? imgp->auxarg_size : - (LINUX_AT_COUNT * 2); - /* - * The '+ 2' is for the null pointers at the end of each of - * the arg and env vector sets,and imgp->auxarg_size is room - * for argument of Runtime loader. - */ - vectp = (char **)(destp - (imgp->args->argc + - imgp->args->envc + 2 + imgp->auxarg_size) * sizeof(char *)); - } else { - /* - * The '+ 2' is for the null pointers at the end of each of - * the arg and env vector sets - */ - vectp = (char **)(destp - (imgp->args->argc + imgp->args->envc + 2) * - sizeof(char *)); + vectp -= howmany(LINUX_AT_COUNT * sizeof(Elf32_Auxinfo), + sizeof(*vectp)); } + /* + * Allocate room for the argv[] and env vectors including the + * terminating NULL pointers. + */ + vectp -= imgp->args->argc + 1 + imgp->args->envc + 1; + /* vectp also becomes our initial stack base. */ stack_base = (register_t *)vectp; diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c index c55851f5b420..29eae3967a68 100644 --- a/sys/kern/kern_exec.c +++ b/sys/kern/kern_exec.c @@ -1537,34 +1537,22 @@ exec_copyout_strings(struct image_params *imgp) destp -= ARG_MAX - imgp->args->stringspace; destp = rounddown2(destp, sizeof(void *)); - /* - * If we have a valid auxargs ptr, prepare some room - * on the stack. - */ + vectp = (char **)destp; if (imgp->auxargs) { /* - * 'AT_COUNT*2' is size for the ELF Auxargs data. This is for - * lower compatibility. + * Allocate room on the stack for the ELF auxargs + * array. It has up to AT_COUNT entries. */ - imgp->auxarg_size = (imgp->auxarg_size) ? imgp->auxarg_size : - (AT_COUNT * 2); - /* - * The '+ 2' is for the null pointers at the end of each of - * the arg and env vector sets,and imgp->auxarg_size is room - * for argument of Runtime loader. - */ - vectp = (char **)(destp - (imgp->args->argc + - imgp->args->envc + 2 + imgp->auxarg_size) - * sizeof(char *)); - } else { - /* - * The '+ 2' is for the null pointers at the end of each of - * the arg and env vector sets - */ - vectp = (char **)(destp - (imgp->args->argc + imgp->args->envc - + 2) * sizeof(char *)); + vectp -= howmany(AT_COUNT * sizeof(Elf_Auxinfo), + sizeof(*vectp)); } + /* + * Allocate room for the argv[] and env vectors including the + * terminating NULL pointers. + */ + vectp -= imgp->args->argc + 1 + imgp->args->envc + 1; + /* * vectp also becomes our initial stack base */ diff --git a/sys/sys/imgact.h b/sys/sys/imgact.h index 309ba0ee5663..d3355ce42169 100644 --- a/sys/sys/imgact.h +++ b/sys/sys/imgact.h @@ -75,7 +75,6 @@ struct image_params { void *auxargs; /* ELF Auxinfo structure pointer */ struct sf_buf *firstpage; /* first page that we mapped */ unsigned long ps_strings; /* PS_STRINGS for BSD/OS binaries */ - size_t auxarg_size; struct image_args *args; /* system call arguments */ struct sysentvec *sysent; /* system entry vector */ char *execpath;