mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Roland McGrath <roland@redhat.com>
To: Kees Cook <kees.cook@canonical.com>
Cc: linux-kernel@vger.kernel.org, oss-security@lists.openwall.com,
	Al Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Neil Horman <nhorman@tuxdriver.com>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH] exec argument expansion can inappropriately trigger OOM-killer
Date: Sun, 29 Aug 2010 17:56:48 -0700 (PDT)	[thread overview]
Message-ID: <20100830005648.431B7400D9@magilla.sf.frob.com> (raw)
In-Reply-To: Kees Cook's message of  Friday, 27 August 2010 15:02:58 -0700 <20100827220258.GF4703@outflux.net>

IMHO unlimited should mean unlimited.  So, on that score, I'd leave this
constraint out and just say whatever deficiencies in the OOM killer (or in
whatever should make a manifestly too-large allocation get ENOMEM) should
just be fixed separately.

But that aside, I'll just consider the intent stated in the comment in
get_arg_page:
		 * Limit to 1/4-th the stack size for the argv+env strings.
		 * This ensures that:
		 *  - the remaining binfmt code will not run out of stack space,
		 *  - the program will have a reasonable amount of stack left
		 *    to work from.
To effect "1/4th the stack size", a cap at TASK_SIZE/4 does make some sense,
since TASK_SIZE is less than RLIM_INFINITY even in the pure 32-bit world,
and that is the true theoretical limit on stack size.

The trouble here, both for that stated intent, and for this "exploit",
is which TASK_SIZE that is on a biarch machine.  In fact, it's the
TASK_SIZE of the process that called execve.  (get_arg_page is called
from copy_strings, from do_execve before search_binary_handler--i.e.,
before anything has looked at the file to decide whether it's going to
be a 32-bit or 64-bit task on exec.)  If it's a 32-bit process exec'ing
a 64-bit program, it's the 32-bit TASK_SIZE (perhaps as little as 3GB).
So that's a limit of 0.75GB on a 64-bit program, which might actually do
just fine with 2 or 3GB.  If it's a 64-bit process exec'ing a 32-bit
program, it's the 64-bit TASK_SIZE (128TB on x86-64).  So that's a limit
of 32TB, which is perhaps not that helpfully less than 2PB minus 1 byte
(RLIM_INFINITY/4) as far as preventing any over-allocation DoS in practice.

So IMHO your change does marginal harm in some cases (32 execs 64)
and makes no appreciable difference to anyone interested in malice
(who can just dodge by exploiting it via 64 execs 64 or 64 execs 32).

If you want to constrain it this way, it's probably simpler just to use
a smaller hard limit for RLIM_STACK at boot time (and hence system-wide).
But it sounds like all you really need is to fix the OOM/allocation
behavior for huge stack allocations.


Thanks,
Roland

  parent reply	other threads:[~2010-08-30  0:57 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-08-27 22:02 Kees Cook
2010-08-30  0:19 ` KOSAKI Motohiro
2010-08-30  0:56 ` Roland McGrath [this message]
2010-08-30  3:23   ` Solar Designer
2010-08-30 10:06     ` Roland McGrath
2010-08-30 19:48       ` Solar Designer
2010-08-31  0:40         ` Roland McGrath
2010-09-08  2:34       ` [PATCH 0/3] execve argument-copying fixes Roland McGrath
2010-09-08  2:35         ` [PATCH 1/3] setup_arg_pages: diagnose excessive argument size Roland McGrath
2010-09-08  8:29           ` pageexec
2010-09-10  8:59             ` Roland McGrath
2010-09-11 13:30               ` pageexec
2010-09-14 19:33                 ` Roland McGrath
2010-09-14 22:35                   ` pageexec
2010-09-08 11:57           ` Brad Spengler
2010-09-09  5:31             ` KOSAKI Motohiro
2010-09-10  9:25               ` Roland McGrath
2010-09-10  9:43                 ` KOSAKI Motohiro
2010-09-11 13:39                 ` pageexec
2010-09-14 18:51                   ` Roland McGrath
2010-09-14 20:28                     ` pageexec
2010-09-14 21:16                       ` Roland McGrath
2010-09-14 22:27                         ` pageexec
2010-09-14 23:04                           ` Roland McGrath
2010-09-15  9:27                             ` pageexec
2010-09-10  9:18             ` Roland McGrath
2010-09-08  2:36         ` [PATCH 2/3] execve: improve interactivity with large arguments Roland McGrath
2010-09-08  2:37         ` [PATCH 3/3] execve: make responsive to SIGKILL " Roland McGrath
2010-09-08  3:00         ` [PATCH 0/3] execve argument-copying fixes KOSAKI Motohiro
2010-09-09  5:01         ` [PATCH 0/2] execve memory exhaust of " KOSAKI Motohiro
2010-09-09  5:03           ` [PATCH 1/2] oom: don't ignore rss in nascent mm KOSAKI Motohiro
2010-09-09 22:05             ` Oleg Nesterov
2010-09-10  9:39               ` Roland McGrath
2010-09-10  9:57               ` [PATCH] move cred_guard_mutex from task_struct to signal_struct KOSAKI Motohiro
2010-09-10 17:24                 ` Oleg Nesterov
2010-09-16  5:51                   ` KOSAKI Motohiro
2010-09-09  5:04           ` [PATCH 2/2] execve: check the VM has enough memory at first KOSAKI Motohiro
2010-09-10 15:06             ` Linus Torvalds
2010-09-14  1:52               ` KOSAKI Motohiro
2010-09-16  5:51                 ` KOSAKI Motohiro
2010-09-16 15:01                   ` Linus Torvalds
2010-08-30 17:49     ` [PATCH] exec argument expansion can inappropriately trigger OOM-killer Solar Designer
2010-08-30 22:08       ` Brad Spengler
2010-08-31 11:53         ` Solar Designer
2010-08-31 11:56         ` [PATCH] exec argument expansion can inappropriately triggerOOM-killer Tetsuo Handa

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=20100830005648.431B7400D9@magilla.sf.frob.com \
    --to=roland@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=kees.cook@canonical.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=oleg@redhat.com \
    --cc=oss-security@lists.openwall.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

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

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

all inboxes | Powered by JetHome®