Ping?
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);
