linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vasiliy Kulikov <segooon@gmail.com>
To: linux-kernel@vger.kernel.org
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Greg Kroah-Hartman <gregkh@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	"David S. Miller" <davem@davemloft.net>,
	Jiri Slaby <jslaby@suse.cz>, James Morris <jmorris@namei.org>,
	Neil Brown <neilb@suse.de>,
	kernel-hardening@lists.openwall.com
Subject: RLIMIT_NPROC check in set_user()
Date: Sun, 12 Jun 2011 17:09:54 +0400	[thread overview]
Message-ID: <20110612130953.GA3709@albatros> (raw)

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

             reply	other threads:[~2011-06-12 13:12 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-12 13:09 Vasiliy Kulikov [this message]
2011-07-06 17:36 ` RLIMIT_NPROC check in set_user() 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110612130953.GA3709@albatros \
    --to=segooon@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=gregkh@suse.de \
    --cc=jmorris@namei.org \
    --cc=jslaby@suse.cz \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).