Author: jhb
Date: Thu Apr 19 16:00:34 2018
New Revision: 332782
URL: https://svnweb.freebsd.org/changeset/base/332782

Log:
  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

Modified:
  head/sys/amd64/linux/linux_sysvec.c
  head/sys/amd64/linux32/linux32_sysvec.c
  head/sys/compat/freebsd32/freebsd32_misc.c
  head/sys/i386/linux/linux_sysvec.c
  head/sys/kern/kern_exec.c
  head/sys/sys/imgact.h

Modified: head/sys/amd64/linux/linux_sysvec.c
==============================================================================
--- head/sys/amd64/linux/linux_sysvec.c Thu Apr 19 15:52:45 2018        
(r332781)
+++ head/sys/amd64/linux/linux_sysvec.c Thu Apr 19 16:00:34 2018        
(r332782)
@@ -331,31 +331,21 @@ 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;

Modified: head/sys/amd64/linux32/linux32_sysvec.c
==============================================================================
--- head/sys/amd64/linux32/linux32_sysvec.c     Thu Apr 19 15:52:45 2018        
(r332781)
+++ head/sys/amd64/linux32/linux32_sysvec.c     Thu Apr 19 16:00:34 2018        
(r332782)
@@ -793,31 +793,21 @@ 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;

Modified: head/sys/compat/freebsd32/freebsd32_misc.c
==============================================================================
--- head/sys/compat/freebsd32/freebsd32_misc.c  Thu Apr 19 15:52:45 2018        
(r332781)
+++ head/sys/compat/freebsd32/freebsd32_misc.c  Thu Apr 19 16:00:34 2018        
(r332782)
@@ -3180,33 +3180,21 @@ 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

Modified: head/sys/i386/linux/linux_sysvec.c
==============================================================================
--- head/sys/i386/linux/linux_sysvec.c  Thu Apr 19 15:52:45 2018        
(r332781)
+++ head/sys/i386/linux/linux_sysvec.c  Thu Apr 19 16:00:34 2018        
(r332782)
@@ -309,29 +309,21 @@ 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;

Modified: head/sys/kern/kern_exec.c
==============================================================================
--- head/sys/kern/kern_exec.c   Thu Apr 19 15:52:45 2018        (r332781)
+++ head/sys/kern/kern_exec.c   Thu Apr 19 16:00:34 2018        (r332782)
@@ -1537,33 +1537,21 @@ 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

Modified: head/sys/sys/imgact.h
==============================================================================
--- head/sys/sys/imgact.h       Thu Apr 19 15:52:45 2018        (r332781)
+++ head/sys/sys/imgact.h       Thu Apr 19 16:00:34 2018        (r332782)
@@ -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;
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "[email protected]"

Reply via email to