linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Solar Designer <solar@openwall.com>
To: Vasiliy Kulikov <segoon@openwall.com>
Cc: NeilBrown <neilb@suse.de>,
	kernel-hardening@lists.openwall.com,
	James Morris <jmorris@namei.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org, Greg Kroah-Hartman <gregkh@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	"David S. Miller" <davem@davemloft.net>,
	Jiri Slaby <jslaby@suse.cz>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Eric Paris <eparis@redhat.com>,
	Stephen Smalley <sds@tycho.nsa.gov>, Willy Tarreau <w@1wt.eu>,
	Sebastian Krahmer <krahmer@suse.de>
Subject: Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common()
Date: Fri, 15 Jul 2011 17:04:09 +0400	[thread overview]
Message-ID: <20110715130409.GA2700@openwall.com> (raw)
In-Reply-To: <20110715073823.GA3821@albatros>

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

  reply	other threads:[~2011-07-15 13:04 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20110612130953.GA3709@albatros>
     [not found] ` <20110706173631.GA5431@albatros>
     [not found]   ` <CA+55aFyfjG1h2zkkGai_VPM8p5bhWhvNXs1HvuWMgxv4RSywYw@mail.gmail.com>
     [not found]     ` <20110706185932.GB3299@albatros>
     [not found]       ` <20110707075610.GA3411@albatros>
     [not found]         ` <20110707081930.GA4393@albatros>
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 [this message]
2011-07-15 13:58                                     ` 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=20110715130409.GA2700@openwall.com \
    --to=solar@openwall.com \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=eparis@redhat.com \
    --cc=gregkh@suse.de \
    --cc=jmorris@namei.org \
    --cc=jslaby@suse.cz \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=krahmer@suse.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=sds@tycho.nsa.gov \
    --cc=segoon@openwall.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=w@1wt.eu \
    /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).