Re: [RFC c/r 4/4] c/r: prctl: Extend PR_SET_MM to set up more mm_struct entries

[Cyrill Gorcunov <gorcunov@openvz.org>]

On Fri, Jan 27, 2012 at 10:37:05AM -0800, Kees Cook wrote:
> On Fri, Jan 27, 2012 at 9:53 AM, Cyrill Gorcunov <gorcunov@openvz.org> wrote:
> > +               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;
> 
> Why not a switch statement here? Not that it really matters. :)
> 

Just to look different from toplevel switch, which is better from my POV.

> > +
> > +       case PR_SET_MM_AUXV:
> > +               if (arg4 > sizeof(mm->saved_auxv))
> > +                       goto out;
> > +               up_read(&mm->mmap_sem);
> > +
> > +               error = -EFAULT;
> > +               if (!copy_from_user(mm->saved_auxv, (const void __user *)addr, arg4))
> > +                       error = 0;
> > +
> > +               return error;
> 
> Is the mmap_sem released here because of the copy_from_user()? Is it
> still safe to write to saved_auxv?
> 

At moment I believe yes (if only I'm not missing something), we poke this
vector at elf loading procedure, so it's up to user to sync access to "own"
saved_auxv and write sane values inside.

	Cyrill