linux-kernel - Re: [RFC] c/r: prctl: Add ability to set new mm_struct::exe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120305160127.GM7366@moon>
Date:	Mon, 5 Mar 2012 20:01:27 +0400
From:	Cyrill Gorcunov <gorcunov@...nvz.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Pavel Emelyanov <xemul@...allels.com>,
	Kees Cook <keescook@...omium.org>, Tejun Heo <tj@...nel.org>
Subject: Re: [RFC] c/r: prctl: Add ability to set new mm_struct::exe_file

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/