* RLIMIT_NPROC check in set_user() @ 2011-06-12 13:09 Vasiliy Kulikov 2011-07-06 17:36 ` Vasiliy Kulikov 0 siblings, 1 reply; 41+ messages in thread From: Vasiliy Kulikov @ 2011-06-12 13:09 UTC (permalink / raw) To: linux-kernel Cc: Linus Torvalds, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, James Morris, Neil Brown, kernel-hardening Hi, I'd want to start a discussion of RLIMIT_NPROC check in set_user(). 8 years ago set_user() was forced to check whether RLIMIT_NPROC limit is reached, and, if so, abort set_user(): http://lkml.org/lkml/2003/7/13/226 [1] Before the patch setuid() and similar were not able to fail because of the reached limit. So, some daemons running as root, dropping root and switching uid/gid to some user were able to greatly exceed the limit of processes running as this user. The patch has solved this problem. But it also unexpectedly created new security threat. Many poorly written programs running as root (or owning CAP_SYS_ADMIN) don't expect setuid() failure and don't check its return code. If it fails, they still assume the uid has changed, but actually it has not, which leads to very sad consequences. In times of Linux 2.4 the initial problem (the lack of RLIMIT_NPROC check) was solved in -ow patches by introducing the check in execve(), not in setuid()/setuid() helpers as a process after dropping privileges usually does execve() call. While strictly speaking it is not a full fix (it doesn't limit the number of not-execve'd but setuid'ed processes) it works just fine for most of programs. Another possible workaround is not moving the check from setuid() to execve(), but sending SIGSEGV to the current process if setuid() failed [2]. This should solve the problem of poor programs and looks like not breaking legitimate applications that handle setuid() failure as they usually just print error message to the logfile/stderr and exit. Also as it is a horribly rare case (setuid() failure), more complicated code path might be not tested very well. I want to repeat myself: I don't consider checking RLIMIT_NPROC in setuid() as a bug (a lack of syscalls return code checking is a real bug), but as a pouring oil on the flames of programs doing poorly written privilege dropping. I believe the situation may be improved by relatively small ABI changes that shouldn't be visible to normal programs. The first solution is reverting [1] and introducing similar check in execve(), just like in -ow patch for 2.4. The second solution is applying [2] and sending SIGSEGV in case of privileges dropping failure. I'd be happy to hear opinions about improving the fixes above or alternative fixes. Related references: [1] - http://lkml.org/lkml/2003/7/13/226 [2] - https://lkml.org/lkml/2006/8/19/129 Thanks, -- Vasiliy ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: RLIMIT_NPROC check in set_user() 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 0 siblings, 1 reply; 41+ messages in thread From: Vasiliy Kulikov @ 2011-07-06 17:36 UTC (permalink / raw) To: linux-kernel Cc: Linus Torvalds, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, James Morris, Neil Brown, kernel-hardening On Sun, Jun 12, 2011 at 17:09 +0400, Vasiliy Kulikov wrote: > I'd be happy to hear opinions about improving the fixes above or > alternative fixes. No comments? Even "Sigh, what a nasty problem. But we cannot really fix it without significantly breaking the stuff. Go and drink something." ? -- Vasiliy ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: RLIMIT_NPROC check in set_user() 2011-07-06 17:36 ` Vasiliy Kulikov @ 2011-07-06 18:01 ` Linus Torvalds 2011-07-06 18:59 ` [kernel-hardening] " Vasiliy Kulikov 0 siblings, 1 reply; 41+ messages in thread From: Linus Torvalds @ 2011-07-06 18:01 UTC (permalink / raw) To: Vasiliy Kulikov Cc: linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, James Morris, Neil Brown, kernel-hardening On Wed, Jul 6, 2011 at 10:36 AM, Vasiliy Kulikov <segoon@openwall.com> wrote: > On Sun, Jun 12, 2011 at 17:09 +0400, Vasiliy Kulikov wrote: >> I'd be happy to hear opinions about improving the fixes above or >> alternative fixes. > > No comments? Even "Sigh, what a nasty problem. But we cannot really > fix it without significantly breaking the stuff. Go and drink something." ? Thanks for reminding me. My reaction is: "let's just remote the crazy check from set_user() entirely". If somebody has credentials to change users, they damn well have credentials to override the RLIMIT_NPROC too, and as you say, failure is likely a bigger security threat than success. The whole point of RLIMIT_NPROC is to avoid fork-bombs. If we go over the limit for some other reason that is controlled by the super-user, who cares? So let's keep it in kernel/fork.c where we actually create a *new* process (and where everybody knows exactly what the limit means, and people who don't check for error cases are just broken). And remove it from everywhere else. Hmm? Linus ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] Re: RLIMIT_NPROC check in set_user() 2011-07-06 18:01 ` Linus Torvalds @ 2011-07-06 18:59 ` Vasiliy Kulikov 2011-07-07 7:56 ` Vasiliy Kulikov 0 siblings, 1 reply; 41+ messages in thread From: Vasiliy Kulikov @ 2011-07-06 18:59 UTC (permalink / raw) To: kernel-hardening Cc: linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, James Morris, Neil Brown On Wed, Jul 06, 2011 at 11:01 -0700, Linus Torvalds wrote: > My reaction is: "let's just remote the crazy check from set_user() > entirely". Honestly, I didn't expect such a positive reaction from you in the first reply :) > The whole point of RLIMIT_NPROC is to avoid fork-bombs. It is also used in cases where there is implicit or explicit limit on some other resource per process leading to the global limit of RLIMIT_NPROC*X. The most obvious case of X is RLIMIT_AS. Purely pragmatic approach is introducing the check in execve() to heuristically limit the number of user processes. If the program uses PAM to register a user session, maxlogins from pam_limits is the Right Way. But many programs simply don't use PAM because of the performance issues. E.g. apache doesn't use PAM. On a shared web hosting this is a real issue. In -ow patch execve() checked for the exceeded RLIMIT_NPROC, which effectively solved Apache's problem. ...and execve() error handling is hard to miss ;-) > So let's keep it in kernel/fork.c where we actually create a *new* > process (and where everybody knows exactly what the limit means, and > people who don't check for error cases are just broken). And remove it > from everywhere else. There are checks only in copy_process() and set_user(). Thanks, -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] Re: RLIMIT_NPROC check in set_user() 2011-07-06 18:59 ` [kernel-hardening] " Vasiliy Kulikov @ 2011-07-07 7:56 ` Vasiliy Kulikov 2011-07-07 8:19 ` Vasiliy Kulikov 0 siblings, 1 reply; 41+ messages in thread From: Vasiliy Kulikov @ 2011-07-07 7:56 UTC (permalink / raw) To: kernel-hardening Cc: linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, James Morris, Neil Brown (Sorry, I've dropped Linus from CC somehow ;-) On Wed, Jul 06, 2011 at 22:59 +0400, Vasiliy Kulikov wrote: > On Wed, Jul 06, 2011 at 11:01 -0700, Linus Torvalds wrote: > > My reaction is: "let's just remote the crazy check from set_user() > > entirely". > > Honestly, I didn't expect such a positive reaction from you in the first > reply :) > > > > The whole point of RLIMIT_NPROC is to avoid fork-bombs. > > It is also used in cases where there is implicit or explicit limit on > some other resource per process leading to the global limit of > RLIMIT_NPROC*X. The most obvious case of X is RLIMIT_AS. > > Purely pragmatic approach is introducing the check in execve() to > heuristically limit the number of user processes. If the program uses > PAM to register a user session, maxlogins from pam_limits is the Right > Way. But many programs simply don't use PAM because of the performance > issues. E.g. apache doesn't use PAM. On a shared web hosting this is a > real issue. > > In -ow patch execve() checked for the exceeded RLIMIT_NPROC, which > effectively solved Apache's problem. > > ...and execve() error handling is hard to miss ;-) > > > > So let's keep it in kernel/fork.c where we actually create a *new* > > process (and where everybody knows exactly what the limit means, and > > people who don't check for error cases are just broken). And remove it > > from everywhere else. > > There are checks only in copy_process() and set_user(). > > Thanks, -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] Re: RLIMIT_NPROC check in set_user() 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 0 siblings, 1 reply; 41+ messages in thread From: Vasiliy Kulikov @ 2011-07-07 8:19 UTC (permalink / raw) To: kernel-hardening, Linus Torvalds Cc: linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, James Morris, Neil Brown (Sorry, I've dropped Linus from CC somehow ;-) On Wed, Jul 06, 2011 at 22:59 +0400, Vasiliy Kulikov wrote: > On Wed, Jul 06, 2011 at 11:01 -0700, Linus Torvalds wrote: > > My reaction is: "let's just remote the crazy check from set_user() > > entirely". > > Honestly, I didn't expect such a positive reaction from you in the first > reply :) > > > > The whole point of RLIMIT_NPROC is to avoid fork-bombs. > > It is also used in cases where there is implicit or explicit limit on > some other resource per process leading to the global limit of > RLIMIT_NPROC*X. The most obvious case of X is RLIMIT_AS. > > Purely pragmatic approach is introducing the check in execve() to > heuristically limit the number of user processes. If the program uses > PAM to register a user session, maxlogins from pam_limits is the Right > Way. But many programs simply don't use PAM because of the performance > issues. E.g. apache doesn't use PAM. On a shared web hosting this is a > real issue. > > In -ow patch execve() checked for the exceeded RLIMIT_NPROC, which > effectively solved Apache's problem. > > ...and execve() error handling is hard to miss ;-) > > > > So let's keep it in kernel/fork.c where we actually create a *new* > > process (and where everybody knows exactly what the limit means, and > > people who don't check for error cases are just broken). And remove it > > from everywhere else. > > There are checks only in copy_process() and set_user(). > > Thanks, -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-07 8:19 ` Vasiliy Kulikov @ 2011-07-12 13:27 ` Vasiliy Kulikov 2011-07-12 21:16 ` Linus Torvalds 2011-07-13 5:36 ` KOSAKI Motohiro 0 siblings, 2 replies; 41+ messages in thread From: Vasiliy Kulikov @ 2011-07-12 13:27 UTC (permalink / raw) To: Linus Torvalds Cc: linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, kernel-hardening, Jiri Slaby, James Morris, Neil Brown, Alexander Viro, linux-fsdevel The patch http://lkml.org/lkml/2003/7/13/226 introduced a RLIMIT_NPROC check in set_user() to check for NPROC exceeding via setuid() and similar functions. Before the check there was a possibility to greatly exceed the allowed number of processes by an unprivileged user if the program relied on rlimit only. But the check created new security threat: many poorly written programs simply don't check setuid() return code and believe it cannot fail if executed with root privileges. So, the check is removed in this patch because of too often privilege escalations related to buggy programs. The NPROC can still be enforced in the common code flow of daemons spawning user processes. Most of daemons do fork()+setuid()+execve(). The check introduced in execve() enforces the same limit as in setuid() and doesn't create similar security issues. Similar check was introduced in -ow patches. Signed-off-by: Vasiliy Kulikov <segoon@openwall.com> --- fs/exec.c | 13 +++++++++++++ kernel/sys.c | 6 ------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index ea5f748..0baf5c9 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1436,6 +1436,19 @@ static int do_execve_common(const char *filename, struct files_struct *displaced; bool clear_in_exec; int retval; + const struct cred *cred = current_cred(); + + /* + * We check for RLIMIT_NPROC in execve() instead of set_user() because + * too many poorly written programs don't check setuid() return code. + * The check in execve() does the same thing for programs doing + * setuid()+execve(), but without similar security issues. + */ + if (atomic_read(&cred->user->processes) > rlimit(RLIMIT_NPROC) && + cred->user != INIT_USER) { + retval = -EAGAIN; + goto out_ret; + } retval = unshare_files(&displaced); if (retval) diff --git a/kernel/sys.c b/kernel/sys.c index e4128b2..0e7509a 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -591,12 +591,6 @@ static int set_user(struct cred *new) if (!new_user) return -EAGAIN; - if (atomic_read(&new_user->processes) >= rlimit(RLIMIT_NPROC) && - new_user != INIT_USER) { - free_uid(new_user); - return -EAGAIN; - } - free_uid(new->user); new->user = new_user; return 0; -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 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 5:36 ` KOSAKI Motohiro 1 sibling, 1 reply; 41+ messages in thread From: Linus Torvalds @ 2011-07-12 21:16 UTC (permalink / raw) To: Vasiliy Kulikov Cc: linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, kernel-hardening, Jiri Slaby, James Morris, Neil Brown, Alexander Viro, linux-fsdevel On Tue, Jul 12, 2011 at 6:27 AM, Vasiliy Kulikov <segoon@openwall.com> wrote: > > The NPROC can still be enforced in the common code flow of daemons > spawning user processes. Most of daemons do fork()+setuid()+execve(). > The check introduced in execve() enforces the same limit as in setuid() > and doesn't create similar security issues. Ok, this looks fine by me. I'd like to get some kind of comment from the selinux etc people (James?) but I'd certainly be willing to take this. Failing when executing a suid application rather than when a suid application releases its heightened credentials seems to be the fundamentally saner approach. IOW, failing to raise privileges rather than failing to lower them. Linus ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-12 21:16 ` Linus Torvalds @ 2011-07-12 23:14 ` NeilBrown 2011-07-13 6:31 ` Solar Designer 0 siblings, 1 reply; 41+ messages in thread From: NeilBrown @ 2011-07-12 23:14 UTC (permalink / raw) To: Linus Torvalds Cc: Vasiliy Kulikov, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, kernel-hardening, Jiri Slaby, James Morris, Alexander Viro, linux-fsdevel On Tue, 12 Jul 2011 14:16:10 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote: > On Tue, Jul 12, 2011 at 6:27 AM, Vasiliy Kulikov <segoon@openwall.com> wrote: > > > > The NPROC can still be enforced in the common code flow of daemons > > spawning user processes. Most of daemons do fork()+setuid()+execve(). > > The check introduced in execve() enforces the same limit as in setuid() > > and doesn't create similar security issues. > > Ok, this looks fine by me. I'd like to get some kind of comment from > the selinux etc people (James?) but I'd certainly be willing to take > this. > > Failing when executing a suid application rather than when a suid > application releases its heightened credentials seems to be the > fundamentally saner approach. IOW, failing to raise privileges rather > than failing to lower them. > > Linus I am happy with the patch in general - it adequately addresses the problem which I fixed by adding the test to set_user which is now being removed. However I don't think your characterisation is quite correct Linus. There is no setuid application, and there is no raising of privileges. 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". 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. 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. NeilBrown ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-12 23:14 ` NeilBrown @ 2011-07-13 6:31 ` Solar Designer 2011-07-13 7:06 ` NeilBrown 0 siblings, 1 reply; 41+ messages in thread From: Solar Designer @ 2011-07-13 6:31 UTC (permalink / raw) To: NeilBrown Cc: Linus Torvalds, Vasiliy Kulikov, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, kernel-hardening, Jiri Slaby, James Morris, Alexander Viro, linux-fsdevel, KOSAKI Motohiro 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-13 6:31 ` Solar Designer @ 2011-07-13 7:06 ` NeilBrown 2011-07-13 20:46 ` Linus Torvalds 0 siblings, 1 reply; 41+ messages in thread From: NeilBrown @ 2011-07-13 7:06 UTC (permalink / raw) To: Solar Designer Cc: Linus Torvalds, Vasiliy Kulikov, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, kernel-hardening, Jiri Slaby, James Morris, Alexander Viro, linux-fsdevel, KOSAKI Motohiro 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 ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-13 7:06 ` NeilBrown @ 2011-07-13 20:46 ` Linus Torvalds 2011-07-14 0:11 ` James Morris 0 siblings, 1 reply; 41+ messages in thread From: Linus Torvalds @ 2011-07-13 20:46 UTC (permalink / raw) To: NeilBrown Cc: Solar Designer, Vasiliy Kulikov, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, kernel-hardening, Jiri Slaby, James Morris, Alexander Viro, linux-fsdevel, KOSAKI Motohiro It sounds like people are effectively Ack'ing the patch, but with this kind of patch I don't want to add the "implicit Ack" that I often do for regular stuff. So could people who think that the patch is ok in its current form just send me their acked-by or reviewed-by? I haven't heard any actual objection to it, and I think it's valid for the current -rc. Alternatively, feel free to send a comment like "I think it's the right thing, but maybe it should wait for the next merge window".. Linus ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-13 20:46 ` Linus Torvalds @ 2011-07-14 0:11 ` James Morris 2011-07-14 1:27 ` NeilBrown 2011-07-14 1:30 ` [PATCH] " KOSAKI Motohiro 0 siblings, 2 replies; 41+ messages in thread From: James Morris @ 2011-07-14 0:11 UTC (permalink / raw) To: Linus Torvalds Cc: NeilBrown, Solar Designer, Vasiliy Kulikov, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, kernel-hardening, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Stephen Smalley On Wed, 13 Jul 2011, Linus Torvalds wrote: > It sounds like people are effectively Ack'ing the patch, but with this > kind of patch I don't want to add the "implicit Ack" that I often do > for regular stuff. > > So could people who think that the patch is ok in its current form > just send me their acked-by or reviewed-by? I haven't heard any actual > objection to it, and I think it's valid for the current -rc. > > Alternatively, feel free to send a comment like "I think it's the > right thing, but maybe it should wait for the next merge window".. Count me in the latter. It does look ok to me, but I'd be happier if it had more testing first (in -mm perhaps). I think some security folk may be on summer vacation, too. - James -- James Morris <jmorris@namei.org> ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-14 0:11 ` James Morris @ 2011-07-14 1:27 ` NeilBrown 2011-07-14 15:06 ` Solar Designer 2011-07-14 1:30 ` [PATCH] " KOSAKI Motohiro 1 sibling, 1 reply; 41+ messages in thread From: NeilBrown @ 2011-07-14 1:27 UTC (permalink / raw) To: James Morris Cc: Linus Torvalds, Solar Designer, Vasiliy Kulikov, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, kernel-hardening, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Stephen Smalley On Thu, 14 Jul 2011 10:11:57 +1000 (EST) James Morris <jmorris@namei.org> wrote: > On Wed, 13 Jul 2011, Linus Torvalds wrote: > > > It sounds like people are effectively Ack'ing the patch, but with this > > kind of patch I don't want to add the "implicit Ack" that I often do > > for regular stuff. > > > > So could people who think that the patch is ok in its current form > > just send me their acked-by or reviewed-by? I haven't heard any actual > > objection to it, and I think it's valid for the current -rc. > > > > Alternatively, feel free to send a comment like "I think it's the > > right thing, but maybe it should wait for the next merge window".. > > Count me in the latter. > > It does look ok to me, but I'd be happier if it had more testing first (in > -mm perhaps). I think some security folk may be on summer vacation, too. > > > - James I'm still trying to understand the full consequences, but I agree that there is no rush - the code has been this way for quite a while and their is no obvious threat that would expedite things (as far as I know). However I'm not convinced that testing will help all that much - if there are problems they will be is rare corner cases that testing is unlikely to find. The only case where this change will improve safety is where: 1/ a process has RLIMIT_NPROC set 2/ the process is running with root privileges 3/ the process calls setuid() and doesn't handle errors. I think the only times that a root process would have RLIMIT_NPROC set are: 1/ if it explicitly set up rlimits before calling setuid. In this case we should be able to expect that the process checks setuid .. maybe this is naive(?) 2/ if the process was setuid-root and inherited rlimits from before, and never re-set them. In this case it is easy to imagine that a setuid() would not be checked. So maybe an alternate 'fix' would be to reset all rlimits to match init_task when a setuid-root happens. There are other corner cases where inappropriate rlimits can cause setuid programs to behave in ways they don't expect. Obviously such programs are buggy, but so are programs that don't check 'setuid'. (There is a CVE about mount potentially corrupting mtab.) ... maybe that would introduce other problems though. In short: while I don't feel bold enough to 'nack' this patch, I don't really feel comfortable enough to 'ack' it either. NeilBrown ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-14 1:27 ` NeilBrown @ 2011-07-14 15:06 ` Solar Designer 2011-07-15 3:30 ` NeilBrown 0 siblings, 1 reply; 41+ messages in thread From: Solar Designer @ 2011-07-14 15:06 UTC (permalink / raw) To: NeilBrown Cc: James Morris, Linus Torvalds, Vasiliy Kulikov, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, kernel-hardening, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Stephen Smalley, Willy Tarreau, Sebastian Krahmer On Thu, Jul 14, 2011 at 11:27:51AM +1000, NeilBrown wrote: > I'm still trying to understand the full consequences, but I agree that there > is no rush - the code has been this way for quite a while and their is no > obvious threat that would expedite things (as far as I know). I don't insist on getting this in sooner than in the next merge window, although I would have liked that. Relevant userspace vulnerabilities are being found quite often - I'll include some examples below. > However I'm not convinced that testing will help all that much - if there are > problems they will be is rare corner cases that testing is unlikely to find. This makes sense. > The only case where this change will improve safety is where: > 1/ a process has RLIMIT_NPROC set > 2/ the process is running with root privileges > 3/ the process calls setuid() and doesn't handle errors. Yes, and this is a pretty common case. > I think the only times that a root process would have RLIMIT_NPROC set are: > 1/ if it explicitly set up rlimits before calling setuid. In this case > we should be able to expect that the process checks setuid .. maybe > this is naive(?) RLIMIT_NPROC could be set by the parent process or by pam_limits. The machine I am typing this on has: * hard nproc 200 (as well as other limits) in /etc/security/limits.conf, so if this machine's kernel let setuid() fail on RLIMIT_NPROC, I would be taking extra risk of a security compromise by reducing the risk of system crashes from inadvertent excessive resource consumption by runaway processes - a tradeoff I'd rather avoid. > 2/ if the process was setuid-root and inherited rlimits from before, and > never re-set them. In this case it is easy to imagine that a setuid() > would not be checked. Right. (In practice, all kinds of programs tend to forget to check setuid() return value, though.) Actually, for the problem to apply to setuid-root programs, they need to switch their real uid first (more fully become root), then try to switch to a user - but this is common. Here are some examples for 2011-2010: "... missing setuid() retval check in opielogin which leads to easy root compromise": http://www.openwall.com/lists/oss-security/2011/06/22/6 "The /usr/lib/libgnomesu/gnomesu-pam-backend suid binary which belongs to the libgnomesu package is not checking setuid() return values. As a result, two cooperating users, or users with access to guest, cgi or web accounts can run arbitrary commands as root very easily." http://www.openwall.com/lists/oss-security/2011/05/30/2 pam_xauth (exploitable if pam_limits is also used): http://www.openwall.com/lists/oss-security/2010/08/16/2 A collection of examples from 2006: http://lists.openwall.net/linux-kernel/2006/08/20/156 > So maybe an alternate 'fix' would be to reset all rlimits to match init_task > when a setuid-root happens. There are other corner cases where inappropriate > rlimits can cause setuid programs to behave in ways they don't expect. > Obviously such programs are buggy, but so are programs that don't check > 'setuid'. (There is a CVE about mount potentially corrupting mtab.) Right, but to me possibly resetting rlimits is not an "alternative" to moving the RLIMIT_NPROC check. setuid-root exec is not the only case where having setuid() fail on RLIMIT_NPROC is undesirable. We also don't want such failures with pam_limits, nor on daemon startup: http://www.openwall.com/lists/oss-security/2009/07/14/2 As to resetting rlimits on SUID/SGID exec, I think this would make sense for RLIMIT_FSIZE, which would mitigate the mount mtab issue (thank you for bringing it up!) But it's to be discussed separately. Alexander ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 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 0 siblings, 2 replies; 41+ messages in thread From: NeilBrown @ 2011-07-15 3:30 UTC (permalink / raw) To: Solar Designer Cc: James Morris, Linus Torvalds, Vasiliy Kulikov, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, kernel-hardening, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Stephen Smalley, Willy Tarreau, Sebastian Krahmer On Thu, 14 Jul 2011 19:06:02 +0400 Solar Designer <solar@openwall.com> wrote: > On Thu, Jul 14, 2011 at 11:27:51AM +1000, NeilBrown wrote: > > I'm still trying to understand the full consequences, but I agree that there > > is no rush - the code has been this way for quite a while and their is no > > obvious threat that would expedite things (as far as I know). > > I don't insist on getting this in sooner than in the next merge window, > although I would have liked that. Relevant userspace vulnerabilities > are being found quite often - I'll include some examples below. > > > However I'm not convinced that testing will help all that much - if there are > > problems they will be is rare corner cases that testing is unlikely to find. > > This makes sense. > > > The only case where this change will improve safety is where: > > 1/ a process has RLIMIT_NPROC set > > 2/ the process is running with root privileges > > 3/ the process calls setuid() and doesn't handle errors. > > Yes, and this is a pretty common case. > > > I think the only times that a root process would have RLIMIT_NPROC set are: > > 1/ if it explicitly set up rlimits before calling setuid. In this case > > we should be able to expect that the process checks setuid .. maybe > > this is naive(?) > > RLIMIT_NPROC could be set by the parent process or by pam_limits. The > machine I am typing this on has: > > * hard nproc 200 > > (as well as other limits) in /etc/security/limits.conf, so if this > machine's kernel let setuid() fail on RLIMIT_NPROC, I would be taking > extra risk of a security compromise by reducing the risk of system > crashes from inadvertent excessive resource consumption by runaway > processes - a tradeoff I'd rather avoid. > > > 2/ if the process was setuid-root and inherited rlimits from before, and > > never re-set them. In this case it is easy to imagine that a setuid() > > would not be checked. > > Right. (In practice, all kinds of programs tend to forget to check > setuid() return value, though.) > > Actually, for the problem to apply to setuid-root programs, they need to > switch their real uid first (more fully become root), then try to switch > to a user - but this is common. > > Here are some examples for 2011-2010: > > "... missing setuid() retval check in opielogin which leads to easy root > compromise": > > http://www.openwall.com/lists/oss-security/2011/06/22/6 > > "The /usr/lib/libgnomesu/gnomesu-pam-backend suid binary which belongs > to the libgnomesu package is not checking setuid() return values. > > As a result, two cooperating users, or users with access to guest, > cgi or web accounts can run arbitrary commands as root very easily." > > http://www.openwall.com/lists/oss-security/2011/05/30/2 > > pam_xauth (exploitable if pam_limits is also used): > > http://www.openwall.com/lists/oss-security/2010/08/16/2 > > A collection of examples from 2006: > > http://lists.openwall.net/linux-kernel/2006/08/20/156 This is useful background that I think would be good have int the changelog. I'm still bothers that the proposed patch can cause exec to fail for an separate 'innocent' process. It also seems to 'hide' the problem - just shuffling code around. The comment in do_execve_common helps. A similar comment in set_user wouldn't hurt. But what do you think of this. It sure that only the process which ignored the return value from setuid is inconvenienced. ?? NeilBrown commit e47554d38340d4c4c3eccf207de682fe6b29224e Author: NeilBrown <neilb@suse.de> Date: Fri Jul 15 13:20:09 2011 +1000 Protect execve from being used after 'setuid' has failed. Many programs do not check setuid for failure so when it fails they might continue to use elevated privileged without intending to. Of particular concern is a call to 'exec' as this is common after dropping privileges and allows the elevated privileges to be misused. The patch http://lkml.org/lkml/2003/7/13/226 introduced a RLIMIT_NPROC check in set_user() to check for NPROC exceeding via setuid() and similar functions. Before the check there was a possibility to greatly exceed the allowed number of processes by an unprivileged user if the program relied on rlimit only. But the check created new security threat: many poorly written programs simply don't check setuid() return code and believe it cannot fail if executed with root privileges. So, when the check fails we set a process flag which causes execve to fail. Following examples of vulnerabilities due to processes not checking error from setuid provided by Solar Designer <solar@openwall.com>: Here are some examples for 2011-2010: "... missing setuid() retval check in opielogin which leads to easy root compromise": http://www.openwall.com/lists/oss-security/2011/06/22/6 "The /usr/lib/libgnomesu/gnomesu-pam-backend suid binary which belongs to the libgnomesu package is not checking setuid() return values. As a result, two cooperating users, or users with access to guest, cgi or web accounts can run arbitrary commands as root very easily." http://www.openwall.com/lists/oss-security/2011/05/30/2 pam_xauth (exploitable if pam_limits is also used): http://www.openwall.com/lists/oss-security/2010/08/16/2 A collection of examples from 2006: http://lists.openwall.net/linux-kernel/2006/08/20/156 Signed-off-by: NeilBrown <neilb@suse.de> diff --git a/fs/exec.c b/fs/exec.c index 6075a1e..c282ebb 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1434,6 +1434,16 @@ static int do_execve_common(const char *filename, bool clear_in_exec; int retval; + if (current->flags & PF_SETUSER_FAILED) { + /* setuid failed and program is trying to + * 'exec' anyway (rather than e.g. clean up and + * exit), so fail rather than allow a possible + * security breach + */ + retval = -EAGAIN; + goto out_ret; + } + retval = unshare_files(&displaced); if (retval) goto out_ret; diff --git a/include/linux/sched.h b/include/linux/sched.h index 496770a..6b13923 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1759,6 +1759,7 @@ extern void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t * #define PF_DUMPCORE 0x00000200 /* dumped core */ #define PF_SIGNALED 0x00000400 /* killed by a signal */ #define PF_MEMALLOC 0x00000800 /* Allocating memory */ +#define PF_SETUSER_FAILED 0x00001000 /* set_user failed so privs should not be used */ #define PF_USED_MATH 0x00002000 /* if unset the fpu must be initialized before use */ #define PF_FREEZING 0x00004000 /* freeze in progress. do not account to load */ #define PF_NOFREEZE 0x00008000 /* this thread should not be frozen */ diff --git a/kernel/cred.c b/kernel/cred.c index 174fa84..05c46fc 100644 --- a/kernel/cred.c +++ b/kernel/cred.c @@ -508,10 +508,8 @@ int commit_creds(struct cred *new) key_fsgid_changed(task); /* do it - * - What if a process setreuid()'s and this brings the - * new uid over his NPROC rlimit? We can check this now - * cheaply with the new uid cache, so if it matters - * we should be checking for it. -DaveM + * RLIMIT_NPROC limits on user->processes have already been imposed + * in set_user(). */ alter_cred_subscribers(new, 2); if (new->user != old->user) diff --git a/kernel/sys.c b/kernel/sys.c index e4128b2..9b8f03f 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -588,6 +588,8 @@ static int set_user(struct cred *new) struct user_struct *new_user; new_user = alloc_uid(current_user_ns(), new->uid); + current->flags |= PF_SETUSER_FAILED; + if (!new_user) return -EAGAIN; @@ -596,6 +598,7 @@ static int set_user(struct cred *new) free_uid(new_user); return -EAGAIN; } + current->flags &= ~PF_SETUSER_FAILED; free_uid(new->user); new->user = new_user; ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-15 3:30 ` NeilBrown @ 2011-07-15 5:35 ` Willy Tarreau 2011-07-15 6:31 ` [kernel-hardening] " Vasiliy Kulikov 1 sibling, 0 replies; 41+ messages in thread From: Willy Tarreau @ 2011-07-15 5:35 UTC (permalink / raw) To: NeilBrown Cc: Solar Designer, James Morris, Linus Torvalds, Vasiliy Kulikov, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, kernel-hardening, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Stephen Smalley, Sebastian Krahmer Hi Neil, On Fri, Jul 15, 2011 at 01:30:13PM +1000, NeilBrown wrote: (...) > But what do you think of this. It sure that only the process which ignored > the return value from setuid is inconvenienced. (...) I think this is a smart idea. But will the flag be inherited by children over a fork() ? If not, we might as well block fork(), because we can expect a lot of fork+exec situations which are as dangerous as the simple execve(). Regards, Willy ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-15 3:30 ` NeilBrown 2011-07-15 5:35 ` Willy Tarreau @ 2011-07-15 6:31 ` Vasiliy Kulikov 2011-07-15 7:06 ` NeilBrown 1 sibling, 1 reply; 41+ messages in thread From: Vasiliy Kulikov @ 2011-07-15 6:31 UTC (permalink / raw) To: kernel-hardening Cc: Solar Designer, James Morris, Linus Torvalds, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Stephen Smalley, Willy Tarreau, Sebastian Krahmer Neil, On Fri, Jul 15, 2011 at 13:30 +1000, NeilBrown wrote: > I'm still bothers that the proposed patch can cause exec to fail for an > separate 'innocent' process. > It also seems to 'hide' the problem - just shuffling code around. > The comment in do_execve_common helps. A similar comment in set_user > wouldn't hurt. > > But what do you think of this. It sure that only the process which ignored > the return value from setuid is inconvenienced. I don't like it. You're mixing the main problem and an RLIMIT check enforcement. The main goal is denying setuid() to fail unless there is not enough privileges, RLIMIT in execve() is just an *attempt* to still count NPROC in *some* widespread cases. But you're trying to fix setuid() where RLIMIT accounting is simple :\ Your patch doesn't address the core issue in this situation: setuid(); /* it fails because of RLIMIT */ do_some_fs(); execve(); do_some_fs() should be called ONLY if root is dropped. In your scheme the process may interact with FS as root while thinking it is nonroot, which almost always leads to privilege escalation. Thanks, -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-15 6:31 ` [kernel-hardening] " Vasiliy Kulikov @ 2011-07-15 7:06 ` NeilBrown 2011-07-15 7:38 ` Vasiliy Kulikov 0 siblings, 1 reply; 41+ messages in thread From: NeilBrown @ 2011-07-15 7:06 UTC (permalink / raw) To: Vasiliy Kulikov Cc: kernel-hardening, Solar Designer, James Morris, Linus Torvalds, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Stephen Smalley, Willy Tarreau, Sebastian Krahmer On Fri, 15 Jul 2011 10:31:13 +0400 Vasiliy Kulikov <segoon@openwall.com> wrote: > Neil, > > On Fri, Jul 15, 2011 at 13:30 +1000, NeilBrown wrote: > > I'm still bothers that the proposed patch can cause exec to fail for an > > separate 'innocent' process. > > It also seems to 'hide' the problem - just shuffling code around. > > The comment in do_execve_common helps. A similar comment in set_user > > wouldn't hurt. > > > > But what do you think of this. It sure that only the process which ignored > > the return value from setuid is inconvenienced. > > I don't like it. You're mixing the main problem and an RLIMIT check > enforcement. The main goal is denying setuid() to fail unless there is not > enough privileges, RLIMIT in execve() is just an *attempt* to still count > NPROC in *some* widespread cases. But you're trying to fix setuid() > where RLIMIT accounting is simple :\ > > Your patch doesn't address the core issue in this situation: > > setuid(); /* it fails because of RLIMIT */ > do_some_fs(); > execve(); > > do_some_fs() should be called ONLY if root is dropped. In your scheme > the process may interact with FS as root while thinking it is nonroot, > which almost always leads to privilege escalation. > > Thanks, > How about this then? >From 4a1411d10c90510a4eedf94bfd6b2578425271ba Mon Sep 17 00:00:00 2001 From: NeilBrown <neilb@suse.de> Date: Fri, 15 Jul 2011 13:20:09 +1000 Subject: [PATCH] Protect execve from being used after 'setuid' exceeds RLIMIT_NPROC Many programs to not check setuid for failure so it is not safe for it to fail. So impose the RLIMIT_NPROC limit by noting the excess in set_user with a process flag, and failing exec() if the flag is set. The patch http://lkml.org/lkml/2003/7/13/226 introduced a RLIMIT_NPROC check in set_user() to check for NPROC exceeding via setuid() and similar functions. Before the check there was a possibility to greatly exceed the allowed number of processes by an unprivileged user if the program relied on rlimit only. But the check created new security threat: many poorly written programs simply don't check setuid() return code and believe it cannot fail if executed with root privileges. So, when the check fails we set a process flag which causes execve to fail. Following examples of vulnerabilities due to processes not checking error from setuid provided by Solar Designer <solar@openwall.com>: Here are some examples for 2011-2010: "... missing setuid() retval check in opielogin which leads to easy root compromise": http://www.openwall.com/lists/oss-security/2011/06/22/6 "The /usr/lib/libgnomesu/gnomesu-pam-backend suid binary which belongs to the libgnomesu package is not checking setuid() return values. As a result, two cooperating users, or users with access to guest, cgi or web accounts can run arbitrary commands as root very easily." http://www.openwall.com/lists/oss-security/2011/05/30/2 pam_xauth (exploitable if pam_limits is also used): http://www.openwall.com/lists/oss-security/2010/08/16/2 A collection of examples from 2006: http://lists.openwall.net/linux-kernel/2006/08/20/156 Signed-off-by: NeilBrown <neilb@suse.de> diff --git a/fs/exec.c b/fs/exec.c index 6075a1e..dfe9c61 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1434,6 +1434,17 @@ static int do_execve_common(const char *filename, bool clear_in_exec; int retval; + if (current->flags & PF_NPROC_EXCEEDED) { + /* setuid noticed that RLIMIT_NPROC was + * exceeded, but didn't fail as that easily + * leads to privileges leaking. So fail + * here instead - we still limit the number of + * processes running the user's code. + */ + retval = -EAGAIN; + goto out_ret; + } + retval = unshare_files(&displaced); if (retval) goto out_ret; diff --git a/include/linux/sched.h b/include/linux/sched.h index 496770a..f024c63 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1759,6 +1759,7 @@ extern void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t * #define PF_DUMPCORE 0x00000200 /* dumped core */ #define PF_SIGNALED 0x00000400 /* killed by a signal */ #define PF_MEMALLOC 0x00000800 /* Allocating memory */ +#define PF_NPROC_EXCEEDED 0x00001000 /* set_user noticed that RLIMIT_NPROC was exceeded */ #define PF_USED_MATH 0x00002000 /* if unset the fpu must be initialized before use */ #define PF_FREEZING 0x00004000 /* freeze in progress. do not account to load */ #define PF_NOFREEZE 0x00008000 /* this thread should not be frozen */ diff --git a/kernel/cred.c b/kernel/cred.c index 174fa84..8ef31f5 100644 --- a/kernel/cred.c +++ b/kernel/cred.c @@ -508,10 +508,8 @@ int commit_creds(struct cred *new) key_fsgid_changed(task); /* do it - * - What if a process setreuid()'s and this brings the - * new uid over his NPROC rlimit? We can check this now - * cheaply with the new uid cache, so if it matters - * we should be checking for it. -DaveM + * RLIMIT_NPROC limits on user->processes have already been checked + * in set_user(). */ alter_cred_subscribers(new, 2); if (new->user != old->user) diff --git a/kernel/sys.c b/kernel/sys.c index e4128b2..dd1fb9d 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -592,10 +592,9 @@ static int set_user(struct cred *new) return -EAGAIN; if (atomic_read(&new_user->processes) >= rlimit(RLIMIT_NPROC) && - new_user != INIT_USER) { - free_uid(new_user); - return -EAGAIN; - } + new_user != INIT_USER) + /* Cause any subsequent 'exec' to fail */ + current->flags |= PF_NPROC_EXCEEDED; free_uid(new->user); new->user = new_user; ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 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 0 siblings, 2 replies; 41+ messages in thread From: Vasiliy Kulikov @ 2011-07-15 7:38 UTC (permalink / raw) To: NeilBrown Cc: kernel-hardening, Solar Designer, James Morris, Linus Torvalds, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Stephen Smalley, Willy Tarreau, Sebastian Krahmer Neil, On Fri, Jul 15, 2011 at 17:06 +1000, NeilBrown wrote: > How about this then? AFAIU, with this patch: 1) setuid() doesn't fail in NPROC exceed case. 2) NPROC is forced on execve() after setuid(). 3) execve() fails only if NPROC was exceeded during setuid() execution. 4) Other processes of the same user doesn't suffer from "occasional" execve() failures. If it is correct, then I like the patch :) It does RLIMIT_NPROC enforcement without mixing other execve() calls like -ow patch did. See minor suggestions about the comments below. > Signed-off-by: NeilBrown <neilb@suse.de> Acked-by: Vasiliy Kulikov <segoon@openwall.com> > diff --git a/fs/exec.c b/fs/exec.c > index 6075a1e..dfe9c61 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -1434,6 +1434,17 @@ static int do_execve_common(const char *filename, > bool clear_in_exec; > int retval; > > + if (current->flags & PF_NPROC_EXCEEDED) { > + /* setuid noticed that RLIMIT_NPROC was > + * exceeded, but didn't fail as that easily > + * leads to privileges leaking. It doesn't lead to privileges leaking as is, it is a threat of buggy programs not checking return code of syscalls dropping privileges (setuid here). > So fail > + * here instead - we still limit the number of > + * processes running the user's code. > + */ > + retval = -EAGAIN; > + goto out_ret; > + } > + > retval = unshare_files(&displaced); > if (retval) > goto out_ret; > diff --git a/include/linux/sched.h b/include/linux/sched.h > index 496770a..f024c63 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -1759,6 +1759,7 @@ extern void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t * > #define PF_DUMPCORE 0x00000200 /* dumped core */ > #define PF_SIGNALED 0x00000400 /* killed by a signal */ > #define PF_MEMALLOC 0x00000800 /* Allocating memory */ > +#define PF_NPROC_EXCEEDED 0x00001000 /* set_user noticed that RLIMIT_NPROC was exceeded */ > #define PF_USED_MATH 0x00002000 /* if unset the fpu must be initialized before use */ > #define PF_FREEZING 0x00004000 /* freeze in progress. do not account to load */ > #define PF_NOFREEZE 0x00008000 /* this thread should not be frozen */ > diff --git a/kernel/cred.c b/kernel/cred.c > index 174fa84..8ef31f5 100644 > --- a/kernel/cred.c > +++ b/kernel/cred.c > @@ -508,10 +508,8 @@ int commit_creds(struct cred *new) > key_fsgid_changed(task); > > /* do it > - * - What if a process setreuid()'s and this brings the > - * new uid over his NPROC rlimit? We can check this now > - * cheaply with the new uid cache, so if it matters > - * we should be checking for it. -DaveM > + * RLIMIT_NPROC limits on user->processes have already been checked > + * in set_user(). > */ > alter_cred_subscribers(new, 2); > if (new->user != old->user) > diff --git a/kernel/sys.c b/kernel/sys.c > index e4128b2..dd1fb9d 100644 > --- a/kernel/sys.c > +++ b/kernel/sys.c > @@ -592,10 +592,9 @@ static int set_user(struct cred *new) > return -EAGAIN; > > if (atomic_read(&new_user->processes) >= rlimit(RLIMIT_NPROC) && > - new_user != INIT_USER) { > - free_uid(new_user); > - return -EAGAIN; > - } > + new_user != INIT_USER) > + /* Cause any subsequent 'exec' to fail */ The comment about why all this bustle is needed is better to put here because the check is logically belongs to set_user(). Specifically, I'd mention that (1) we don't want setuid() to fail while having enough privileges and (2) RLIMIT_NPROC can be still harmlessly checked for programs doing setuid()+execve() if the error code is deferred to the execve stage. > + current->flags |= PF_NPROC_EXCEEDED; > > free_uid(new->user); > new->user = new_user; Thanks, -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-15 7:38 ` Vasiliy Kulikov @ 2011-07-15 13:04 ` Solar Designer 2011-07-15 13:58 ` [kernel-hardening] " Stephen Smalley 1 sibling, 0 replies; 41+ messages in thread From: Solar Designer @ 2011-07-15 13:04 UTC (permalink / raw) To: Vasiliy Kulikov Cc: NeilBrown, kernel-hardening, James Morris, Linus Torvalds, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Stephen Smalley, Willy Tarreau, Sebastian Krahmer Neil, Vasiliy - On Fri, Jul 15, 2011 at 11:38:23AM +0400, Vasiliy Kulikov wrote: > On Fri, Jul 15, 2011 at 17:06 +1000, NeilBrown wrote: > > How about this then? > > AFAIU, with this patch: > > 1) setuid() doesn't fail in NPROC exceed case. > 2) NPROC is forced on execve() after setuid(). > 3) execve() fails only if NPROC was exceeded during setuid() execution. > 4) Other processes of the same user doesn't suffer from "occasional" > execve() failures. > > If it is correct, then I like the patch :) I'm not convinced this has any advantages over the patch Vasiliy had posted (which simply moved the RLIMIT_NPROC check), but I don't mind, with one important correction: Although in the primary use cases we're considering, setuid() is very soon followed by execve(), this doesn't always have to be the case. There may be cases where the process would sleep between these two calls (for a variety of reasons). It would be surprising to see a process fail on execve() because of RLIMIT_NPROC when that limit had been reached, say, days ago and is no longer reached at the time of execve(). Thus, if we go with Neil's proposal (adding/checking an extra flag), we should also re-check the process count against RLIMIT_NPROC on execve(), and fail the exec only when both the flag is set and the current process count is excessive (still or again). Also, if we go with a patch like this, it brings up the question of whether the flag should be preserved or reset on fork(). I think it should be preserved. > It does RLIMIT_NPROC > enforcement without mixing other execve() calls like -ow patch did. "Mixing other execve() calls" (4 on the list above) is not obviously undesirable. Thus, either Vasiliy's original patch or Neil's patch with the addition mentioned above would be fine by me. Thanks, Alexander ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-15 7:38 ` Vasiliy Kulikov 2011-07-15 13:04 ` Solar Designer @ 2011-07-15 13:58 ` Stephen Smalley 2011-07-15 15:26 ` Vasiliy Kulikov 1 sibling, 1 reply; 41+ messages in thread From: Stephen Smalley @ 2011-07-15 13:58 UTC (permalink / raw) To: Vasiliy Kulikov Cc: NeilBrown, kernel-hardening, Solar Designer, James Morris, Linus Torvalds, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer On Fri, 2011-07-15 at 11:38 +0400, Vasiliy Kulikov wrote: > Neil, > > On Fri, Jul 15, 2011 at 17:06 +1000, NeilBrown wrote: > > How about this then? > > AFAIU, with this patch: > > 1) setuid() doesn't fail in NPROC exceed case. > 2) NPROC is forced on execve() after setuid(). > 3) execve() fails only if NPROC was exceeded during setuid() execution. > 4) Other processes of the same user doesn't suffer from "occasional" > execve() failures. > > If it is correct, then I like the patch :) It does RLIMIT_NPROC > enforcement without mixing other execve() calls like -ow patch did. Does this have implications for Android's zygote model? There you have a long running uid 0 / all caps process (the zygote), which forks itself upon receiving a request to spawn an app and then calls setgroups(); setrlimit(); setgid(); setuid(); assuming the limits and credentials of the app but never does an exec at all, as it is just loading the app's class and executing it from memory. Also, can't setuid() fail under other conditions, e.g. ENOMEM upon prepare_creds() allocation failure? Is it ever reasonable for a program to not check setuid() for failure? Certainly there are plenty of examples of programs not doing that, but it isn't clear that this is a bug in the kernel. -- Stephen Smalley National Security Agency ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-15 13:58 ` [kernel-hardening] " Stephen Smalley @ 2011-07-15 15:26 ` Vasiliy Kulikov 2011-07-15 19:54 ` Stephen Smalley 0 siblings, 1 reply; 41+ messages in thread From: Vasiliy Kulikov @ 2011-07-15 15:26 UTC (permalink / raw) To: Stephen Smalley Cc: NeilBrown, kernel-hardening, Solar Designer, James Morris, Linus Torvalds, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer Hi Stephen, On Fri, Jul 15, 2011 at 09:58 -0400, Stephen Smalley wrote: > Does this have implications for Android's zygote model? There you have > a long running uid 0 / all caps process (the zygote), which forks itself > upon receiving a request to spawn an app and then calls > setgroups(); > setrlimit(); setgid(); setuid(); Is RLIMIT_NPROC forced in your model and setuid() is expected to fail because of NPROC exceeding? If no, then it is not touched at all. > Also, can't setuid() fail under other conditions, e.g. ENOMEM upon > prepare_creds() allocation failure? Is it ever reasonable for a program > to not check setuid() for failure? Certainly there are plenty of > examples of programs not doing that, but it isn't clear that this is a > bug in the kernel. This thing is better to discuss separately, after the patch is applied. Shorter, in current implementation it is not possible as mm layer tries to alloc small structures (less than 8 pages) indefinitely. cred and user_struct are less than 64kb, so ENOMEM is impossible. Thanks, -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-15 15:26 ` Vasiliy Kulikov @ 2011-07-15 19:54 ` Stephen Smalley 2011-07-21 4:09 ` NeilBrown 0 siblings, 1 reply; 41+ messages in thread From: Stephen Smalley @ 2011-07-15 19:54 UTC (permalink / raw) To: Vasiliy Kulikov Cc: NeilBrown, kernel-hardening, Solar Designer, James Morris, Linus Torvalds, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer On Fri, 2011-07-15 at 19:26 +0400, Vasiliy Kulikov wrote: > Hi Stephen, > > On Fri, Jul 15, 2011 at 09:58 -0400, Stephen Smalley wrote: > > Does this have implications for Android's zygote model? There you have > > a long running uid 0 / all caps process (the zygote), which forks itself > > upon receiving a request to spawn an app and then calls > > > setgroups(); > > setrlimit(); setgid(); setuid(); > > Is RLIMIT_NPROC forced in your model and setuid() is expected to fail > because of NPROC exceeding? If no, then it is not touched at all. I don't know what their intent is. But it is an example of a case where moving the enforcement from setuid() to a subsequent execve() causes the check to never get applied. As to whether or not they care, I don't know. An app that calls fork() repeatedly will still be stopped, but an app that repeatedly connects to the zygote and asks to spawn another instance of itself would be unlimited. OTOH, the current RLIMIT_NPROC check and lack of setuid() error checking has been a repeated issue for Android. -- Stephen Smalley National Security Agency ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-15 19:54 ` Stephen Smalley @ 2011-07-21 4:09 ` NeilBrown 2011-07-21 12:48 ` Solar Designer 0 siblings, 1 reply; 41+ messages in thread From: NeilBrown @ 2011-07-21 4:09 UTC (permalink / raw) To: Stephen Smalley Cc: Vasiliy Kulikov, kernel-hardening, Solar Designer, James Morris, Linus Torvalds, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer On Fri, 15 Jul 2011 15:54:43 -0400 Stephen Smalley <sds@tycho.nsa.gov> wrote: > On Fri, 2011-07-15 at 19:26 +0400, Vasiliy Kulikov wrote: > > Hi Stephen, > > > > On Fri, Jul 15, 2011 at 09:58 -0400, Stephen Smalley wrote: > > > Does this have implications for Android's zygote model? There you have > > > a long running uid 0 / all caps process (the zygote), which forks itself > > > upon receiving a request to spawn an app and then calls > > > > > setgroups(); > > > setrlimit(); setgid(); setuid(); > > > > Is RLIMIT_NPROC forced in your model and setuid() is expected to fail > > because of NPROC exceeding? If no, then it is not touched at all. > > I don't know what their intent is. But it is an example of a case where > moving the enforcement from setuid() to a subsequent execve() causes the > check to never get applied. As to whether or not they care, I don't > know. An app that calls fork() repeatedly will still be stopped, but an > app that repeatedly connects to the zygote and asks to spawn another > instance of itself would be unlimited. > > OTOH, the current RLIMIT_NPROC check and lack of setuid() error checking > has been a repeated issue for Android. > So where does this leave us? Between a rock and a hard place? It says to me that moving the check from set_user to execve is simply Wrong(TM). It may be convenient and do TheRightThing in certain common cases, but it also can do the Wrong thing in other cases and I don't think that is an acceptable trade off. Having setuid succeed when it should fail is simply incorrect. The problem - as we all know - is that user space doesn't always check error status properly. If we were to look for precedent I would point to SIGPIPE. The only reason for that to exist is because programs don't always check that a "write" succeeds so we have a mechanism to kill off processes that don't check the error status and keep sending. I would really like to apply that to this problem ... but that has already been suggested (years ago) and found wanting. Maybe we can revisit it? The value of the SIGPIPE approach (whether it is SIGPIPE or SIGXCPU or SIGVEC ... if only there were a SIGXNPROC) is that the program remains in complete control. It can find out if the NPROC limit has been exceeded at the "right" time. The only other solution that I can think of which isn't "Wrong(TM)" is my first patch which introduced PF_SETUSER_FAILED. With this patch setuid() still fails if it should, so the calling process still remains in control. But if it fails to exercise that control, the kernel steps in. Vasiliy didn't like that because it allows a process to ignore the setuid failure, perform some in-process activity as root when expecting it to be as some-other-user, and only fails when execve is attempted - possibly too late. Against this I ask: what exactly is our goal here? Is it to stop all possible abuses? I think not. That is impossible. Is it to stop certain known and commonly repeated errors? I think so. That is clearly valuable. We know (Thanks to Solar Designer's list) that unchecked setuid followed by execve is a commonly repeated error, so trapping that can be justified. Do we know that accessing the filesystem first is a commonly repeated error? If not, there is no clear motive to deal with that problem. If, however, it is then maybe a check for PF_SETUSER_FAILED in inode_permission() would be the right thing. Or maybe we just set PF_SETUSER_FAILED and leave it up to some security module to decide what to disable or report when that is set? In short: I don't think there can be a solution that is both completely correct and completely safe. I would go for "as correct as possible" with "closes common vulnerabilities". NeilBrown ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-21 4:09 ` NeilBrown @ 2011-07-21 12:48 ` Solar Designer 2011-07-21 18:21 ` Linus Torvalds 0 siblings, 1 reply; 41+ messages in thread From: Solar Designer @ 2011-07-21 12:48 UTC (permalink / raw) To: NeilBrown Cc: Stephen Smalley, Vasiliy Kulikov, kernel-hardening, James Morris, Linus Torvalds, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer On Thu, Jul 21, 2011 at 02:09:36PM +1000, NeilBrown wrote: > On Fri, 15 Jul 2011 15:54:43 -0400 Stephen Smalley <sds@tycho.nsa.gov> wrote: > > On Fri, 2011-07-15 at 19:26 +0400, Vasiliy Kulikov wrote: > > > On Fri, Jul 15, 2011 at 09:58 -0400, Stephen Smalley wrote: > > > > Does this have implications for Android's zygote model? There you have > > > > a long running uid 0 / all caps process (the zygote), which forks itself > > > > upon receiving a request to spawn an app and then calls > > > > > > > setgroups(); > > > > setrlimit(); setgid(); setuid(); > > > > > > Is RLIMIT_NPROC forced in your model and setuid() is expected to fail > > > because of NPROC exceeding? If no, then it is not touched at all. > > > > I don't know what their intent is. But it is an example of a case where > > moving the enforcement from setuid() to a subsequent execve() causes the > > check to never get applied. As to whether or not they care, I don't > > know. An app that calls fork() repeatedly will still be stopped, but an > > app that repeatedly connects to the zygote and asks to spawn another > > instance of itself would be unlimited. > > > > OTOH, the current RLIMIT_NPROC check and lack of setuid() error checking > > has been a repeated issue for Android. > > So where does this leave us? Between a rock and a hard place? Maybe. I just took a look at Android's Zygote code, as found via Google Code Search. This appears to be the relevant place: http://www.google.com/codesearch#atE6BTe41-M/vm/native/dalvik_system_Zygote.c&q=forkAndSpecialize&type=cs&l=439 As we can see, Stephen's description of the sequence of calls is indeed correct. Also, the return value from setuid() is checked here. :-) As to which rlimits are actually set, I don't know. This appears to be configured at a much higher level: http://www.google.com/codesearch#uX1GffpyOZk/core/java/com/android/internal/os/ZygoteConnection.java&q=ZygoteConnection.java&type=cs&l=247 --rlimit=r,c,m tuple of values for setrlimit() call. I have no idea whether this --rlimit argument is ever supplied and with what settings. The intent of supporting it could well be other than making use of RLIMIT_NPROC specifically. > It says to me that moving the check from set_user to execve is simply > Wrong(TM). It may be convenient and do TheRightThing in certain common > cases, but it also can do the Wrong thing in other cases and I don't think > that is an acceptable trade off. I disagree. There's nothing wrong in having the check on execve(), especially not if we combine it with a check of your proposed flag set on setuid() (only fail execve() when the flag is set and RLIMIT_NPROC is still or again exceeded). > Having setuid succeed when it should fail is simply incorrect. As far as I'm aware, no standard says that setuid() should fail if it would exceed RLIMIT_NPROC for the target user. There's a notion of "appropriate privileges", but what these are is implementation-defined and it was hardly meant to include rlimits. > The problem - as we all know - is that user space doesn't always check error > status properly. If we were to look for precedent I would point to SIGPIPE. > The only reason for that to exist is because programs don't always check that > a "write" succeeds so we have a mechanism to kill off processes that don't > check the error status and keep sending. > > I would really like to apply that to this problem ... but that has already > been suggested (years ago) and found wanting. Maybe we can revisit it? > > The value of the SIGPIPE approach (whether it is SIGPIPE or SIGXCPU or > SIGVEC ... if only there were a SIGXNPROC) is that the program remains in > complete control. It can find out if the NPROC limit has been exceeded at > the "right" time. I don't mind having setuid() signal (and by default actually kill) a process if it would exceed RLIMIT_NPROC. However, I doubt that others will agree. BTW, as I told Vasiliy on the kernel-hardening list, I think we should revisit the "can't happen" memory allocation failure in set_user() _after_ we have dealt with the RLIMIT_NPROC issue. I would support the killing of process with SIGKILL or SIGSEGV (as opposed to return -EAGAIN) on that "can't happen" condition (which might become possible in a further revision of the code, so better safe than sorry). Let's actually revisit this later, after having the most important fix applied. > The only other solution that I can think of which isn't "Wrong(TM)" is my > first patch which introduced PF_SETUSER_FAILED. > With this patch setuid() still fails if it should, so the calling process > still remains in control. But if it fails to exercise that control, the > kernel steps in. > > Vasiliy didn't like that because it allows a process to ignore the setuid > failure, perform some in-process activity as root when expecting it to be as > some-other-user, and only fails when execve is attempted - possibly too late. I am with Vasiliy on this. > Against this I ask: what exactly is our goal here? > Is it to stop all possible abuses? I think not. That is impossible. > Is it to stop certain known and commonly repeated errors? I think so. That > is clearly valuable. Not checking the return value from setuid() and proceeding to do other work is a known and commonly repeated error. As to whether it is also common for that other work to involve risky syscalls other than execve(), I expect that it is, although I did not research this. > We know (Thanks to Solar Designer's list) that unchecked setuid followed by > execve is a commonly repeated error, so trapping that can be justified. > Do we know that accessing the filesystem first is a commonly repeated error? > If not, there is no clear motive to deal with that problem. > If, however, it is then maybe a check for PF_SETUSER_FAILED in > inode_permission() would be the right thing. > > Or maybe we just set PF_SETUSER_FAILED and leave it up to some security > module to decide what to disable or report when that is set? I feel that we'd be inventing something more complicated yet worse than simply moving the check would be. Here's my current proposal: 1. Apply Vasiliy's patch to move the RLIMIT_NPROC check from setuid() to execve(), optionally enhanced with setting PF_SETUSER_FAILED on would-be-failed setuid() and checking this flag in execve() (in addition to repeating the RLIMIT_NPROC check). 2. With a separate patch, add a prctl() to read the PF_SETUSER_FAILED flag. Android will be able to use this if it wants to. Yes, this might break RLIMIT_NPROC for Android (I wrote "might" because I have no idea if it actually sets that specific limit or not) until it learns to use the new prctl(). But I think that's fine, and it is not a reason for us to introduce more complexity into the kernel, yet make our security hardening change more limited. There was never a guarantee (in any standard or piece of documentation) that setuid() would fail on exceeding RLIMIT_NPROC, and the Android/Zygote code might not actually rely on that anyway (there's no clear indication that it does; RLIMIT_NPROC is not in the code, nor is it mentioned in any comment). > In short: I don't think there can be a solution that is both completely > correct and completely safe. I would go for "as correct as possible" with > "closes common vulnerabilities". Maybe, and if so I think that one I proposed above falls in this category as well, but it closes more vulnerabilities (and/or does so more fully). Thanks, Alexander ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-21 12:48 ` Solar Designer @ 2011-07-21 18:21 ` Linus Torvalds 2011-07-21 19:39 ` [kernel-hardening] " Solar Designer 0 siblings, 1 reply; 41+ messages in thread From: Linus Torvalds @ 2011-07-21 18:21 UTC (permalink / raw) To: Solar Designer Cc: NeilBrown, Stephen Smalley, Vasiliy Kulikov, kernel-hardening, James Morris, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer On Thu, Jul 21, 2011 at 5:48 AM, Solar Designer <solar@openwall.com> wrote: > > Maybe, and if so I think that one I proposed above falls in this > category as well, but it closes more vulnerabilities (and/or does so > more fully). I think we could have a pretty simple approach that "works in practice": retain the check at setuid() time, but make it a higher limit. IOW, the logic is that we have two competing pressures: (a) we should try to avoid failing on setuid(), because there is a real risk that the setuid caller doesn't really check the failure case and opens itself up for a security problem and (b) never failing setuid at all is in itself a security problem, since it can lead to DoS attacks in the form of excessive resource use by one user. But the sane (intelligent) solution to that is to say that we *PREFER* to fail in execve(), but that at some point a failure in setuid() is preferable to no failure at all. After all, we have no hard knowledge that there is any actual setuid() issue. Neither generally does the user (iow, look at this whole discussion where intelligent people simply have different inputs depending on "what could happen"). So it really seems like the natural approach would be to simply fail *earlier* on execve() and fork(). That will catch most cases, and has no downsides. But if we notice that we are in a situation where some privileged user can be tricked into forking a lot and doing setuid(), then at that point the setuid() path becomes relevant. IOW, I'd suggest simply making the rule be that "setuid() allows 10% more users than the limit technically says". It's not a guarantee, but it means that in order to hit the problem, you need to have *both* a setuid application that allows unconstrained user forking *and* doesn't check the setuid() return value. Put another way: a user cannot force the "we're at the edge of the setuid() limit" on its own by just forking - the user will be stopped 10% before the setuid() failure case can ever trigger. Is this some "guarantee of nothing bad can ever happen"? No. If you have bad setuid applications, you will have problems. But it's a "you need to really work harder at it and you need to find more things to go wrong", which is after all what real security is all about. No? Linus ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-21 18:21 ` Linus Torvalds @ 2011-07-21 19:39 ` Solar Designer 2011-07-25 17:14 ` Vasiliy Kulikov 0 siblings, 1 reply; 41+ messages in thread From: Solar Designer @ 2011-07-21 19:39 UTC (permalink / raw) To: Linus Torvalds Cc: NeilBrown, Stephen Smalley, Vasiliy Kulikov, kernel-hardening, James Morris, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer On Thu, Jul 21, 2011 at 11:21:07AM -0700, Linus Torvalds wrote: > I think we could have a pretty simple approach that "works in > practice": retain the check at setuid() time, but make it a higher > limit. > > IOW, the logic is that we have two competing pressures: > > (a) we should try to avoid failing on setuid(), because there is a > real risk that the setuid caller doesn't really check the failure case > and opens itself up for a security problem > > and > > (b) never failing setuid at all is in itself a security problem, > since it can lead to DoS attacks in the form of excessive resource use > by one user. I don't recall anyone stating (b) the way you did above (or sufficiently similar). I wouldn't consider setuid() never failing to be a security problem. BTW, some people consider setuid() failing on RLIMIT_NPROC kernel "brokenness", which applications have to "work around": http://www.openwall.com/lists/musl/2011/07/21/3 "I'm aware of course that some interfaces *can* fail for nonstandard reasons under Linux (dup2 and set*uid come to mind), and I've tried to work around these and shield applications from the brokenness..." So opinions on setuid() failing vary, whereas (a) is clear - there have been vulnerabilities caused by setuid() failing. The DoS that you mention doesn't necessarily have to be dealt with by setuid() failing on RLIMIT_NPROC (nor on a higher limit). It could also be dealt with by checking the limit on execve(), like we've been doing on shared web hosting servers for years, and, if desired, by letting applications like Android/Zygote check for the condition themselves via a new prctl() (or they can simply pass an extra fork(), although that's a bit costly). > IOW, I'd suggest simply making the rule be that "setuid() allows 10% > more users than the limit technically says". It's not a guarantee, but > it means that in order to hit the problem, you need to have *both* a > setuid application that allows unconstrained user forking *and* > doesn't check the setuid() return value. > > Put another way: a user cannot force the "we're at the edge of the > setuid() limit" on its own by just forking - the user will be stopped > 10% before the setuid() failure case can ever trigger. For a malicious user, this merely adds the task of triggering a race condition - have a sufficient number of processes accumulate in the between setuid() and execve() state. If the program in question can be made to sleep, this may be trivial to do. Otherwise, it may require very rapid requests (automated) and high system load. (BTW, 10% of 0 would be 0, which would allow for attacks that are as simple as they're now, but that's an implementation detail. Of course, you'd actually add some constant as well.) > Is this some "guarantee of nothing bad can ever happen"? No. If you > have bad setuid applications, you will have problems. But it's a "you > need to really work harder at it and you need to find more things to > go wrong", which is after all what real security is all about. > > No? I generally support having multiple layers of security even if some are non-perfect, but in this case we have a problem that we can _fully_ deal with rather than merely make attacks harder. So my proposal remains to go with Vasiliy's trivial patch and maybe add a few things on top of it as I mentioned in my previous message. Thanks, Alexander ^ permalink raw reply [flat|nested] 41+ messages in thread
* [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 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 0 siblings, 1 reply; 41+ messages in thread From: Vasiliy Kulikov @ 2011-07-25 17:14 UTC (permalink / raw) To: Linus Torvalds Cc: NeilBrown, Stephen Smalley, kernel-hardening, James Morris, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer The patch http://lkml.org/lkml/2003/7/13/226 introduced an RLIMIT_NPROC check in set_user() to check for NPROC exceeding via setuid() and similar functions. Before the check there was a possibility to greatly exceed the allowed number of processes by an unprivileged user if the program relied on rlimit only. But the check created new security threat: many poorly written programs simply don't check setuid() return code and believe it cannot fail if executed with root privileges. So, the check is removed in this patch because of too often privilege escalations related to buggy programs. The NPROC can still be enforced in the common code flow of daemons spawning user processes. Most of daemons do fork()+setuid()+execve(). The check introduced in execve() (1) enforces the same limit as in setuid() and (2) doesn't create similar security issues. Neil Brown suggested to track what specific process has exceeded the limit by setting PF_NPROC_EXCEEDED process flag. With the change only this process would fail on execve(), and other processes' execve() behaviour is not changed. Solar Designer suggested to re-check whether NPROC limit is still exceeded at the moment of execve(). If the process was sleeping for days between set*uid() and execve(), and the NPROC counter step down under the limit, the deferred execve() failure because NPROC limit was exceeded days ago would be unexpected. Similar check was introduced in -ow patches (without the process flag). Signed-off-by: Vasiliy Kulikov <segoon@openwall.com> --- fs/exec.c | 13 +++++++++++++ include/linux/sched.h | 1 + kernel/cred.c | 7 +++---- kernel/sys.c | 13 +++++++++---- 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 6075a1e..706a213 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1433,6 +1433,19 @@ static int do_execve_common(const char *filename, struct files_struct *displaced; bool clear_in_exec; int retval; + const struct cred *cred = current_cred(); + + /* + * We move the actual failure in case of RLIMIT_NPROC excess from + * set*uid() to execve() because too many poorly written programs + * don't check setuid() return code. Here we additionally recheck + * whether NPROC limit is still exceeded. + */ + if ((current->flags & PF_NPROC_EXCEEDED) && + atomic_read(&cred->user->processes) > rlimit(RLIMIT_NPROC)) { + retval = -EAGAIN; + goto out_ret; + } retval = unshare_files(&displaced); if (retval) diff --git a/include/linux/sched.h b/include/linux/sched.h index 496770a..f024c63 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1759,6 +1759,7 @@ extern void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t * #define PF_DUMPCORE 0x00000200 /* dumped core */ #define PF_SIGNALED 0x00000400 /* killed by a signal */ #define PF_MEMALLOC 0x00000800 /* Allocating memory */ +#define PF_NPROC_EXCEEDED 0x00001000 /* set_user noticed that RLIMIT_NPROC was exceeded */ #define PF_USED_MATH 0x00002000 /* if unset the fpu must be initialized before use */ #define PF_FREEZING 0x00004000 /* freeze in progress. do not account to load */ #define PF_NOFREEZE 0x00008000 /* this thread should not be frozen */ diff --git a/kernel/cred.c b/kernel/cred.c index 174fa84..52f4342 100644 --- a/kernel/cred.c +++ b/kernel/cred.c @@ -508,11 +508,10 @@ int commit_creds(struct cred *new) key_fsgid_changed(task); /* do it - * - What if a process setreuid()'s and this brings the - * new uid over his NPROC rlimit? We can check this now - * cheaply with the new uid cache, so if it matters - * we should be checking for it. -DaveM + * RLIMIT_NPROC limits on user->processes have already been checked + * in set_user(). */ + alter_cred_subscribers(new, 2); if (new->user != old->user) atomic_inc(&new->user->processes); diff --git a/kernel/sys.c b/kernel/sys.c index e4128b2..fc40cbc 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -591,11 +591,16 @@ static int set_user(struct cred *new) if (!new_user) return -EAGAIN; + /* + * We don't fail in case of NPROC limit excess here because too many + * poorly written programs don't check set*uid() return code, assuming + * it never fails if called by root. We may still enforce NPROC limit + * for programs doing set*uid()+execve() by harmlessly deferring the + * failure to the execve() stage. + */ if (atomic_read(&new_user->processes) >= rlimit(RLIMIT_NPROC) && - new_user != INIT_USER) { - free_uid(new_user); - return -EAGAIN; - } + new_user != INIT_USER) + current->flags |= PF_NPROC_EXCEEDED; free_uid(new->user); new->user = new_user; -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-25 17:14 ` Vasiliy Kulikov @ 2011-07-25 23:40 ` Solar Designer 2011-07-26 0:47 ` NeilBrown 0 siblings, 1 reply; 41+ messages in thread From: Solar Designer @ 2011-07-25 23:40 UTC (permalink / raw) To: Vasiliy Kulikov Cc: Linus Torvalds, NeilBrown, Stephen Smalley, kernel-hardening, James Morris, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer Vasiliy, On Mon, Jul 25, 2011 at 09:14:23PM +0400, Vasiliy Kulikov wrote: > @@ -1433,6 +1433,19 @@ static int do_execve_common(const char *filename, > struct files_struct *displaced; > bool clear_in_exec; > int retval; > + const struct cred *cred = current_cred(); > + > + /* > + * We move the actual failure in case of RLIMIT_NPROC excess from > + * set*uid() to execve() because too many poorly written programs > + * don't check setuid() return code. Here we additionally recheck > + * whether NPROC limit is still exceeded. > + */ > + if ((current->flags & PF_NPROC_EXCEEDED) && > + atomic_read(&cred->user->processes) > rlimit(RLIMIT_NPROC)) { > + retval = -EAGAIN; > + goto out_ret; > + } Do you possibly need: current->flags &= ~PF_NPROC_EXCEEDED; somewhere after this point? I think it's weird to have past set_user() failure affect other than the very next execve(). Perhaps also reset the flag on fork() because we have an RLIMIT_NPROC check on fork() anyway. Thanks, Alexander ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-25 23:40 ` [kernel-hardening] " Solar Designer @ 2011-07-26 0:47 ` NeilBrown 2011-07-26 1:16 ` Solar Designer 0 siblings, 1 reply; 41+ messages in thread From: NeilBrown @ 2011-07-26 0:47 UTC (permalink / raw) To: Solar Designer Cc: Vasiliy Kulikov, Linus Torvalds, Stephen Smalley, kernel-hardening, James Morris, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer On Tue, 26 Jul 2011 03:40:13 +0400 Solar Designer <solar@openwall.com> wrote: > Vasiliy, > > On Mon, Jul 25, 2011 at 09:14:23PM +0400, Vasiliy Kulikov wrote: > > @@ -1433,6 +1433,19 @@ static int do_execve_common(const char *filename, > > struct files_struct *displaced; > > bool clear_in_exec; > > int retval; > > + const struct cred *cred = current_cred(); > > + > > + /* > > + * We move the actual failure in case of RLIMIT_NPROC excess from > > + * set*uid() to execve() because too many poorly written programs > > + * don't check setuid() return code. Here we additionally recheck > > + * whether NPROC limit is still exceeded. > > + */ > > + if ((current->flags & PF_NPROC_EXCEEDED) && > > + atomic_read(&cred->user->processes) > rlimit(RLIMIT_NPROC)) { > > + retval = -EAGAIN; > > + goto out_ret; > > + } > > Do you possibly need: > > current->flags &= ~PF_NPROC_EXCEEDED; > > somewhere after this point? > > I think it's weird to have past set_user() failure affect other than the > very next execve(). So we are hoping that no program uses execvp() or similar... Maybe that is reasonable but "in for a penny, in for a pound" - I'd fail them all. I think the flag should only be cleared once we notice that the limit is no longer exceeded. So clearing the flag can appear *after* the code you quote above, but not in the middle of it. > > Perhaps also reset the flag on fork() because we have an RLIMIT_NPROC > check on fork() anyway. I agree it should be cleared here too. > > Thanks, > > Alexander But there is still the issue of 'zygot' like services.... Let me try another suggestion. Instead of catching the error in do_execve_common, how about we catch it in do_mmap_pgoff. i.e. if the flag is set and an attempt it made to create an executable mapping, we check the user->processes against the limit then - either failing or clearing the flag and succeeding. This will stop an execve, and an attempt to load a shared library and call it. In the case of 'exec' the process will get a SIGKILL as well, which is probably a good thing. Thoughts? NeilBrown ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-26 0:47 ` NeilBrown @ 2011-07-26 1:16 ` Solar Designer 2011-07-26 4:11 ` NeilBrown 0 siblings, 1 reply; 41+ messages in thread From: Solar Designer @ 2011-07-26 1:16 UTC (permalink / raw) To: NeilBrown Cc: Vasiliy Kulikov, Linus Torvalds, Stephen Smalley, kernel-hardening, James Morris, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer On Tue, Jul 26, 2011 at 10:47:13AM +1000, NeilBrown wrote: > On Tue, 26 Jul 2011 03:40:13 +0400 Solar Designer <solar@openwall.com> wrote: > > On Mon, Jul 25, 2011 at 09:14:23PM +0400, Vasiliy Kulikov wrote: > > > @@ -1433,6 +1433,19 @@ static int do_execve_common(const char *filename, > > > struct files_struct *displaced; > > > bool clear_in_exec; > > > int retval; > > > + const struct cred *cred = current_cred(); > > > + > > > + /* > > > + * We move the actual failure in case of RLIMIT_NPROC excess from > > > + * set*uid() to execve() because too many poorly written programs > > > + * don't check setuid() return code. Here we additionally recheck > > > + * whether NPROC limit is still exceeded. > > > + */ > > > + if ((current->flags & PF_NPROC_EXCEEDED) && > > > + atomic_read(&cred->user->processes) > rlimit(RLIMIT_NPROC)) { > > > + retval = -EAGAIN; > > > + goto out_ret; > > > + } > > > > Do you possibly need: > > > > current->flags &= ~PF_NPROC_EXCEEDED; > > > > somewhere after this point? > > > > I think it's weird to have past set_user() failure affect other than the > > very next execve(). > > So we are hoping that no program uses execvp() or similar... Why? No, we don't, unless I am missing something. > Maybe that is > reasonable but "in for a penny, in for a pound" - I'd fail them all. > > I think the flag should only be cleared once we notice that the limit is no > longer exceeded. So clearing the flag can appear *after* the code you quote > above, but not in the middle of it. Definitely. In case execve() fails because of the limit, the flag remains set, so a second execve() by the process will fail too. > > Perhaps also reset the flag on fork() because we have an RLIMIT_NPROC > > check on fork() anyway. > > I agree it should be cleared here too. Great. Just to clarify my own words: on fork(), clear the flag in the child process only. > But there is still the issue of 'zygot' like services.... Here's my take on it: 1. It is not known (from the discussion so far) whether Android/Zygote even cares about RLIMIT_NPROC specifically or not. The code is very generic, usable for any rlimits, and the rationale behind it might have been to be able to apply certain other limits. I don't know whether or not there exists a system that actually sets RLIMIT_NPROC via that mechanism and expects it working. 2. If desired, Android/Zygote will be able to check the PF_NPROC_EXCEEDED flag, via procfs or via a prctl() interface that we might introduce. Or it may simply pass an extra fork(). > Let me try another suggestion. Instead of catching the error in > do_execve_common, how about we catch it in do_mmap_pgoff. > i.e. if the flag is set and an attempt it made to create an executable > mapping, we check the user->processes against the limit then - either failing > or clearing the flag and succeeding. > > This will stop an execve, and an attempt to load a shared library and call it. This sounds too hackish to me, although if others are (unexpectedly) OK with it, I don't mind. Thanks, Alexander ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-26 1:16 ` Solar Designer @ 2011-07-26 4:11 ` NeilBrown 2011-07-26 14:48 ` [patch v2] " Vasiliy Kulikov 0 siblings, 1 reply; 41+ messages in thread From: NeilBrown @ 2011-07-26 4:11 UTC (permalink / raw) To: Solar Designer Cc: Vasiliy Kulikov, Linus Torvalds, Stephen Smalley, kernel-hardening, James Morris, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer On Tue, 26 Jul 2011 05:16:29 +0400 Solar Designer <solar@openwall.com> wrote: > > Let me try another suggestion. Instead of catching the error in > > do_execve_common, how about we catch it in do_mmap_pgoff. > > i.e. if the flag is set and an attempt it made to create an executable > > mapping, we check the user->processes against the limit then - either failing > > or clearing the flag and succeeding. > > > > This will stop an execve, and an attempt to load a shared library and call it. > > This sounds too hackish to me, although if others are (unexpectedly) OK > with it, I don't mind. Thanks ... I think :-) I don't really see that failing mmap is any more hackish than failing execve. Both are certainly hacks. It is setuid that should fail, but that is problematic. We seem to agree that it is acceptable to delay the failure until the process actually tries to run some code for the user. I just think that mapping-a-file-for-exec is a more direct measure of "trying to run some code for the user" than "execve" is. So they are both hacks, but one it more thorough than the other. In the world of security I would hope that "thorough" would win. NeilBrown ^ permalink raw reply [flat|nested] 41+ messages in thread
* [patch v2] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-26 4:11 ` NeilBrown @ 2011-07-26 14:48 ` Vasiliy Kulikov 2011-07-27 2:15 ` NeilBrown 2011-07-29 8:06 ` Vasiliy Kulikov 0 siblings, 2 replies; 41+ messages in thread From: Vasiliy Kulikov @ 2011-07-26 14:48 UTC (permalink / raw) To: kernel-hardening Cc: Solar Designer, Linus Torvalds, Stephen Smalley, James Morris, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer Neil, Solar, On Tue, Jul 26, 2011 at 14:11 +1000, NeilBrown wrote: > I don't really see that failing mmap is any more hackish than failing execve. > > Both are certainly hacks. It is setuid that should fail, but that is > problematic. > > We seem to agree that it is acceptable to delay the failure until the process > actually tries to run some code for the user. I just think that > mapping-a-file-for-exec is a more direct measure of "trying to run some code > for the user" than "execve" is. > > So they are both hacks, but one it more thorough than the other. In the > world of security I would hope that "thorough" would win. Well, I don't mind against something more generic than the check in execve(), however, the usefulness of the check in mmap() is unclear to me. You want to make more programs fail after setuid(), but does mmap stops really many programs? Do you know any program doing mmap/dlopen after setuid() call? What if the program will not do any mmap/dlopen and e.g. start to handle network connections or do some computations? I suppose the latter case is much more often than mmap/dlopen. (Also, reading significant amount of data could be gained via read() without any mmap.) More generic place is all resource allocation syscalls, but it start to be too complex for this sort of things IMO. Meanwhile, the patch with the cleared PF on successful fork/exec is as follows: ------------------------------------------- From: Vasiliy Kulikov <segoon@openwall.com> Subject: [patch v2] move RLIMIT_NPROC check from set_user() to do_execve_common() The patch http://lkml.org/lkml/2003/7/13/226 introduced an RLIMIT_NPROC check in set_user() to check for NPROC exceeding via setuid() and similar functions. Before the check there was a possibility to greatly exceed the allowed number of processes by an unprivileged user if the program relied on rlimit only. But the check created new security threat: many poorly written programs simply don't check setuid() return code and believe it cannot fail if executed with root privileges. So, the check is removed in this patch because of too often privilege escalations related to buggy programs. The NPROC can still be enforced in the common code flow of daemons spawning user processes. Most of daemons do fork()+setuid()+execve(). The check introduced in execve() (1) enforces the same limit as in setuid() and (2) doesn't create similar security issues. Neil Brown suggested to track what specific process has exceeded the limit by setting PF_NPROC_EXCEEDED process flag. With the change only this process would fail on execve(), and other processes' execve() behaviour is not changed. Solar Designer suggested to re-check whether NPROC limit is still exceeded at the moment of execve(). If the process was sleeping for days between set*uid() and execve(), and the NPROC counter step down under the limit, the defered execve() failure because NPROC limit was exceeded days ago would be unexpected. If the limit is not exceeded anymore, we clear the flag on both execve() and fork(). Similar check was introduced in -ow patches (without the process flag). Signed-off-by: Vasiliy Kulikov <segoon@openwall.com> --- fs/exec.c | 17 +++++++++++++++++ include/linux/sched.h | 1 + kernel/cred.c | 7 +++---- kernel/fork.c | 2 +- kernel/sys.c | 13 +++++++++---- 5 files changed, 31 insertions(+), 9 deletions(-) --- diff --git a/fs/exec.c b/fs/exec.c index 6075a1e..e8b7c1a 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1433,6 +1433,23 @@ static int do_execve_common(const char *filename, struct files_struct *displaced; bool clear_in_exec; int retval; + const struct cred *cred = current_cred(); + + /* + * We move the actual failure in case of RLIMIT_NPROC excess from + * set*uid() to execve() because too many poorly written programs + * don't check setuid() return code. Here we additionally recheck + * whether NPROC limit is still exceeded. + */ + if ((current->flags & PF_NPROC_EXCEEDED) && + atomic_read(&cred->user->processes) > rlimit(RLIMIT_NPROC)) { + retval = -EAGAIN; + goto out_ret; + } + + /* If we steep below the limit, we don't want to make following + * execve() fail. */ + current->flags &= ~PF_NPROC_EXCEEDED; retval = unshare_files(&displaced); if (retval) diff --git a/include/linux/sched.h b/include/linux/sched.h index 496770a..f024c63 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1759,6 +1759,7 @@ extern void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t * #define PF_DUMPCORE 0x00000200 /* dumped core */ #define PF_SIGNALED 0x00000400 /* killed by a signal */ #define PF_MEMALLOC 0x00000800 /* Allocating memory */ +#define PF_NPROC_EXCEEDED 0x00001000 /* set_user noticed that RLIMIT_NPROC was exceeded */ #define PF_USED_MATH 0x00002000 /* if unset the fpu must be initialized before use */ #define PF_FREEZING 0x00004000 /* freeze in progress. do not account to load */ #define PF_NOFREEZE 0x00008000 /* this thread should not be frozen */ diff --git a/kernel/cred.c b/kernel/cred.c index 174fa84..52f4342 100644 --- a/kernel/cred.c +++ b/kernel/cred.c @@ -508,11 +508,10 @@ int commit_creds(struct cred *new) key_fsgid_changed(task); /* do it - * - What if a process setreuid()'s and this brings the - * new uid over his NPROC rlimit? We can check this now - * cheaply with the new uid cache, so if it matters - * we should be checking for it. -DaveM + * RLIMIT_NPROC limits on user->processes have already been checked + * in set_user(). */ + alter_cred_subscribers(new, 2); if (new->user != old->user) atomic_inc(&new->user->processes); diff --git a/kernel/fork.c b/kernel/fork.c index 0276c30..0f44484 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -995,7 +995,7 @@ static void copy_flags(unsigned long clone_flags, struct task_struct *p) { unsigned long new_flags = p->flags; - new_flags &= ~(PF_SUPERPRIV | PF_WQ_WORKER); + new_flags &= ~(PF_SUPERPRIV | PF_WQ_WORKER | PF_NPROC_EXCEEDED); new_flags |= PF_FORKNOEXEC; new_flags |= PF_STARTING; p->flags = new_flags; diff --git a/kernel/sys.c b/kernel/sys.c index e4128b2..fc40cbc 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -591,11 +591,16 @@ static int set_user(struct cred *new) if (!new_user) return -EAGAIN; + /* + * We don't fail in case of NPROC limit excess here because too many + * poorly written programs don't check set*uid() return code, assuming + * it never fails if called by root. We may still enforce NPROC limit + * for programs doing set*uid()+execve() by harmlessly defering the + * failure to the execve() stage. + */ if (atomic_read(&new_user->processes) >= rlimit(RLIMIT_NPROC) && - new_user != INIT_USER) { - free_uid(new_user); - return -EAGAIN; - } + new_user != INIT_USER) + current->flags |= PF_NPROC_EXCEEDED; free_uid(new->user); new->user = new_user; --- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [patch v2] move RLIMIT_NPROC check from set_user() to do_execve_common() 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 1 sibling, 1 reply; 41+ messages in thread From: NeilBrown @ 2011-07-27 2:15 UTC (permalink / raw) To: Vasiliy Kulikov Cc: kernel-hardening, Solar Designer, Linus Torvalds, Stephen Smalley, James Morris, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer On Tue, 26 Jul 2011 18:48:48 +0400 Vasiliy Kulikov <segoon@openwall.com> wrote: > Neil, Solar, > > On Tue, Jul 26, 2011 at 14:11 +1000, NeilBrown wrote: > > I don't really see that failing mmap is any more hackish than failing execve. > > > > Both are certainly hacks. It is setuid that should fail, but that is > > problematic. > > > > We seem to agree that it is acceptable to delay the failure until the process > > actually tries to run some code for the user. I just think that > > mapping-a-file-for-exec is a more direct measure of "trying to run some code > > for the user" than "execve" is. > > > > So they are both hacks, but one it more thorough than the other. In the > > world of security I would hope that "thorough" would win. > > Well, I don't mind against something more generic than the check in > execve(), however, the usefulness of the check in mmap() is unclear to > me. You want to make more programs fail after setuid(), but does mmap > stops really many programs? Do you know any program doing mmap/dlopen > after setuid() call? What if the program will not do any mmap/dlopen > and e.g. start to handle network connections or do some computations? > I suppose the latter case is much more often than mmap/dlopen. I think I didn't make myself clear. I don't mean we should intercept the mmap system call. I mean we could intercept the internal kernel function do_mmap_pgoff. This is used by the mmap system call but also (and more importantly) by the execve system call and the uselib system call. So any attempt to map a file and execute the code in that file - whether via exec or via mapping a shared object - will go through do_mmap_pgoff. So if we disable do_mmap_pgoff() requests which ask for execute permission when a setuid has caused RLIMIT_NPROC to be exceeded, then we catch every attempt to run the user's code as the user. I won't catch a situation where an interpreter is already loaded into the root-owned process and the setuid is followed by loading a script and running that, it is isn't perfect. But I think it is more general than just trapping in execve. NeilBrown ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [kernel-hardening] Re: [patch v2] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-27 2:15 ` NeilBrown @ 2011-07-29 7:07 ` Vasiliy Kulikov 0 siblings, 0 replies; 41+ messages in thread From: Vasiliy Kulikov @ 2011-07-29 7:07 UTC (permalink / raw) To: kernel-hardening Cc: Solar Designer, Linus Torvalds, Stephen Smalley, James Morris, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer On Wed, Jul 27, 2011 at 12:15 +1000, NeilBrown wrote: > On Tue, 26 Jul 2011 18:48:48 +0400 Vasiliy Kulikov <segoon@openwall.com> > wrote: > > On Tue, Jul 26, 2011 at 14:11 +1000, NeilBrown wrote: > > > I don't really see that failing mmap is any more hackish than failing execve. > > > > > > Both are certainly hacks. It is setuid that should fail, but that is > > > problematic. > > > > > > We seem to agree that it is acceptable to delay the failure until the process > > > actually tries to run some code for the user. I just think that > > > mapping-a-file-for-exec is a more direct measure of "trying to run some code > > > for the user" than "execve" is. > > > > > > So they are both hacks, but one it more thorough than the other. In the > > > world of security I would hope that "thorough" would win. > > > > Well, I don't mind against something more generic than the check in > > execve(), however, the usefulness of the check in mmap() is unclear to > > me. You want to make more programs fail after setuid(), but does mmap > > stops really many programs? Do you know any program doing mmap/dlopen > > after setuid() call? What if the program will not do any mmap/dlopen > > and e.g. start to handle network connections or do some computations? > > I suppose the latter case is much more often than mmap/dlopen. > > I think I didn't make myself clear. > I don't mean we should intercept the mmap system call. > > I mean we could intercept the internal kernel function do_mmap_pgoff. > > This is used by the mmap system call but also (and more importantly) by the > execve system call and the uselib system call. Here I have 2 doubts: 1) uselib() after setuid() is a rare pattern. If it doesn't catch relatively many programs, then it is only an additional complexity - addition the check deeply into mm rather than into obvious do_execve(). 2) As it touches mmap(), I'm afraid it could affect mmap-based heap allocations as well. The problem is that it might be somewhat not predictable and not repeating event - one time the process did mmap() to increase the heap, one time it did not because of minor allocation differences. The former process would be terminated just after mmap() failed, the latter would not. I don't think such uncertainty is good (even if it is rare). So, I'm hesitating to call the check in do_mmap_pgoff() "simply more generic". Thanks, -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [patch v2] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-26 14:48 ` [patch v2] " Vasiliy Kulikov 2011-07-27 2:15 ` NeilBrown @ 2011-07-29 8:06 ` Vasiliy Kulikov 2011-07-29 8:11 ` [patch v3] " Vasiliy Kulikov 1 sibling, 1 reply; 41+ messages in thread From: Vasiliy Kulikov @ 2011-07-29 8:06 UTC (permalink / raw) To: kernel-hardening Cc: Solar Designer, Linus Torvalds, Stephen Smalley, James Morris, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer On Tue, Jul 26, 2011 at 18:48 +0400, Vasiliy Kulikov wrote: > if (atomic_read(&new_user->processes) >= rlimit(RLIMIT_NPROC) && > - new_user != INIT_USER) { > - free_uid(new_user); > - return -EAGAIN; > - } > + new_user != INIT_USER) > + current->flags |= PF_NPROC_EXCEEDED; It doesn't respect the chain: setresuid() with exceeded rlimit to user A, setresuid() with normal limit to user B. While being user B, the PF is kept, which is wrong as it is not B's exceeded limit. So, it must be cleared on successful set_user() calls. I'll send a patch. -- Vasiliy Kulikov http://www.openwall.com - bringing security into open computing environments ^ permalink raw reply [flat|nested] 41+ messages in thread
* [patch v3] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-29 8:06 ` Vasiliy Kulikov @ 2011-07-29 8:11 ` Vasiliy Kulikov 2011-07-29 8:17 ` James Morris 0 siblings, 1 reply; 41+ messages in thread From: Vasiliy Kulikov @ 2011-07-29 8:11 UTC (permalink / raw) To: Linus Torvalds Cc: kernel-hardening, Stephen Smalley, James Morris, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer The patch http://lkml.org/lkml/2003/7/13/226 introduced an RLIMIT_NPROC check in set_user() to check for NPROC exceeding via setuid() and similar functions. Before the check there was a possibility to greatly exceed the allowed number of processes by an unprivileged user if the program relied on rlimit only. But the check created new security threat: many poorly written programs simply don't check setuid() return code and believe it cannot fail if executed with root privileges. So, the check is removed in this patch because of too often privilege escalations related to buggy programs. The NPROC can still be enforced in the common code flow of daemons spawning user processes. Most of daemons do fork()+setuid()+execve(). The check introduced in execve() (1) enforces the same limit as in setuid() and (2) doesn't create similar security issues. Neil Brown suggested to track what specific process has exceeded the limit by setting PF_NPROC_EXCEEDED process flag. With the change only this process would fail on execve(), and other processes' execve() behaviour is not changed. Solar Designer suggested to re-check whether NPROC limit is still exceeded at the moment of execve(). If the process was sleeping for days between set*uid() and execve(), and the NPROC counter step down under the limit, the defered execve() failure because NPROC limit was exceeded days ago would be unexpected. If the limit is not exceeded anymore, we clear the flag on successful calls to execve() and fork(). The flag is also cleared on successful calls to set_user() as the limit was exceeded for the previous user, not the current one. Similar check was introduced in -ow patches (without the process flag). v3 - clear PF_NPROC_EXCEEDED on successful calls to set_user(). Signed-off-by: Vasiliy Kulikov <segoon@openwall.com> --- fs/exec.c | 17 +++++++++++++++++ include/linux/sched.h | 1 + kernel/cred.c | 7 +++---- kernel/fork.c | 1 + kernel/sys.c | 15 +++++++++++---- 5 files changed, 33 insertions(+), 8 deletions(-) --- diff --git a/fs/exec.c b/fs/exec.c index 6075a1e..e8b7c1a 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1433,6 +1433,23 @@ static int do_execve_common(const char *filename, struct files_struct *displaced; bool clear_in_exec; int retval; + const struct cred *cred = current_cred(); + + /* + * We move the actual failure in case of RLIMIT_NPROC excess from + * set*uid() to execve() because too many poorly written programs + * don't check setuid() return code. Here we additionally recheck + * whether NPROC limit is still exceeded. + */ + if ((current->flags & PF_NPROC_EXCEEDED) && + atomic_read(&cred->user->processes) > rlimit(RLIMIT_NPROC)) { + retval = -EAGAIN; + goto out_ret; + } + + /* If we steep below the limit, we don't want to make following + * execve() fail. */ + current->flags &= ~PF_NPROC_EXCEEDED; retval = unshare_files(&displaced); if (retval) diff --git a/include/linux/sched.h b/include/linux/sched.h index 496770a..f024c63 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1759,6 +1759,7 @@ extern void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t * #define PF_DUMPCORE 0x00000200 /* dumped core */ #define PF_SIGNALED 0x00000400 /* killed by a signal */ #define PF_MEMALLOC 0x00000800 /* Allocating memory */ +#define PF_NPROC_EXCEEDED 0x00001000 /* set_user noticed that RLIMIT_NPROC was exceeded */ #define PF_USED_MATH 0x00002000 /* if unset the fpu must be initialized before use */ #define PF_FREEZING 0x00004000 /* freeze in progress. do not account to load */ #define PF_NOFREEZE 0x00008000 /* this thread should not be frozen */ diff --git a/kernel/cred.c b/kernel/cred.c index 174fa84..52f4342 100644 --- a/kernel/cred.c +++ b/kernel/cred.c @@ -508,11 +508,10 @@ int commit_creds(struct cred *new) key_fsgid_changed(task); /* do it - * - What if a process setreuid()'s and this brings the - * new uid over his NPROC rlimit? We can check this now - * cheaply with the new uid cache, so if it matters - * we should be checking for it. -DaveM + * RLIMIT_NPROC limits on user->processes have already been checked + * in set_user(). */ + alter_cred_subscribers(new, 2); if (new->user != old->user) atomic_inc(&new->user->processes); diff --git a/kernel/fork.c b/kernel/fork.c index 0276c30..de06c0a 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1110,6 +1110,7 @@ static struct task_struct *copy_process(unsigned long clone_flags, p->real_cred->user != INIT_USER) goto bad_fork_free; } + current->flags &= ~PF_NPROC_EXCEEDED; retval = copy_creds(p, clone_flags); if (retval < 0) diff --git a/kernel/sys.c b/kernel/sys.c index e4128b2..a6a8a2d 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -591,11 +591,18 @@ static int set_user(struct cred *new) if (!new_user) return -EAGAIN; + /* + * We don't fail in case of NPROC limit excess here because too many + * poorly written programs don't check set*uid() return code, assuming + * it never fails if called by root. We may still enforce NPROC limit + * for programs doing set*uid()+execve() by harmlessly defering the + * failure to the execve() stage. + */ if (atomic_read(&new_user->processes) >= rlimit(RLIMIT_NPROC) && - new_user != INIT_USER) { - free_uid(new_user); - return -EAGAIN; - } + new_user != INIT_USER) + current->flags |= PF_NPROC_EXCEEDED; + else + current->flags &= ~PF_NPROC_EXCEEDED; free_uid(new->user); new->user = new_user; --- ^ permalink raw reply related [flat|nested] 41+ messages in thread
* Re: [patch v3] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-29 8:11 ` [patch v3] " Vasiliy Kulikov @ 2011-07-29 8:17 ` James Morris 0 siblings, 0 replies; 41+ messages in thread From: James Morris @ 2011-07-29 8:17 UTC (permalink / raw) To: Vasiliy Kulikov Cc: Linus Torvalds, kernel-hardening, Stephen Smalley, linux-kernel, Greg Kroah-Hartman, Andrew Morton, David S. Miller, Jiri Slaby, Alexander Viro, linux-fsdevel, KOSAKI Motohiro, Eric Paris, Willy Tarreau, Sebastian Krahmer On Fri, 29 Jul 2011, Vasiliy Kulikov wrote: > + * RLIMIT_NPROC limits on user->processes have already been checked > + * in set_user(). > */ > + > alter_cred_subscribers(new, 2); Please get rid of the extra blank line if you re-spin. Reviewed-by: James Morris <jmorris@namei.org> -- James Morris <jmorris@namei.org> ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 2011-07-14 0:11 ` James Morris 2011-07-14 1:27 ` NeilBrown @ 2011-07-14 1:30 ` KOSAKI Motohiro 1 sibling, 0 replies; 41+ messages in thread From: KOSAKI Motohiro @ 2011-07-14 1:30 UTC (permalink / raw) To: jmorris Cc: torvalds, neilb, solar, segoon, linux-kernel, gregkh, akpm, davem, kernel-hardening, jslaby, viro, linux-fsdevel, eparis, sds (2011/07/14 9:11), James Morris wrote: > On Wed, 13 Jul 2011, Linus Torvalds wrote: > >> It sounds like people are effectively Ack'ing the patch, but with this >> kind of patch I don't want to add the "implicit Ack" that I often do >> for regular stuff. >> >> So could people who think that the patch is ok in its current form >> just send me their acked-by or reviewed-by? I haven't heard any actual >> objection to it, and I think it's valid for the current -rc. >> >> Alternatively, feel free to send a comment like "I think it's the >> right thing, but maybe it should wait for the next merge window".. > > Count me in the latter. > > It does look ok to me, but I'd be happier if it had more testing first (in > -mm perhaps). I think some security folk may be on summer vacation, too. I don't think I am best person to take ack. but I also don't want to hesitate to help Solar's good improvemnt. Reviewed-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> And I'll second James. next mere window is probably safer. Thanks. ^ permalink raw reply [flat|nested] 41+ messages in thread
* Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() 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-13 5:36 ` KOSAKI Motohiro 1 sibling, 0 replies; 41+ messages in thread From: KOSAKI Motohiro @ 2011-07-13 5:36 UTC (permalink / raw) To: segoon Cc: torvalds, linux-kernel, gregkh, akpm, davem, kernel-hardening, jslaby, jmorris, neilb, viro, linux-fsdevel (2011/07/12 22:27), Vasiliy Kulikov wrote: > The patch http://lkml.org/lkml/2003/7/13/226 introduced a RLIMIT_NPROC > check in set_user() to check for NPROC exceeding via setuid() and > similar functions. Before the check there was a possibility to greatly > exceed the allowed number of processes by an unprivileged user if the > program relied on rlimit only. But the check created new security > threat: many poorly written programs simply don't check setuid() return > code and believe it cannot fail if executed with root privileges. So, > the check is removed in this patch because of too often privilege > escalations related to buggy programs. > > The NPROC can still be enforced in the common code flow of daemons > spawning user processes. Most of daemons do fork()+setuid()+execve(). > The check introduced in execve() enforces the same limit as in setuid() > and doesn't create similar security issues. > > Similar check was introduced in -ow patches. > > Signed-off-by: Vasiliy Kulikov <segoon@openwall.com> BSD folks tell me NetBSD has the exactly same hack (ie check at exec instead setuid) since 2008. Then, I think this is enough proved safer way. http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/kern/kern_exec.c?rev=1.316&content-type=text/x-cvsweb-markup&only_with_tag=MAIN Thx. ^ permalink raw reply [flat|nested] 41+ messages in thread
end of thread, other threads:[~2011-07-29 8:21 UTC | newest] Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 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
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).