Messages in this thread |  | | Date | Thu, 2 Feb 2012 15:27:05 -0800 | From | Andrew Morton <> | Subject | Re: [patch cr 4/4] c/r: prctl: Extend PR_SET_MM to set up more mm_struct entries |
| |
On Mon, 30 Jan 2012 18:09:09 +0400 Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> After restore we would like the 'ps' command show the command > line and evironment exactly the same it was at checkpoint time. > > So this additional PR_SET_MM_ allow us to do so. Note that > these members of mm_struct is rather used for output in > procfs, except auxv vector which is used by ld.so mostly.
This changelog is pretty darned hard to understand. Can we have a version 2 please?
> > ... > > @@ -1753,19 +1755,6 @@ static int prctl_set_mm(int opt, unsigne > mm->end_data = addr; > break; > > - case PR_SET_MM_START_STACK: > - > -#ifdef CONFIG_STACK_GROWSUP > - vm_req_flags = VM_READ | VM_WRITE | VM_GROWSUP; > -#else > - vm_req_flags = VM_READ | VM_WRITE | VM_GROWSDOWN; > -#endif > - if ((vma->vm_flags & vm_req_flags) != vm_req_flags) > - goto out; > - > - mm->start_stack = addr; > - break; > - > case PR_SET_MM_START_BRK: > if (addr <= mm->end_data) > goto out; > @@ -1790,16 +1779,53 @@ static int prctl_set_mm(int opt, unsigne > mm->brk = addr; > break;
Here would be a good place to add some nice comments explaining what these do. Although I guess that isn't needed if one can get that info by typing "man prctl".
> + case PR_SET_MM_START_STACK: > + case PR_SET_MM_ARG_START: > + case PR_SET_MM_ARG_END: > + case PR_SET_MM_ENV_START: > + case PR_SET_MM_ENV_END: > +#ifdef CONFIG_STACK_GROWSUP > + if (vma_flags_mismatch(vma, VM_READ | VM_WRITE | VM_GROWSUP, 0)) > +#else > + if (vma_flags_mismatch(vma, VM_READ | VM_WRITE | VM_GROWSDOWN, 0)) > +#endif > + goto out; > + if (opt == PR_SET_MM_START_STACK) > + mm->start_stack = addr; > + else if (opt == PR_SET_MM_ARG_START) > + mm->arg_start = addr; > + else if (opt == PR_SET_MM_ARG_END) > + mm->arg_end = addr; > + else if (opt == PR_SET_MM_ENV_START) > + mm->env_start = addr; > + else if (opt == PR_SET_MM_ENV_END) > + mm->env_end = addr; > + break; > + > + case PR_SET_MM_AUXV: { > + unsigned long user_auxv[AT_VECTOR_SIZE]; > + > + if (arg4 > sizeof(mm->saved_auxv)) > + goto out; > + up_read(&mm->mmap_sem); > + > + if (copy_from_user(user_auxv, (const void __user *)addr, arg4)) > + return EFAULT; > + > + task_lock(current); > + memcpy(mm->saved_auxv, user_auxv, arg4); > + task_unlock(current); > + > + return 0; > + }
I worry a bit about this. We're giving userspace the ability to modify various mm_struct fields. Userspace can already do this via exec(elf-file), but perhaps this opens up a way in which userspace can newly trigger kernel bugs.
|  |