Date: Wed, 5 Jul 2017 19:05:52 +0200
From: Michal Hocko <mhocko@kernel.org>
To: Ben Hutchings <ben@decadent.org.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
        Willy Tarreau <w@1wt.eu>, Hugh Dickins <hughd@google.com>,
        Oleg Nesterov <oleg@redhat.com>,
        "Jason A. Donenfeld" <Jason@zx2c4.com>, Rik van Riel <riel@redhat.com>,
        Larry Woodman <lwoodman@redhat.com>,
        "Kirill A. Shutemov" <kirill@shutemov.name>,
        Tony Luck <tony.luck@intel.com>,
        "James E.J. Bottomley" <jejb@parisc-linux.org>,
        Helge Diller <deller@gmx.de>, James Hogan <james.hogan@imgtec.com>,
        Laura Abbott <labbott@redhat.com>, Greg KH <greg@kroah.com>,
        "security@kernel.org" <security@kernel.org>,
        Qualys Security Advisory <qsa@qualys.com>,
        LKML <linux-kernel@vger.kernel.org>, Ximin Luo <infinity0@debian.org>
Subject: Re: [PATCH] mm: larger stack guard gap, between vmas
Message-ID: <20170705170552.GA13487@dhcp22.suse.cz>
References: <20170704093538.GF14722@dhcp22.suse.cz>
 <20170704094728.GB22013@1wt.eu>
 <20170704104211.GG14722@dhcp22.suse.cz>
 <20170704113611.GA4732@decadent.org.uk>
 <1499209315.2707.29.camel@decadent.org.uk>
 <CA+55aFzjibuaV=GBZQN8KOaCpf1P3B1abJCYGaxEM9RVpf9fXg@mail.gmail.com>
 <1499257180.2707.34.camel@decadent.org.uk>
 <20170705142354.GB21220@dhcp22.suse.cz>
 <1499268300.2707.41.camel@decadent.org.uk>
 <20170705165845.GB4732@decadent.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170705165845.GB4732@decadent.org.uk>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1619
Lines: 54

On Wed 05-07-17 17:58:45, Ben Hutchings wrote:
[...]
> diff --git a/mm/mmap.c b/mm/mmap.c
> index c7906ae1a7a1..f8131a94e56e 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2307,6 +2307,25 @@ int expand_upwards(struct vm_area_struct *vma, unsigned long address)
>  }
>  #endif /* CONFIG_STACK_GROWSUP || CONFIG_IA64 */
>  
> +unsigned long __vm_start_gap(struct vm_area_struct *vma)
> +{
> +	unsigned long stack_limit =
> +		current->signal->rlim[RLIMIT_STACK].rlim_cur;
> +	unsigned long vm_start;
> +
> +	if (stack_limit != RLIM_INFINITY &&
> +	    vma->vm_end - vma->vm_start < stack_limit)
> +		vm_start = vma->vm_end - PAGE_ALIGN(stack_limit);

This is exactly what I was worried about in my previous email. Say
somebody sets stack ulimit to 1G or so. Should we reduce the available
address space that much? Say you are 32b and you have an application
with multiple stacks each doing its MAP_GROWSDOWN. You are quickly out
of address space. That's why I've said that we would need to find a cap
for the user defined limit. How much that should be though? Few (tens,
hundreds) megs. If we can figure that up I would be of course quite
happy about such a change because MAP_GROWSDOWN doesn't work really well
these days.

> +	else
> +		vm_start = vma->vm_start;
> +
> +	vm_start -= stack_guard_gap;
> +	if (vm_start > vma->vm_start)
> +		vm_start = 0;
> +
> +	return vm_start;
> +}
> +
>  /*
>   * vma is the first one with address < vma->vm_start.  Have to extend vma.
>   */
> 
> -- 
> Ben Hutchings
> For every complex problem
> there is a solution that is simple, neat, and wrong.


-- 
Michal Hocko
SUSE Labs