Linus, Neil, Motohiro - thank you for your comments! On Wed, Jul 13, 2011 at 09:14:08AM +1000, NeilBrown wrote: > The contrast is really "failing when trying to use reduced privileges is > safer than failing to reduce privileges - if the reduced privileges are not > available". Right. > Note that there is room for a race that could have unintended consequences. > > Between the 'setuid(ordinary-user)' and a subsequent 'exit()' after execve() > has failed, any other process owned by the same user (and we know where are > quite a few) would fail an execve() where it really should not. It is not obvious to me that this is unintended, and that dealing with it in some way makes much of a difference. (Also, it's not exactly "any other process owned by the same user" - this only affects processes that also run with similar or lower RLIMIT_NPROC. So, for example, if a web server is set to use RLIMIT_NPROC of 30, but interactive logins use 40, then the latter may succeed and allow for shell commands to succeed. This is actually a common combination of settings that we've been using on some systems for years.) > I think it would be safer to add a test for PF_SUPERPRIV and PF_FORKNOEXEC > in current->flags and only fail the execve if both are set. > i.e. > (current->flags & (PF_SUPERPRIV|PF_FORKNOEXEC)) == (PF_SUPERPRIV|PF_FORKNOEXEC) > > That should narrow it down to only failing in the particular case that we are > interested in. That's a curious idea, and apparently this is what NetBSD does, but unfortunately it does not match a common use case that we are interested in - specifically, Apache with suEXEC (which is part of the Apache distribution). Here's what happens: httpd runs as non-root. It forks, execs suexec (SUID root). suexec calls setuid() to the target non-root user and execve() on the CGI program (script, interpreter, whatever). Notice how the fork() and the setuid() are separated by execve() of suexec itself. Thus, we need to apply the RLIMIT_NPROC check on execve() unconditionally (well, we may allow processes with CAP_SYS_RESOURCE to proceed despite of the failed check, like it's done in -ow patches), or at least not on the condition proposed above. Alexander -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
