Re: [RFC] c/r: prctl: Add ability to set new mm_struct::exe_file

[Cyrill Gorcunov <gorcunov@openvz.org>]

On Mon, Mar 05, 2012 at 04:40:29PM +0100, Oleg Nesterov wrote:
> >
> > Hi Oleg!
> >
> > Replying to both your email -- I wanted to be as close to open_exec
> > as possible.
> 
> I see. But open_exec() is different, it returns the file we are going
> to read/mmap. PR_SET_MM_EXE_FILE is different, I think O_RDONLY buys
> nothing and looks confusing.
> 
> Anyway, as I said I won't argue.
> 

OK

> > This prctl does cheat the kernel
> 
> Yep. Except, well, it cheats the user-space.

And kernel as well, since this link is not anymore
the same as it was, and memory read from this file
(at execution time) will keep data irrelevant to what
the new symlink points to (but it's exactly by design).

> 
> > but with this tests
> > the cheating should be minimized (it's almost the same as open_exec
> > does).
> 
> I don't reallt understand "minimized" ;) With this tests
> proc/pid/exe can't look "obviously wrong", I agree. But that is all.

call it "minimized wrong" ;)

> 
> > > Seriously, I think we should cleanup this before c/r adds more
> > > ugliness. I'll try to make the patch today.
> >
> > Cleanup what? If you mean this patch -- just point me what
> > should I do.
> 
> I just sent the patch, "turn mm->exe_file into mm->exe_path"
> 

Cool, just got it, thanks!

> > > And with all these checks I am no longer sure that fd is better
> > > than filename ;)
> >
> > This security tests was a reason why I've used open_exec in
> > first version of the patch
> 
> Yes, but me and Pavel forced you to use "int fd" ;)
> 

With plain fd it doesn't require kernel to allocate temp
memory and copy path from user space, this is a positive
effect.

> > (and I still would prefer to
> > have open_exec here instead of fd).
> 
> With the patch I sent "struct file *" is not needed at all.
> I think prctl() can use user_path().

OK!

> 
> > As to allow-write-access -- it should be cleaned once process
> > finished, no?
> 
> Exactly! And who will increment ->i_writecount? Nobody, that is
> the problem.

I see, thanks!

	Cyrill