[PATCH v2] x86: increase MIN_GAP to include randomized stack

[PATCH v2] x86: increase MIN_GAP to include randomized stack

[Michal Hocko]

Currently we are not including randomized stack size when calculating
mmap_base address in arch_pick_mmap_layout for topdown case. This might
cause that mmap_base starts in the stack reserved area because stack is
randomized by 1GB for 64b (8MB for 32b) and the minimum gap is 128MB.

If the stack really grows down to mmap_base then we can get silent mmap
region overwrite by the stack values.

Let's include maximum stack randomization size into MIN_GAP which is
used as the low bound for the gap in mmap.

Signed-off-by: Michal Hocko <mhocko@suse.cz>
---
 arch/x86/mm/mmap.c |   21 +++++++++++++++++++--
 1 files changed, 19 insertions(+), 2 deletions(-)

I wasn't sure about STACK_RND_MASK because it is defined also in 
arch/x86/include/asm/elf.h and I couldn't find a common header file for 
both of them. If you have any idea I would like to help with that.
Or should we just let it for later cleanup?

I think that this is also stable material and I will repost it to 
stable@kernel.org once you ack it.

Changes from v1:
Fixed unsigned int overflow in MIN_GAP calculation.


diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index 1658296..ac41955 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -30,12 +30,29 @@
 #include <linux/limits.h>
 #include <linux/sched.h>
 
+/* 1GB for 64bit, 8MB for 32bit definition taken from arch/x86/include/asm/elf.h */
+#ifndef STACK_RND_MASK
+#define STACK_RND_MASK (test_thread_flag(TIF_IA32) ? 0x7ff : 0x3fffff)
+#endif
+
+static unsigned int stack_maxrandom_size(void)
+{
+	unsigned int max = 0;
+	if ((current->flags & PF_RANDOMIZE) &&
+		!(current->personality & ADDR_NO_RANDOMIZE)) {
+		max = ((-1U) & STACK_RND_MASK) << PAGE_SHIFT;
+	}
+
+	return max;
+}
+
+
 /*
  * Top of mmap area (just below the process stack).
  *
- * Leave an at least ~128 MB hole.
+ * Leave an at least ~128 MB hole with possible stack randomization.
  */
-#define MIN_GAP (128*1024*1024)
+#define MIN_GAP (128*1024*1024UL + stack_maxrandom_size())
 #define MAX_GAP (TASK_SIZE/6*5)
 
 /*
-- 
1.6.3.3

Re: [PATCH v2] x86: increase MIN_GAP to include randomized stack

[Michal Hocko]

[-- Attachment #1: Type: text/plain, Size: 3123 bytes --]

On Fri 04-09-09 11:37:00, Michal Hocko wrote:
> Currently we are not including randomized stack size when calculating
> mmap_base address in arch_pick_mmap_layout for topdown case. This might
> cause that mmap_base starts in the stack reserved area because stack is
> randomized by 1GB for 64b (8MB for 32b) and the minimum gap is 128MB.

Just for reference, attached you can find simple reproduction program
(which has been attached to our original bug report which unfortunately I
cannot make public). 

It prints memory layout when maximum mmaped address is too close to the
stack top. I understand that stack_top (as read from /proc/<PID>/stat)
is not the best choice because it doesn't include auxv, env and argv but
this should not influence the real problem detection here.

Reproduction steps:
cc -o manymap manymap.c
ulimit -v unlimited
while true
do
	./manymap || break
done

> 
> If the stack really grows down to mmap_base then we can get silent mmap
> region overwrite by the stack values.
> 
> Let's include maximum stack randomization size into MIN_GAP which is
> used as the low bound for the gap in mmap.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.cz>
> ---
>  arch/x86/mm/mmap.c |   21 +++++++++++++++++++--
>  1 files changed, 19 insertions(+), 2 deletions(-)
> 
> I wasn't sure about STACK_RND_MASK because it is defined also in 
> arch/x86/include/asm/elf.h and I couldn't find a common header file for 
> both of them. If you have any idea I would like to help with that.
> Or should we just let it for later cleanup?
> 
> I think that this is also stable material and I will repost it to 
> stable@kernel.org once you ack it.
> 
> Changes from v1:
> Fixed unsigned int overflow in MIN_GAP calculation.
> 
> 
> diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> index 1658296..ac41955 100644
> --- a/arch/x86/mm/mmap.c
> +++ b/arch/x86/mm/mmap.c
> @@ -30,12 +30,29 @@
>  #include <linux/limits.h>
>  #include <linux/sched.h>
>  
> +/* 1GB for 64bit, 8MB for 32bit definition taken from arch/x86/include/asm/elf.h */
> +#ifndef STACK_RND_MASK
> +#define STACK_RND_MASK (test_thread_flag(TIF_IA32) ? 0x7ff : 0x3fffff)
> +#endif
> +
> +static unsigned int stack_maxrandom_size(void)
> +{
> +	unsigned int max = 0;
> +	if ((current->flags & PF_RANDOMIZE) &&
> +		!(current->personality & ADDR_NO_RANDOMIZE)) {
> +		max = ((-1U) & STACK_RND_MASK) << PAGE_SHIFT;
> +	}
> +
> +	return max;
> +}
> +
> +
>  /*
>   * Top of mmap area (just below the process stack).
>   *
> - * Leave an at least ~128 MB hole.
> + * Leave an at least ~128 MB hole with possible stack randomization.
>   */
> -#define MIN_GAP (128*1024*1024)
> +#define MIN_GAP (128*1024*1024UL + stack_maxrandom_size())
>  #define MAX_GAP (TASK_SIZE/6*5)
>  
>  /*
> -- 
> 1.6.3.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

[-- Attachment #2: manymap.c --]
[-- Type: text/x-csrc, Size: 2364 bytes --]


#include <stdlib.h>
#include <string.h>
#include <sys/mman.h>
#include <stdio.h>
#include <errno.h>
#include <sys/time.h>
#include <sys/resource.h>
#include <sys/stat.h>
#include <fcntl.h>

#define TRIES 2

void *
get_stack_addr();

void print_layout(pid_t pid)
{
	char maps_file[128];
	char buf[BUFSIZ];
	int fd;
	int bytes;
	snprintf(maps_file, sizeof(maps_file)-1, "/proc/%d/maps", pid);
	
	fd = open(maps_file, O_RDONLY);
	printf("%u MEMORY layout\n", pid);
	while((bytes = read(fd, buf, sizeof(buf)))>0)
	{
		buf[bytes]='\0';
		printf("%s", buf);
	}
	printf("END\n");
}

main()
{
    size_t len;
    int i;
    void *v1, *v, *vold;
    void *stacktop;
    void *mina, *maxa;
    struct rlimit stack_limit;
    pid_t pid=getpid();
    

    // len = 17179869184;
    // len = 34359738368;

   // did not infringe
    // len = 100000L  * 4096;

    len = 20000L  * 4096 * 1024;

    stacktop = get_stack_addr();
    getrlimit(RLIMIT_STACK, &stack_limit);

    //printf("stacktop is %lx\n",(long) stacktop);

    mina = (void*)0xFFFFFFFFFFFFFFFFL;
    maxa = 0;

    vold = 0;
    for (i=0; i<TRIES; i++) {
      	v = mmap (NULL, len, PROT_READ | PROT_WRITE,
            MAP_SHARED | MAP_ANONYMOUS | MAP_NORESERVE, -1, (off_t)0);
      	if (v == MAP_FAILED) {	printf("failed, errno=%d\n",errno); abort(); } 

	if (v < mina)       mina = v;
	if (v + len > maxa) maxa = v + len;

	vold = v;

  }

  unsigned long delta = (unsigned long)stacktop - (unsigned long)maxa;
  if (delta<stack_limit.rlim_cur)
  {
	printf("%u: mmaps zone %lx - %lx stack %lx delta %ld\n", pid,
				(unsigned long)mina, (unsigned long)maxa,
				(unsigned long)stacktop, 
				delta);
	printf("Dangerous layout!\n");
	print_layout(pid);
	abort();
  }
  return 0;
}

void *
get_stack_addr()
{
    char fname[BUFSIZ], fmt_str[BUFSIZ];
    FILE *fp;
    long val;
    int i;

    // find the stack starting addr in /proc/<PID>/stat

    sprintf(fname,"/proc/%d/stat",getpid());
    if (!(fp = fopen(fname, "r"))) {
        printf("Error opening %s\n",fname);
	abort();
    }

    // create format string.  The stack starting addr is param #28
    // we don't care about the first 27 parameters.

    strcpy(fmt_str,"%*d %*s %*c");
    for (i=4;i<28;i++) {
      strcat(fmt_str," %*d");
    }
    strcat(fmt_str," %lu");

    fscanf(fp,fmt_str,&val);
    fclose(fp);

    return (void *)val;
}

Re: [PATCH v2] x86: increase MIN_GAP to include randomized stack

[Jiri Kosina]

On Fri, 4 Sep 2009, Michal Hocko wrote:

> Currently we are not including randomized stack size when calculating
> mmap_base address in arch_pick_mmap_layout for topdown case. This might
> cause that mmap_base starts in the stack reserved area because stack is
> randomized by 1GB for 64b (8MB for 32b) and the minimum gap is 128MB.
> 
> If the stack really grows down to mmap_base then we can get silent mmap
> region overwrite by the stack values.
> 
> Let's include maximum stack randomization size into MIN_GAP which is
> used as the low bound for the gap in mmap.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.cz>
> ---
>  arch/x86/mm/mmap.c |   21 +++++++++++++++++++--
>  1 files changed, 19 insertions(+), 2 deletions(-)
> 
> I wasn't sure about STACK_RND_MASK because it is defined also in 
> arch/x86/include/asm/elf.h and I couldn't find a common header file for 
> both of them. If you have any idea I would like to help with that.
> Or should we just let it for later cleanup?

What will break if we include <asm/elf.h> from mmap.c?

> I think that this is also stable material and I will repost it to 
> stable@kernel.org once you ack it.
> 
> Changes from v1:
> Fixed unsigned int overflow in MIN_GAP calculation.
> 
> 
> diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> index 1658296..ac41955 100644
> --- a/arch/x86/mm/mmap.c
> +++ b/arch/x86/mm/mmap.c
> @@ -30,12 +30,29 @@
>  #include <linux/limits.h>
>  #include <linux/sched.h>
>  
> +/* 1GB for 64bit, 8MB for 32bit definition taken from arch/x86/include/asm/elf.h */
> +#ifndef STACK_RND_MASK
> +#define STACK_RND_MASK (test_thread_flag(TIF_IA32) ? 0x7ff : 0x3fffff)
> +#endif
> +
> +static unsigned int stack_maxrandom_size(void)
> +{
> +	unsigned int max = 0;
> +	if ((current->flags & PF_RANDOMIZE) &&
> +		!(current->personality & ADDR_NO_RANDOMIZE)) {
> +		max = ((-1U) & STACK_RND_MASK) << PAGE_SHIFT;
> +	}
> +
> +	return max;
> +}
> +
> +
>  /*
>   * Top of mmap area (just below the process stack).
>   *
> - * Leave an at least ~128 MB hole.
> + * Leave an at least ~128 MB hole with possible stack randomization.
>   */
> -#define MIN_GAP (128*1024*1024)
> +#define MIN_GAP (128*1024*1024UL + stack_maxrandom_size())
>  #define MAX_GAP (TASK_SIZE/6*5)

Apart from doubling the STACK_RND_MASK definition

	Acked-by: Jiri Kosina <jkosina@suse.cz>

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v2] x86: increase MIN_GAP to include randomized stack

[Michal Hocko]

On Mon 07-09-09 17:18:44, Jiri Kosina wrote:
> On Fri, 4 Sep 2009, Michal Hocko wrote:
> 
> > Currently we are not including randomized stack size when calculating
> > mmap_base address in arch_pick_mmap_layout for topdown case. This might
> > cause that mmap_base starts in the stack reserved area because stack is
> > randomized by 1GB for 64b (8MB for 32b) and the minimum gap is 128MB.
> > 
> > If the stack really grows down to mmap_base then we can get silent mmap
> > region overwrite by the stack values.
> > 
> > Let's include maximum stack randomization size into MIN_GAP which is
> > used as the low bound for the gap in mmap.
> > 
> > Signed-off-by: Michal Hocko <mhocko@suse.cz>
> > ---
> >  arch/x86/mm/mmap.c |   21 +++++++++++++++++++--
> >  1 files changed, 19 insertions(+), 2 deletions(-)
> > 
> > I wasn't sure about STACK_RND_MASK because it is defined also in 
> > arch/x86/include/asm/elf.h and I couldn't find a common header file for 
> > both of them. If you have any idea I would like to help with that.
> > Or should we just let it for later cleanup?
> 
> What will break if we include <asm/elf.h> from mmap.c?

I have tried that and as far as compilation went it was OK... Except for
one thing which I haven't noticed before as well. STACK_RND_MASK is not
defined outside of CONFIG_X86_64 (in <arch/elf.h>).

One possible way to fix this is to update the original patch with:

diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index ac41955..a4566a8 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -32,7 +32,11 @@
 
 /* 1GB for 64bit, 8MB for 32bit definition taken from arch/x86/include/asm/elf.h */
 #ifndef STACK_RND_MASK
+#ifdef CONFIG_X86_64
 #define STACK_RND_MASK (test_thread_flag(TIF_IA32) ? 0x7ff : 0x3fffff)
+#else
+#define STACK_RND_MASK (0x7ff)
+#endif
 #endif
 
 static unsigned int stack_maxrandom_size(void)

This, however, doesn't look very nice. If there is no objection I would
define STACK_RND_MASK in <arch/elf.h> also out of CONFIG_X86_64 and
remove this definition from mmap.c completely. What do you think?

> 
> > I think that this is also stable material and I will repost it to 
> > stable@kernel.org once you ack it.
> > 
> > Changes from v1:
> > Fixed unsigned int overflow in MIN_GAP calculation.
> > 
> > 
> > diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> > index 1658296..ac41955 100644
> > --- a/arch/x86/mm/mmap.c
> > +++ b/arch/x86/mm/mmap.c
> > @@ -30,12 +30,29 @@
> >  #include <linux/limits.h>
> >  #include <linux/sched.h>
> >  
> > +/* 1GB for 64bit, 8MB for 32bit definition taken from arch/x86/include/asm/elf.h */
> > +#ifndef STACK_RND_MASK
> > +#define STACK_RND_MASK (test_thread_flag(TIF_IA32) ? 0x7ff : 0x3fffff)
> > +#endif
> > +
> > +static unsigned int stack_maxrandom_size(void)
> > +{
> > +	unsigned int max = 0;
> > +	if ((current->flags & PF_RANDOMIZE) &&
> > +		!(current->personality & ADDR_NO_RANDOMIZE)) {
> > +		max = ((-1U) & STACK_RND_MASK) << PAGE_SHIFT;
> > +	}
> > +
> > +	return max;
> > +}
> > +
> > +
> >  /*
> >   * Top of mmap area (just below the process stack).
> >   *
> > - * Leave an at least ~128 MB hole.
> > + * Leave an at least ~128 MB hole with possible stack randomization.
> >   */
> > -#define MIN_GAP (128*1024*1024)
> > +#define MIN_GAP (128*1024*1024UL + stack_maxrandom_size())
> >  #define MAX_GAP (TASK_SIZE/6*5)
> 
> Apart from doubling the STACK_RND_MASK definition
> 
> 	Acked-by: Jiri Kosina <jkosina@suse.cz>
> 
> Thanks,
> 
> -- 
> Jiri Kosina
> SUSE Labs

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

[PATCH v3] x86: increase MIN_GAP to include randomized stack

[Michal Hocko]

Currently we are not including randomized stack size when calculating
mmap_base address in arch_pick_mmap_layout for topdown case. This might
cause that mmap_base starts in the stack reserved area because stack is
randomized by 1GB for 64b (8MB for 32b) and the minimum gap is 128MB.

If the stack really grows down to mmap_base then we can get silent mmap
region overwrite by the stack values.

Let's include maximum stack randomization size into MIN_GAP which is
used as the low bound for the gap in mmap.

Signed-off-by: Michal Hocko <mhocko@suse.cz>
---
 arch/x86/mm/mmap.c |   25 +++++++++++++++++++++++--
 1 files changed, 23 insertions(+), 2 deletions(-)

I think that this is also stable material and I will repost it to 
stable@kernel.org once you ack it.

Changes from v2:
STACK_RND_MASK has to be defined for CONFIG_X86_32 as well.

Changes from v1:
Fixed unsigned int overflow in MIN_GAP calculation.


diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index 1658296..a4566a8 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -30,12 +30,33 @@
 #include <linux/limits.h>
 #include <linux/sched.h>
 
+/* 1GB for 64bit, 8MB for 32bit definition taken from arch/x86/include/asm/elf.h */
+#ifndef STACK_RND_MASK
+#ifdef CONFIG_X86_64
+#define STACK_RND_MASK (test_thread_flag(TIF_IA32) ? 0x7ff : 0x3fffff)
+#else
+#define STACK_RND_MASK (0x7ff)
+#endif
+#endif
+
+static unsigned int stack_maxrandom_size(void)
+{
+	unsigned int max = 0;
+	if ((current->flags & PF_RANDOMIZE) &&
+		!(current->personality & ADDR_NO_RANDOMIZE)) {
+		max = ((-1U) & STACK_RND_MASK) << PAGE_SHIFT;
+	}
+
+	return max;
+}
+
+
 /*
  * Top of mmap area (just below the process stack).
  *
- * Leave an at least ~128 MB hole.
+ * Leave an at least ~128 MB hole with possible stack randomization.
  */
-#define MIN_GAP (128*1024*1024)
+#define MIN_GAP (128*1024*1024UL + stack_maxrandom_size())
 #define MAX_GAP (TASK_SIZE/6*5)
 
 /*
-- 
1.6.3.3

Re: [PATCH v3] x86: increase MIN_GAP to include randomized stack

[Jiri Kosina]

On Tue, 8 Sep 2009, Michal Hocko wrote:

> Currently we are not including randomized stack size when calculating
> mmap_base address in arch_pick_mmap_layout for topdown case. This might
> cause that mmap_base starts in the stack reserved area because stack is
> randomized by 1GB for 64b (8MB for 32b) and the minimum gap is 128MB.
> 
> If the stack really grows down to mmap_base then we can get silent mmap
> region overwrite by the stack values.
> 
> Let's include maximum stack randomization size into MIN_GAP which is
> used as the low bound for the gap in mmap.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.cz>
> ---
>  arch/x86/mm/mmap.c |   25 +++++++++++++++++++++++--
>  1 files changed, 23 insertions(+), 2 deletions(-)
> 
> I think that this is also stable material and I will repost it to 
> stable@kernel.org once you ack it.
> 
> Changes from v2:
> STACK_RND_MASK has to be defined for CONFIG_X86_32 as well.
> 
> Changes from v1:
> Fixed unsigned int overflow in MIN_GAP calculation.
> 
> 
> diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> index 1658296..a4566a8 100644
> --- a/arch/x86/mm/mmap.c
> +++ b/arch/x86/mm/mmap.c
> @@ -30,12 +30,33 @@
>  #include <linux/limits.h>
>  #include <linux/sched.h>
>  
> +/* 1GB for 64bit, 8MB for 32bit definition taken from arch/x86/include/asm/elf.h */
> +#ifndef STACK_RND_MASK
> +#ifdef CONFIG_X86_64
> +#define STACK_RND_MASK (test_thread_flag(TIF_IA32) ? 0x7ff : 0x3fffff)
> +#else
> +#define STACK_RND_MASK (0x7ff)
> +#endif
> +#endif

I'd still prefer all this macro-magic happening in <asm/elf.h> and 
including this file from arch/x86/mm/mmap.c, otherwise we are duplicating 
the STACK_RND_MASK definition, which is messy.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v3] x86: increase MIN_GAP to include randomized stack

[Michal Hocko]

On Tue 08-09-09 10:47:46, Jiri Kosina wrote:
> On Tue, 8 Sep 2009, Michal Hocko wrote:
> 
> > Currently we are not including randomized stack size when calculating
> > mmap_base address in arch_pick_mmap_layout for topdown case. This might
> > cause that mmap_base starts in the stack reserved area because stack is
> > randomized by 1GB for 64b (8MB for 32b) and the minimum gap is 128MB.
> > 
> > If the stack really grows down to mmap_base then we can get silent mmap
> > region overwrite by the stack values.
> > 
> > Let's include maximum stack randomization size into MIN_GAP which is
> > used as the low bound for the gap in mmap.
> > 
> > Signed-off-by: Michal Hocko <mhocko@suse.cz>
> > ---
> >  arch/x86/mm/mmap.c |   25 +++++++++++++++++++++++--
> >  1 files changed, 23 insertions(+), 2 deletions(-)
> > 
> > I think that this is also stable material and I will repost it to 
> > stable@kernel.org once you ack it.
> > 
> > Changes from v2:
> > STACK_RND_MASK has to be defined for CONFIG_X86_32 as well.
> > 
> > Changes from v1:
> > Fixed unsigned int overflow in MIN_GAP calculation.
> > 
> > 
> > diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> > index 1658296..a4566a8 100644
> > --- a/arch/x86/mm/mmap.c
> > +++ b/arch/x86/mm/mmap.c
> > @@ -30,12 +30,33 @@
> >  #include <linux/limits.h>
> >  #include <linux/sched.h>
> >  
> > +/* 1GB for 64bit, 8MB for 32bit definition taken from arch/x86/include/asm/elf.h */
> > +#ifndef STACK_RND_MASK
> > +#ifdef CONFIG_X86_64
> > +#define STACK_RND_MASK (test_thread_flag(TIF_IA32) ? 0x7ff : 0x3fffff)
> > +#else
> > +#define STACK_RND_MASK (0x7ff)
> > +#endif
> > +#endif
> 
> I'd still prefer all this macro-magic happening in <asm/elf.h> and 
> including this file from arch/x86/mm/mmap.c, otherwise we are duplicating 
> the STACK_RND_MASK definition, which is messy.

I definitely agree. I just wanted to post the complete patch for
reference. The elf variant will follow.

> 
> -- 
> Jiri Kosina
> SUSE Labs

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

[PATCH v4] x86: increase MIN_GAP to include randomized stack

[Michal Hocko]

Currently we are not including randomized stack size when calculating
mmap_base address in arch_pick_mmap_layout for topdown case. This might
cause that mmap_base starts in the stack reserved area because stack is
randomized by 1GB for 64b (8MB for 32b) and the minimum gap is 128MB.

If the stack really grows down to mmap_base then we can get silent mmap
region overwrite by the stack values.

Let's include maximum stack randomization size into MIN_GAP which is
used as the low bound for the gap in mmap.

Signed-off-by: Michal Hocko <mhocko@suse.cz>
---
 arch/x86/include/asm/elf.h |    2 ++
 arch/x86/mm/mmap.c         |   17 +++++++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

Maybe we should split the patch into 2 parts, one for STACK_RND_MASK
definition cleanup for X86_32 and the other one for the MIN_GAP issue.
What do you think?

Changes from v3:
Remove STACK_RND_MASK from mmap.c completely and rather include asm/elf.h
instead. Also define STACK_RND_MASK for CONFIG_X86_32 case.

Changes from v2:
STACK_RND_MASK has to be defined for CONFIG_X86_32 as well.

Changes from v1:
Fixed unsigned int overflow in MIN_GAP calculation.

diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index 83c1bc8..456a304 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -299,6 +299,8 @@ do {									\
 
 #ifdef CONFIG_X86_32
 
+#define STACK_RND_MASK (0x7ff)
+
 #define VDSO_HIGH_BASE		(__fix_to_virt(FIX_VDSO))
 
 #define ARCH_DLINFO		ARCH_DLINFO_IA32(vdso_enabled)
diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index 1658296..c8191de 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -29,13 +29,26 @@
 #include <linux/random.h>
 #include <linux/limits.h>
 #include <linux/sched.h>
+#include <asm/elf.h>
+
+static unsigned int stack_maxrandom_size(void)
+{
+	unsigned int max = 0;
+	if ((current->flags & PF_RANDOMIZE) &&
+		!(current->personality & ADDR_NO_RANDOMIZE)) {
+		max = ((-1U) & STACK_RND_MASK) << PAGE_SHIFT;
+	}
+
+	return max;
+}
+
 
 /*
  * Top of mmap area (just below the process stack).
  *
- * Leave an at least ~128 MB hole.
+ * Leave an at least ~128 MB hole with possible stack randomization.
  */
-#define MIN_GAP (128*1024*1024)
+#define MIN_GAP (128*1024*1024UL + stack_maxrandom_size())
 #define MAX_GAP (TASK_SIZE/6*5)
 
 /*
-- 
1.6.3.3

Re: [PATCH v4] x86: increase MIN_GAP to include randomized stack

[Jiri Kosina]

On Tue, 8 Sep 2009, Michal Hocko wrote:

> Currently we are not including randomized stack size when calculating
> mmap_base address in arch_pick_mmap_layout for topdown case. This might
> cause that mmap_base starts in the stack reserved area because stack is
> randomized by 1GB for 64b (8MB for 32b) and the minimum gap is 128MB.
> 
> If the stack really grows down to mmap_base then we can get silent mmap
> region overwrite by the stack values.
> 
> Let's include maximum stack randomization size into MIN_GAP which is
> used as the low bound for the gap in mmap.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.cz>

Acked-by: Jiri Kosina <jkosina@suse.cz>

Ingo, could you please consider applying this?

> ---
>  arch/x86/include/asm/elf.h |    2 ++
>  arch/x86/mm/mmap.c         |   17 +++++++++++++++--
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
> index 83c1bc8..456a304 100644
> --- a/arch/x86/include/asm/elf.h
> +++ b/arch/x86/include/asm/elf.h
> @@ -299,6 +299,8 @@ do {									\
>  
>  #ifdef CONFIG_X86_32
>  
> +#define STACK_RND_MASK (0x7ff)
> +
>  #define VDSO_HIGH_BASE		(__fix_to_virt(FIX_VDSO))
>  
>  #define ARCH_DLINFO		ARCH_DLINFO_IA32(vdso_enabled)
> diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> index 1658296..c8191de 100644
> --- a/arch/x86/mm/mmap.c
> +++ b/arch/x86/mm/mmap.c
> @@ -29,13 +29,26 @@
>  #include <linux/random.h>
>  #include <linux/limits.h>
>  #include <linux/sched.h>
> +#include <asm/elf.h>
> +
> +static unsigned int stack_maxrandom_size(void)
> +{
> +	unsigned int max = 0;
> +	if ((current->flags & PF_RANDOMIZE) &&
> +		!(current->personality & ADDR_NO_RANDOMIZE)) {
> +		max = ((-1U) & STACK_RND_MASK) << PAGE_SHIFT;
> +	}
> +
> +	return max;
> +}
> +
>  
>  /*
>   * Top of mmap area (just below the process stack).
>   *
> - * Leave an at least ~128 MB hole.
> + * Leave an at least ~128 MB hole with possible stack randomization.
>   */
> -#define MIN_GAP (128*1024*1024)
> +#define MIN_GAP (128*1024*1024UL + stack_maxrandom_size())
>  #define MAX_GAP (TASK_SIZE/6*5)
>  
>  /*
> -- 
> 1.6.3.3
> 

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v4] x86: increase MIN_GAP to include randomized stack

[Jiri Kosina]

On Tue, 8 Sep 2009, Jiri Kosina wrote:

> > Currently we are not including randomized stack size when calculating
> > mmap_base address in arch_pick_mmap_layout for topdown case. This might
> > cause that mmap_base starts in the stack reserved area because stack is
> > randomized by 1GB for 64b (8MB for 32b) and the minimum gap is 128MB.
> > 
> > If the stack really grows down to mmap_base then we can get silent mmap
> > region overwrite by the stack values.
> > 
> > Let's include maximum stack randomization size into MIN_GAP which is
> > used as the low bound for the gap in mmap.
> > 
> > Signed-off-by: Michal Hocko <mhocko@suse.cz>
> 
> Acked-by: Jiri Kosina <jkosina@suse.cz>
> 
> Ingo, could you please consider applying this?

Any comments about this patch please? (aka. "ping") :)

> > ---
> >  arch/x86/include/asm/elf.h |    2 ++
> >  arch/x86/mm/mmap.c         |   17 +++++++++++++++--
> >  2 files changed, 17 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
> > index 83c1bc8..456a304 100644
> > --- a/arch/x86/include/asm/elf.h
> > +++ b/arch/x86/include/asm/elf.h
> > @@ -299,6 +299,8 @@ do {									\
> >  
> >  #ifdef CONFIG_X86_32
> >  
> > +#define STACK_RND_MASK (0x7ff)
> > +
> >  #define VDSO_HIGH_BASE		(__fix_to_virt(FIX_VDSO))
> >  
> >  #define ARCH_DLINFO		ARCH_DLINFO_IA32(vdso_enabled)
> > diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> > index 1658296..c8191de 100644
> > --- a/arch/x86/mm/mmap.c
> > +++ b/arch/x86/mm/mmap.c
> > @@ -29,13 +29,26 @@
> >  #include <linux/random.h>
> >  #include <linux/limits.h>
> >  #include <linux/sched.h>
> > +#include <asm/elf.h>
> > +
> > +static unsigned int stack_maxrandom_size(void)
> > +{
> > +	unsigned int max = 0;
> > +	if ((current->flags & PF_RANDOMIZE) &&
> > +		!(current->personality & ADDR_NO_RANDOMIZE)) {
> > +		max = ((-1U) & STACK_RND_MASK) << PAGE_SHIFT;
> > +	}
> > +
> > +	return max;
> > +}
> > +
> >  
> >  /*
> >   * Top of mmap area (just below the process stack).
> >   *
> > - * Leave an at least ~128 MB hole.
> > + * Leave an at least ~128 MB hole with possible stack randomization.
> >   */
> > -#define MIN_GAP (128*1024*1024)
> > +#define MIN_GAP (128*1024*1024UL + stack_maxrandom_size())
> >  #define MAX_GAP (TASK_SIZE/6*5)
> >  
> >  /*
> > -- 
> > 1.6.3.3
> > 

-- 
Jiri Kosina
SUSE Labs, Novell Inc.

Re: [PATCH v4] x86: increase MIN_GAP to include randomized stack

[H. Peter Anvin]

On 09/10/2009 04:14 PM, Jiri Kosina wrote:
> On Tue, 8 Sep 2009, Jiri Kosina wrote:
> 
>>> Currently we are not including randomized stack size when calculating
>>> mmap_base address in arch_pick_mmap_layout for topdown case. This might
>>> cause that mmap_base starts in the stack reserved area because stack is
>>> randomized by 1GB for 64b (8MB for 32b) and the minimum gap is 128MB.
>>>
>>> If the stack really grows down to mmap_base then we can get silent mmap
>>> region overwrite by the stack values.
>>>
>>> Let's include maximum stack randomization size into MIN_GAP which is
>>> used as the low bound for the gap in mmap.
>>>
>>> Signed-off-by: Michal Hocko <mhocko@suse.cz>
>>
>> Acked-by: Jiri Kosina <jkosina@suse.cz>
>>
>> Ingo, could you please consider applying this?
> 
> Any comments about this patch please? (aka. "ping") :)
> 

Looks right to me... I'll apply it and see how it does.

	-hpa

[tip:x86/urgent] x86: Increase MIN_GAP to include randomized stack

[tip-bot for Michal Hocko]

Commit-ID:  80938332d8cf652f6b16e0788cf0ca136befe0b5
Gitweb:     http://git.kernel.org/tip/80938332d8cf652f6b16e0788cf0ca136befe0b5
Author:     Michal Hocko <mhocko@suse.cz>
AuthorDate: Tue, 8 Sep 2009 11:01:55 +0200
Committer:  H. Peter Anvin <hpa@zytor.com>
CommitDate: Thu, 10 Sep 2009 17:00:12 -0700

x86: Increase MIN_GAP to include randomized stack

Currently we are not including randomized stack size when calculating
mmap_base address in arch_pick_mmap_layout for topdown case. This might
cause that mmap_base starts in the stack reserved area because stack is
randomized by 1GB for 64b (8MB for 32b) and the minimum gap is 128MB.

If the stack really grows down to mmap_base then we can get silent mmap
region overwrite by the stack values.

Let's include maximum stack randomization size into MIN_GAP which is
used as the low bound for the gap in mmap.

Signed-off-by: Michal Hocko <mhocko@suse.cz>
LKML-Reference: <1252400515-6866-1-git-send-email-mhocko@suse.cz>
Acked-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: H. Peter Anvin <hpa@zytor.com>
Cc: Stable Team <stable@kernel.org>


---
 arch/x86/include/asm/elf.h |    2 ++
 arch/x86/mm/mmap.c         |   17 +++++++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index 83c1bc8..456a304 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -299,6 +299,8 @@ do {									\
 
 #ifdef CONFIG_X86_32
 
+#define STACK_RND_MASK (0x7ff)
+
 #define VDSO_HIGH_BASE		(__fix_to_virt(FIX_VDSO))
 
 #define ARCH_DLINFO		ARCH_DLINFO_IA32(vdso_enabled)
diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index 1658296..c8191de 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -29,13 +29,26 @@
 #include <linux/random.h>
 #include <linux/limits.h>
 #include <linux/sched.h>
+#include <asm/elf.h>
+
+static unsigned int stack_maxrandom_size(void)
+{
+	unsigned int max = 0;
+	if ((current->flags & PF_RANDOMIZE) &&
+		!(current->personality & ADDR_NO_RANDOMIZE)) {
+		max = ((-1U) & STACK_RND_MASK) << PAGE_SHIFT;
+	}
+
+	return max;
+}
+
 
 /*
  * Top of mmap area (just below the process stack).
  *
- * Leave an at least ~128 MB hole.
+ * Leave an at least ~128 MB hole with possible stack randomization.
  */
-#define MIN_GAP (128*1024*1024)
+#define MIN_GAP (128*1024*1024UL + stack_maxrandom_size())
 #define MAX_GAP (TASK_SIZE/6*5)
 
 /*