From: NeilBrown <neilb@suse.de>
To: Solar Designer <solar@openwall.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
Vasiliy Kulikov <segoon@openwall.com>,
linux-kernel@vger.kernel.org, Greg Kroah-Hartman <gregkh@suse.de>,
Andrew Morton <akpm@linux-foundation.org>,
"David S. Miller" <davem@davemloft.net>,
kernel-hardening@lists.openwall.com, Jiri Slaby <jslaby@suse.cz>,
James Morris <jmorris@namei.org>,
Alexander Viro <viro@zeniv.linux.org.uk>,
linux-fsdevel@vger.kernel.org,
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Subject: Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common()
Date: Wed, 13 Jul 2011 17:06:57 +1000 [thread overview]
Message-ID: <20110713170657.59dae548@notabene.brown> (raw)
In-Reply-To: <20110713063142.GA19976@openwall.com>
On Wed, 13 Jul 2011 10:31:42 +0400 Solar Designer <solar@openwall.com> wrote:
> 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 don't think it can be intended to cause 'execve' to fail when a user is at
the NPROC limit - except in the specific case that the process has previously
called setuid. So I feel justified in calling it an unintended consequence.
It my not be a very common consequence but but we all know that uncommon
things do happen.
I agree that having different limits for different cases could make this much
less of a problem, but it doesn't necessarily remove it.
>
> > 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
Yes, the PF_FORKNOEXEC test causes problems in that case.
Using just the PF_SUPERPRIV test would still be a good idea I think, but would
not be quite as thorough a check.
Adding a new PF flag would be possible (there seem to be 3 unused) but is
probably not justified.
NeilBrown
next prev parent reply other threads:[~2011-07-13 7:07 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-12 13:09 RLIMIT_NPROC check in set_user() Vasiliy Kulikov
2011-07-06 17:36 ` Vasiliy Kulikov
2011-07-06 18:01 ` Linus Torvalds
2011-07-06 18:59 ` [kernel-hardening] " Vasiliy Kulikov
2011-07-07 7:56 ` Vasiliy Kulikov
2011-07-07 8:19 ` Vasiliy Kulikov
2011-07-12 13:27 ` [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() Vasiliy Kulikov
2011-07-12 21:16 ` Linus Torvalds
2011-07-12 23:14 ` NeilBrown
2011-07-13 6:31 ` Solar Designer
2011-07-13 7:06 ` NeilBrown [this message]
2011-07-13 20:46 ` Linus Torvalds
2011-07-14 0:11 ` James Morris
2011-07-14 1:27 ` NeilBrown
2011-07-14 15:06 ` Solar Designer
2011-07-15 3:30 ` NeilBrown
2011-07-15 5:35 ` Willy Tarreau
2011-07-15 6:31 ` [kernel-hardening] " Vasiliy Kulikov
2011-07-15 7:06 ` NeilBrown
2011-07-15 7:38 ` Vasiliy Kulikov
2011-07-15 13:04 ` Solar Designer
2011-07-15 13:58 ` [kernel-hardening] " Stephen Smalley
2011-07-15 15:26 ` Vasiliy Kulikov
2011-07-15 19:54 ` Stephen Smalley
2011-07-21 4:09 ` NeilBrown
2011-07-21 12:48 ` Solar Designer
2011-07-21 18:21 ` Linus Torvalds
2011-07-21 19:39 ` [kernel-hardening] " Solar Designer
2011-07-25 17:14 ` Vasiliy Kulikov
2011-07-25 23:40 ` [kernel-hardening] " Solar Designer
2011-07-26 0:47 ` NeilBrown
2011-07-26 1:16 ` Solar Designer
2011-07-26 4:11 ` NeilBrown
2011-07-26 14:48 ` [patch v2] " Vasiliy Kulikov
2011-07-27 2:15 ` NeilBrown
2011-07-29 7:07 ` [kernel-hardening] " Vasiliy Kulikov
2011-07-29 8:06 ` Vasiliy Kulikov
2011-07-29 8:11 ` [patch v3] " Vasiliy Kulikov
2011-07-29 8:17 ` James Morris
2011-07-14 1:30 ` [PATCH] " KOSAKI Motohiro
2011-07-13 5:36 ` KOSAKI Motohiro
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110713170657.59dae548@notabene.brown \
--to=neilb@suse.de \
--cc=akpm@linux-foundation.org \
--cc=davem@davemloft.net \
--cc=gregkh@suse.de \
--cc=jmorris@namei.org \
--cc=jslaby@suse.cz \
--cc=kernel-hardening@lists.openwall.com \
--cc=kosaki.motohiro@jp.fujitsu.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=segoon@openwall.com \
--cc=solar@openwall.com \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).