From: Oleg Nesterov <oleg@redhat.com>
To: Hugh Dickins <hughd@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Cyrill Gorcunov <gorcunov@openvz.org>,
Manfred Spraul <manfred@colorfullife.com>,
Davidlohr Bueso <davidlohr.bueso@hp.com>,
Kees Cook <keescook@chromium.org>, Tejun Heo <tj@kernel.org>,
Andrew Vagin <avagin@openvz.org>,
"Eric W. Biederman" <ebiederm@xmission.com>,
"H. Peter Anvin" <hpa@zytor.com>,
Serge Hallyn <serge.hallyn@canonical.com>,
Pavel Emelyanov <xemul@parallels.com>,
Vasiliy Kulikov <segoon@openwall.com>,
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
Michael Kerrisk <mtk.manpages@gmail.com>,
Julien Tinnes <jln@google.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] ipc/shm: fix the historical/wrong mm->start_stack check
Date: Tue, 26 Aug 2014 16:32:27 +0200 [thread overview]
Message-ID: <20140826143227.GA30900@redhat.com> (raw)
In-Reply-To: <alpine.LSU.2.11.1408251303220.25799@eggly.anvils>
On 08/25, Hugh Dickins wrote:
>
> On Mon, 25 Aug 2014, Oleg Nesterov wrote:
>
> > As Hugh pointed out, we actually need to require the additional
> > guard page, but this code was written before linux had it.
> >
> > 3. This wrongly assumes that the stack can only grown down.
> >
> > Personally I think we should simply kill this check, but I did not
> > dare to do this. So the patch only fixes the 1st problem (mostly to
> > avoid the usage of mm->start_stack) and ignores the VM_GROWSUP case.
> >
> > Signed-off-by: Oleg Nesterov <oleg@redhat.com>
>
> Sorry, I cannot ack this,
Hugh, I appreciate you nack even more. Thanks!
> because your comment below "at least 4 pages
> plus a guard page enforced by check_stack_guard_page()" makes no sense
> to me as an explanation for the 5. The guard page (gap) enforced by
> check_stack_guard_page() is already at vma->vm_start
^^^^^^^^^^^^^^^^^^^^^^^^
Ahh. Yes I misunderstood this logic. And yes, you tried to explain it
twice but I was too stupid.
> I did come across 2.6.34's 128k stack_expand inherited from 2.6.11's
> 20 page EXTRA_STACK_VM_PAGES. With Linus's guard page enforcing a
> page gap since 2.6.36.
OK, and setup_arg_pages() still does expand_stack() although stack_expand
depends on RLIMIT_STACK. But I think this doesn't matter. The room was
already reserved, in general mm->start_stack points into the middle of
the stack.
So that check in do_shmat() can only help if the stack was not expanded
due to the low RLIMIT_STACK, then this application raises RLIMIT_STACK,
then does do_shmat(). But in this case it should likely crash when exec
returns to usermode.
> and our difficulty
> in communicating a sensible way of updating and describing the test,
I like your polite interpretation of my ignorance and inability to listen ;)
> now drives me to agree with you. Please just rip out the start_stack
> test and the comment defending it.
Great. Will do.
Oleg.
next prev parent reply other threads:[~2014-08-26 14:35 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-23 14:42 [PATCH 0/1] ipc/shm: fix the historical/wrong mm->start_stack check Oleg Nesterov
2014-08-23 14:43 ` [PATCH 1/1] " Oleg Nesterov
2014-08-23 15:22 ` Cyrill Gorcunov
2014-08-23 15:58 ` Oleg Nesterov
2014-08-25 1:38 ` Hugh Dickins
2014-08-25 15:03 ` Oleg Nesterov
2014-08-25 16:18 ` Hugh Dickins
2014-08-25 17:18 ` Oleg Nesterov
2014-08-25 18:51 ` Hugh Dickins
2014-08-25 19:09 ` Oleg Nesterov
2014-08-24 17:41 ` Manfred Spraul
2014-08-25 14:53 ` Oleg Nesterov
2014-08-25 19:12 ` [PATCH v2] " Oleg Nesterov
2014-08-25 21:22 ` Cyrill Gorcunov
2014-08-26 19:37 ` Oleg Nesterov
2014-08-25 21:49 ` Hugh Dickins
2014-08-26 14:32 ` Oleg Nesterov [this message]
2014-08-26 19:31 ` [PATCH v3] ipc/shm: kill " Oleg Nesterov
2014-08-26 20:28 ` Hugh Dickins
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=20140826143227.GA30900@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=avagin@openvz.org \
--cc=davidlohr.bueso@hp.com \
--cc=ebiederm@xmission.com \
--cc=gorcunov@openvz.org \
--cc=hpa@zytor.com \
--cc=hughd@google.com \
--cc=jln@google.com \
--cc=kamezawa.hiroyu@jp.fujitsu.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=manfred@colorfullife.com \
--cc=mtk.manpages@gmail.com \
--cc=segoon@openwall.com \
--cc=serge.hallyn@canonical.com \
--cc=tj@kernel.org \
--cc=xemul@parallels.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.