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

[Matt Helsley]

On Sat, Mar 10, 2012 at 11:48:54AM +0400, Cyrill Gorcunov wrote:
> On Fri, Mar 09, 2012 at 03:59:01PM -0800, Matt Helsley wrote:
> > > Well, in final version I switched back to
> > > 
> > > +	if (mm->num_exe_file_vmas)
> > > +		return -EBUSY;
> > > 
> > > simply because it's more flexible than mm->exe_file.
> > > 
> > > With mm->exe_file this prctl option become a one-shot
> > > only, and while at moment our user-space tool can perfectly
> > > live with that I thought that there is no strict need to
> > > limit the option this way from the very beginning.
> > 
> > As far as backward compatibility, isn't it better to lift that restriction
> > later rather than add it? I think the latter would very likely "break"
> > things whereas the former would not.
> 
> Indeed. But I think any change will mean compatibility broken, programs
> may rely on one-shot or multi-shot behaviour. So I personally vote
> for more flexible approach here.

Very true. In fact thinking about this prctl a bit more makes me more certain
that one-shot is better and it ought to stay that way forever. The
flexibility to change the /proc/pid/exe symlink could be yet another
way for malicious code to obscure a compromised program and
masquerade as a benign process. That's a problem inherent in this prctl
whether its one-shot or multi-shot. However, if you use the one-shot
approach then a security-concious program can use this prctl once
during its early initialization to ensure the prctl cannot later be abused
for this purpose.

> 
> > 
> > I also prefer that restriction because it establishes a bound on how
> > frequently the symlink can change. Keeping it a one-shot deal makes the
> > values that show up in tools like top more reliable for admins.
> 
> How? Once exe_file changed -- we already cheating the kernel, it's not
> bad, not good, just work this way ;) I mean imagine an admin which
> runs top and sees some program in 'top' ouput (btw, I'm not sure but
> does top really parse /proc/pid/exe?) so say he sees some programs

Sorry, my phrase "tools like top" was ambiguous. I was trying to succinctly
refer to top as one type of program which might rely on this information.
I don't know if top itself uses this file but it would be reasonable for
tools of that kind to use/display it to an administrator.

> names -- how would he know if a program did change own /proc/pid/exe
> at all? Note it's not that important how many times the symlink was
> changed there is simply no way to find out if it was changed at all,
> and actually from my POV it's a win for transparent c/r, that was
> all the idea.

I am quite aware of the c/r use for this prctl :). However I also
wonder if there aren't serious malicious uses of it. I'm not saying the
symlink has to be perfectly accurate at all times,  but it's easy and
reasonable to make it much harder to abuse this particular prctl for
malicious purposes by making it one-shot.

> 
> > > 
> > > As far as I understand overall num_exe_file_vmas concept -- we track
> > > a number of VM_EXECUTABLE with it, so setting new exe_file should not
> > > change num_exe_file_vmas I think.
> > 
> > True, it's literally correct. However the whole reason for having it
> > is to turn the mm->exe_file reference into a sort of weak reference
> > which happens to coincide with counting the number of VM_EXECUTABLE vmas
> > until you do c/r (really just the restart side of c/r).
> > 
> 
> Look. We actually have a period of time where exe_file is set but
> num_exe_file_vmas = 0 when we start program execution before elf
> map get parsed, so I dare to say such state is legitime (yes, userspace
> doesn't see this state and yes, we start mapping elf sections pretty
> immediately after exe_file assigned). So I don't see much problem in
> extending this "state" (exe_file!=0,num_exe_file_vmas = 0).

True, there is a time when that combination of values occurs. However
who sets those values and when is the important difference.

Before this patch that state was rather ephemeral and almost entirely
under the control of the kernel. The only way userspace could change it
was by unmapping the region(s) mapped during exec*(). At that point it
could not "lie" and insert some other symlink there and the admin would
be better able to determine what had happened.

With this patch -- especially the multi-shot form -- the symlink will
be entirely under the control of (potentially untrusted) userspace code
and the admin is totally at the mercy of the userspace code. In
single-shot form programs could use the prctl() to ensure the symlink
could not be changed later -- the restart tool would be the only program
that would need to ensure that prctl() had not been used since the last
exec*().

If we're going to let userspace do arbitrary things to the symlink I can't
help but wonder why we can't skip the prctl() altogether and just enable
MAP_EXECUTABLE in mmap().

Cheers,
	-Matt Helsley

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