> From: Maxim Pugachev <[email protected]> > Date: Tue, 5 Jan 2016 14:28:48 +0300 > > Ping?
Sorry, but I don't consider this enough of an improvement to spend time on analyzing it further and testing it. > On Wed, Dec 16, 2015 at 9:55 PM, Maxim Pugachev <[email protected]> wrote: > > Ping? > > > > On Sat, Dec 12, 2015 at 8:38 PM, Maxim Pugachev <[email protected]> > > wrote: > >> Hi, > >> > >> This patch removes copypasted code that prepares args and env in exec > >> system call. > >> > >> > >> Index: sys/kern/kern_exec.c > >> =================================================================== > >> RCS file: /cvs/src/sys/kern/kern_exec.c,v > >> retrieving revision 1.173 > >> diff -u -p -r1.173 kern_exec.c > >> --- sys/kern/kern_exec.c 5 Dec 2015 10:11:53 -0000 1.173 > >> +++ sys/kern/kern_exec.c 12 Dec 2015 17:34:43 -0000 > >> @@ -77,6 +77,12 @@ const struct kmem_va_mode kv_exec = { > >> .kv_map = &exec_map > >> }; > >> > >> +struct argcontext > >> +{ > >> + char *buffer; > >> + char *bptr; > >> +}; > >> + > >> /* > >> * Map the shared signal code. > >> */ > >> @@ -231,6 +237,58 @@ bad1: > >> return (error); > >> } > >> > >> +void > >> +copy_fake_args(struct exec_package *pack, struct argcontext *argctx, > >> + long *count) > >> +{ > >> + char **fake_args_vec = pack->ep_fa; > >> + > >> + while (*fake_args_vec != NULL) { > >> + char *arg = *fake_args_vec; > >> + > >> + while (*arg) > >> + *argctx->bptr++ = *arg++; > >> + > >> + *argctx->bptr++ = '\0'; > >> + > >> + free(*fake_args_vec, M_EXEC, 0); > >> + > >> + fake_args_vec++; > >> + (*count)++; > >> + } > >> + > >> + free(pack->ep_fa, M_EXEC, 0); > >> + pack->ep_flags &= ~EXEC_HASARGL; > >> +} > >> + > >> +int > >> +copy_strings(char * const *src, struct argcontext *argctx, long *count) > >> +{ > >> + int error; > >> + size_t len; > >> + size_t maxlen = argctx->buffer + ARG_MAX - argctx->bptr; > >> + char *sp; > >> + > >> + while (1) { > >> + if ((error = copyin(src, &sp, sizeof(sp))) != 0) > >> + return error; > >> + if (!sp) > >> + break; > >> + if ((error = copyinstr(sp, argctx->bptr, maxlen, &len)) != > >> 0) { > >> + if (error == ENAMETOOLONG) > >> + error = E2BIG; > >> + return error; > >> + } > >> + > >> + maxlen -= len; > >> + argctx->bptr += len; > >> + src++; > >> + (*count)++; > >> + } > >> + > >> + return 0; > >> +} > >> + > >> /* > >> * exec system call > >> */ > >> @@ -242,13 +300,13 @@ sys_execve(struct proc *p, void *v, regi > >> syscallarg(char *const *) argp; > >> syscallarg(char *const *) envp; > >> } */ *uap = v; > >> + struct argcontext argctx; > >> int error; > >> struct exec_package pack; > >> struct nameidata nid; > >> struct vattr attr; > >> struct ucred *cred = p->p_ucred; > >> - char *argp; > >> - char * const *cpp, *dp, *sp; > >> + char * const *cpp; > >> #ifdef KTRACE > >> char *env_start; > >> #endif > >> @@ -261,7 +319,6 @@ sys_execve(struct proc *p, void *v, regi > >> char *stack; > >> struct ps_strings arginfo; > >> struct vmspace *vm = pr->ps_vmspace; > >> - char **tmpfap; > >> extern struct emul emul_native; > >> #if NSYSTRACE > 0 > >> int wassugid = ISSET(pr->ps_flags, PS_SUGID | PS_SUGIDEXEC); > >> @@ -317,38 +374,23 @@ sys_execve(struct proc *p, void *v, regi > >> pack.ep_flags = 0; > >> > >> /* see if we can run it. */ > >> - if ((error = check_exec(p, &pack)) != 0) { > >> + if ((error = check_exec(p, &pack)) != 0) > >> goto freehdr; > >> - } > >> > >> /* XXX -- THE FOLLOWING SECTION NEEDS MAJOR CLEANUP */ > >> - > >> + > >> /* allocate an argument buffer */ > >> - argp = km_alloc(NCARGS, &kv_exec, &kp_pageable, &kd_waitok); > >> + argctx.buffer = km_alloc(NCARGS, &kv_exec, &kp_pageable, > >> &kd_waitok); > >> #ifdef DIAGNOSTIC > >> - if (argp == NULL) > >> - panic("execve: argp == NULL"); > >> + if (argctx.buffer == NULL) > >> + panic("execve: argctx.buffer == NULL"); > >> #endif > >> - dp = argp; > >> + argctx.bptr = argctx.buffer; > >> argc = 0; > >> > >> /* copy the fake args list, if there's one, freeing it as we go */ > >> - if (pack.ep_flags & EXEC_HASARGL) { > >> - tmpfap = pack.ep_fa; > >> - while (*tmpfap != NULL) { > >> - char *cp; > >> - > >> - cp = *tmpfap; > >> - while (*cp) > >> - *dp++ = *cp++; > >> - *dp++ = '\0'; > >> - > >> - free(*tmpfap, M_EXEC, 0); > >> - tmpfap++; argc++; > >> - } > >> - free(pack.ep_fa, M_EXEC, 0); > >> - pack.ep_flags &= ~EXEC_HASARGL; > >> - } > >> + if (pack.ep_flags & EXEC_HASARGL) > >> + copy_fake_args(&pack, &argctx, &argc); > >> > >> /* Now get argv & environment */ > >> if (!(cpp = SCARG(uap, argp))) { > >> @@ -359,21 +401,9 @@ sys_execve(struct proc *p, void *v, regi > >> if (pack.ep_flags & EXEC_SKIPARG) > >> cpp++; > >> > >> - while (1) { > >> - len = argp + ARG_MAX - dp; > >> - if ((error = copyin(cpp, &sp, sizeof(sp))) != 0) > >> - goto bad; > >> - if (!sp) > >> - break; > >> - if ((error = copyinstr(sp, dp, len, &len)) != 0) { > >> - if (error == ENAMETOOLONG) > >> - error = E2BIG; > >> - goto bad; > >> - } > >> - dp += len; > >> - cpp++; > >> - argc++; > >> - } > >> + /* copy args */ > >> + if (copy_strings(cpp, &argctx, &argc) != 0) > >> + goto bad; > >> > >> /* must have at least one argument */ > >> if (argc == 0) { > >> @@ -382,39 +412,31 @@ sys_execve(struct proc *p, void *v, regi > >> } > >> > >> #ifdef KTRACE > >> - if (KTRPOINT(p, KTR_EXECARGS)) > >> - ktrexec(p, KTR_EXECARGS, argp, dp - argp); > >> + if (KTRPOINT(p, KTR_EXECARGS)) { > >> + ktrexec(p, KTR_EXECARGS, argctx.buffer, > >> + argctx.bptr - argctx.buffer); > >> + } > >> #endif > >> > >> envc = 0; > >> /* environment does not need to be there */ > >> if ((cpp = SCARG(uap, envp)) != NULL ) { > >> #ifdef KTRACE > >> - env_start = dp; > >> + env_start = argctx.bptr; > >> #endif > >> - while (1) { > >> - len = argp + ARG_MAX - dp; > >> - if ((error = copyin(cpp, &sp, sizeof(sp))) != 0) > >> - goto bad; > >> - if (!sp) > >> - break; > >> - if ((error = copyinstr(sp, dp, len, &len)) != 0) { > >> - if (error == ENAMETOOLONG) > >> - error = E2BIG; > >> - goto bad; > >> - } > >> - dp += len; > >> - cpp++; > >> - envc++; > >> - } > >> - > >> + if (copy_strings(cpp, &argctx, &envc) != 0) > >> + goto bad; > >> #ifdef KTRACE > >> - if (KTRPOINT(p, KTR_EXECENV)) > >> - ktrexec(p, KTR_EXECENV, env_start, dp - env_start); > >> + if (KTRPOINT(p, KTR_EXECENV)) { > >> + ktrexec(p, KTR_EXECENV, env_start, > >> + argctx.bptr - env_start); > >> + } > >> #endif > >> } > >> > >> - dp = (char *)(((long)dp + _STACKALIGNBYTES) & ~_STACKALIGNBYTES); > >> + argctx.bptr = > >> + (char *)(((long)argctx.bptr + _STACKALIGNBYTES) & > >> + ~_STACKALIGNBYTES); > >> > >> sgap = STACKGAPLEN; > >> > >> @@ -430,7 +452,8 @@ sys_execve(struct proc *p, void *v, regi > >> > >> /* Now check if args & environ fit into new stack */ > >> len = ((argc + envc + 2 + pack.ep_emul->e_arglen) * sizeof(char *) > >> + > >> - sizeof(long) + dp + sgap + sizeof(struct ps_strings)) - argp; > >> + sizeof(long) + argctx.bptr + sgap + sizeof(struct ps_strings)) > >> - > >> + argctx.buffer; > >> > >> len = (len + _STACKALIGNBYTES) &~ _STACKALIGNBYTES; > >> > >> @@ -505,7 +528,7 @@ sys_execve(struct proc *p, void *v, regi > >> stack = (char *)(vm->vm_minsaddr - len); > >> #endif > >> /* Now copy argc, args & environ to new stack */ > >> - if (!(*pack.ep_emul->e_copyargs)(&pack, &arginfo, stack, argp)) > >> + if (!(*pack.ep_emul->e_copyargs)(&pack, &arginfo, stack, > >> argctx.buffer)) > >> goto exec_abort; > >> > >> /* copy out the process's ps_strings structure */ > >> @@ -672,7 +695,7 @@ sys_execve(struct proc *p, void *v, regi > >> timespecclear(&p->p_tu.tu_runtime); > >> p->p_tu.tu_uticks = p->p_tu.tu_sticks = p->p_tu.tu_iticks = 0; > >> > >> - km_free(argp, NCARGS, &kv_exec, &kp_pageable); > >> + km_free(argctx.buffer, NCARGS, &kv_exec, &kp_pageable); > >> > >> pool_put(&namei_pool, nid.ni_cnd.cn_pnbuf); > >> vn_close(pack.ep_vp, FREAD, cred, p); > >> @@ -777,7 +800,7 @@ bad: > >> /* close and put the exec'd file */ > >> vn_close(pack.ep_vp, FREAD, cred, p); > >> pool_put(&namei_pool, nid.ni_cnd.cn_pnbuf); > >> - km_free(argp, NCARGS, &kv_exec, &kp_pageable); > >> + km_free(argctx.buffer, NCARGS, &kv_exec, &kp_pageable); > >> > >> freehdr: > >> free(pack.ep_hdr, M_EXEC, pack.ep_hdrlen); > >> @@ -806,7 +829,7 @@ exec_abort: > >> free(pack.ep_emul_arg, M_TEMP, pack.ep_emul_argsize); > >> pool_put(&namei_pool, nid.ni_cnd.cn_pnbuf); > >> vn_close(pack.ep_vp, FREAD, cred, p); > >> - km_free(argp, NCARGS, &kv_exec, &kp_pageable); > >> + km_free(argctx.buffer, NCARGS, &kv_exec, &kp_pageable); > >> > >> free_pack_abort: > >> free(pack.ep_hdr, M_EXEC, pack.ep_hdrlen); > >
