linux-kernel - Re: [PATCH 0/1] expand_downwards: don't require the gap if !vm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 28 Jun 2017 16:26:33 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Oleg Nesterov <oleg@...hat.com>
Cc:     Hugh Dickins <hughd@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Larry Woodman <lwoodman@...hat.com>,
        Michal Hocko <mhocko@...e.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/1] expand_downwards: don't require the gap if !vm_prev

On Wed, Jun 28, 2017 at 10:52 AM, Oleg Nesterov <oleg@...hat.com> wrote:
>
> Now that the stack-guard-page has gone, why do we need to allow to grow
> into the previous VM_GROWSDOWN vma? IOW, why we can not simply remove
> the VM_GROWSDOWN check in expand_downwards() ?

Because the "prev" vma may actually be the original vma.

I think I described it in an earlier thread, but what happened at
least once was:

 - program has some part that uses a lot of stack for part of the
execution for some temp buffer or deep recursion or whatever

 - somebody noticed this, and decided to free up the no-longer-used
pages by doing a "munmap()" after the program was done with that part
of the stack

 - but the "munmap()" wasn't complete (maybe it only accounted for the
explicitly used buffer, whatever), so the munmap actually didn't just
remove the no-longer used bottom of the stack, it actually split the
stack segment into two (with a small remaining stack turd that was the
*real* bottom of the deep stack that used to exist)

 - you do want to be able to grow the stack back to what it was, and
it needs to be able to meet that old turd without triggering the
"oops, guard area!" logic.

As to your patch: I would prefer to actually keep the new failure
behavior of unconditionally breaking a big stack expansion), unless
there's an actual thing it breaks.

In fact, I'd even be quite open to adding a kernel warning about badly
behaved binaries that grow their stack by a big amount in one go. Not
only is it bad taste (and we really should encourage compilers to do
probing every page when growing the stack), but it migth be a sign of
an attempted attack.

                Linus