All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Fix mmap_base entropy for >31 bits.
@ 2016-02-04 22:06 Daniel Cashman
  2016-02-04 22:06 ` [PATCH 1/2] drivers: char: random: Add get_random_long() Daniel Cashman
  2016-02-04 22:29 ` [PATCH 0/2] Fix mmap_base entropy for >31 bits Kees Cook
  0 siblings, 2 replies; 7+ messages in thread
From: Daniel Cashman @ 2016-02-04 22:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux, akpm, keescook, tytso, arnd, gregkh, catalin.marinas,
	will.deacon, ralf, benh, paulus, mpe, davem, tglx, mingo, hpa,
	x86, viro, nnk, jeffv, salyzyn, Daniel Cashman

Upstream commit: d07e22597d1d355829b7b18ac19afa912cf758d1 added the
ability to choose from a range of values to use for entropy count in
generating the random offset to the mmap_base address.  The
maximum value on this range was set to 32 bits for 64-bit x86 systems,
but this value could be increased further, requiring more than the 32
bits of randomness provided by get_random_int(), as is already possible
for arm64. Add a new function: get_random_long() which more naturally
fits with the mmap usage of get_random_int() but operates exactly the
same as get_random_int().

Also, fix the shifting constant in mmap_rnd() to be an unsigned long so
that values greater than 31 bits generate an appropriate mask without
overflow.  This is especially important on x86, as its shift instruction
uses a 5-bit mask for the shift operand, which meant that any value for
mmap_rnd_bits over 31 acts as a no-op and effectively disables mmap_base
randomization.

Finally, replace calls to get_random_int() with get_random_long() where
appropriate.

Daniel Cashman (2):
  drivers: char: random: Add get_random_long().
  use get_random_long().

 arch/arm/mm/mmap.c               |  2 +-
 arch/arm64/mm/mmap.c             |  4 ++--
 arch/mips/mm/mmap.c              |  4 ++--
 arch/powerpc/kernel/process.c    |  4 ++--
 arch/powerpc/mm/mmap.c           |  4 ++--
 arch/sparc/kernel/sys_sparc_64.c |  2 +-
 arch/x86/mm/mmap.c               |  6 +++---
 drivers/char/random.c            | 22 ++++++++++++++++++++++
 fs/binfmt_elf.c                  |  2 +-
 include/linux/random.h           |  1 +
 10 files changed, 37 insertions(+), 14 deletions(-)

-- 
2.7.0.rc3.207.g0ac5344

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] drivers: char: random: Add get_random_long().
  2016-02-04 22:06 [PATCH 0/2] Fix mmap_base entropy for >31 bits Daniel Cashman
@ 2016-02-04 22:06 ` Daniel Cashman
  2016-02-04 22:06   ` [PATCH 2/2] use get_random_long() Daniel Cashman
  2016-02-04 22:29 ` [PATCH 0/2] Fix mmap_base entropy for >31 bits Kees Cook
  1 sibling, 1 reply; 7+ messages in thread
From: Daniel Cashman @ 2016-02-04 22:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux, akpm, keescook, tytso, arnd, gregkh, catalin.marinas,
	will.deacon, ralf, benh, paulus, mpe, davem, tglx, mingo, hpa,
	x86, viro, nnk, jeffv, salyzyn, Daniel Cashman

Signed-off-by: Daniel Cashman <dcashman@android.com>
---
 drivers/char/random.c  | 22 ++++++++++++++++++++++
 include/linux/random.h |  1 +
 2 files changed, 23 insertions(+)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index d0da5d8..b583e53 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1819,6 +1819,28 @@ unsigned int get_random_int(void)
 EXPORT_SYMBOL(get_random_int);
 
 /*
+ * Same as get_random_int(), but returns unsigned long.
+ */
+unsigned long get_random_long(void)
+{
+	__u32 *hash;
+	unsigned long ret;
+
+	if (arch_get_random_long(&ret))
+		return ret;
+
+	hash = get_cpu_var(get_random_int_hash);
+
+	hash[0] += current->pid + jiffies + random_get_entropy();
+	md5_transform(hash, random_int_secret);
+	ret = *(unsigned long *)hash;
+	put_cpu_var(get_random_int_hash);
+
+	return ret;
+}
+EXPORT_SYMBOL(get_random_long);
+
+/*
  * randomize_range() returns a start address such that
  *
  *    [...... <range> .....]
diff --git a/include/linux/random.h b/include/linux/random.h
index a75840c..9c29122 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -34,6 +34,7 @@ extern const struct file_operations random_fops, urandom_fops;
 #endif
 
 unsigned int get_random_int(void);
+unsigned long get_random_long(void);
 unsigned long randomize_range(unsigned long start, unsigned long end, unsigned long len);
 
 u32 prandom_u32(void);
-- 
2.7.0.rc3.207.g0ac5344

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] use get_random_long().
  2016-02-04 22:06 ` [PATCH 1/2] drivers: char: random: Add get_random_long() Daniel Cashman
@ 2016-02-04 22:06   ` Daniel Cashman
  2016-02-04 22:38     ` Kees Cook
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Cashman @ 2016-02-04 22:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux, akpm, keescook, tytso, arnd, gregkh, catalin.marinas,
	will.deacon, ralf, benh, paulus, mpe, davem, tglx, mingo, hpa,
	x86, viro, nnk, jeffv, salyzyn, Daniel Cashman

Replace calls to get_random_int() followed by a cast to (unsigned long)
with calls to get_random_long().  Also address shifting bug which, in
case of x86 removed entropy mask for mmap_rnd_bits values > 31 bits.

Signed-off-by: Daniel Cashman <dcashman@android.com>
---
 arch/arm/mm/mmap.c               | 2 +-
 arch/arm64/mm/mmap.c             | 4 ++--
 arch/mips/mm/mmap.c              | 4 ++--
 arch/powerpc/kernel/process.c    | 4 ++--
 arch/powerpc/mm/mmap.c           | 4 ++--
 arch/sparc/kernel/sys_sparc_64.c | 2 +-
 arch/x86/mm/mmap.c               | 6 +++---
 fs/binfmt_elf.c                  | 2 +-
 8 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mm/mmap.c b/arch/arm/mm/mmap.c
index 4b4058d..66353ca 100644
--- a/arch/arm/mm/mmap.c
+++ b/arch/arm/mm/mmap.c
@@ -173,7 +173,7 @@ unsigned long arch_mmap_rnd(void)
 {
 	unsigned long rnd;
 
-	rnd = (unsigned long)get_random_int() & ((1 << mmap_rnd_bits) - 1);
+	rnd = get_random_long() & ((1UL << mmap_rnd_bits) - 1);
 
 	return rnd << PAGE_SHIFT;
 }
diff --git a/arch/arm64/mm/mmap.c b/arch/arm64/mm/mmap.c
index 4c893b5..232f787 100644
--- a/arch/arm64/mm/mmap.c
+++ b/arch/arm64/mm/mmap.c
@@ -53,10 +53,10 @@ unsigned long arch_mmap_rnd(void)
 
 #ifdef CONFIG_COMPAT
 	if (test_thread_flag(TIF_32BIT))
-		rnd = (unsigned long)get_random_int() & ((1 << mmap_rnd_compat_bits) - 1);
+		rnd = get_random_long() & ((1UL << mmap_rnd_compat_bits) - 1);
 	else
 #endif
-		rnd = (unsigned long)get_random_int() & ((1 << mmap_rnd_bits) - 1);
+		rnd = get_random_long() & ((1UL << mmap_rnd_bits) - 1);
 	return rnd << PAGE_SHIFT;
 }
 
diff --git a/arch/mips/mm/mmap.c b/arch/mips/mm/mmap.c
index 5c81fdd..3530376 100644
--- a/arch/mips/mm/mmap.c
+++ b/arch/mips/mm/mmap.c
@@ -146,7 +146,7 @@ unsigned long arch_mmap_rnd(void)
 {
 	unsigned long rnd;
 
-	rnd = (unsigned long)get_random_int();
+	rnd = get_random_long();
 	rnd <<= PAGE_SHIFT;
 	if (TASK_IS_32BIT_ADDR)
 		rnd &= 0xfffffful;
@@ -174,7 +174,7 @@ void arch_pick_mmap_layout(struct mm_struct *mm)
 
 static inline unsigned long brk_rnd(void)
 {
-	unsigned long rnd = get_random_int();
+	unsigned long rnd = get_random_long();
 
 	rnd = rnd << PAGE_SHIFT;
 	/* 8MB for 32bit, 256MB for 64bit */
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index dccc87e..3c5736e 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1768,9 +1768,9 @@ static inline unsigned long brk_rnd(void)
 
 	/* 8MB for 32bit, 1GB for 64bit */
 	if (is_32bit_task())
-		rnd = (long)(get_random_int() % (1<<(23-PAGE_SHIFT)));
+		rnd = (get_random_long() % (1UL<<(23-PAGE_SHIFT)));
 	else
-		rnd = (long)(get_random_int() % (1<<(30-PAGE_SHIFT)));
+		rnd = (get_random_long() % (1UL<<(30-PAGE_SHIFT)));
 
 	return rnd << PAGE_SHIFT;
 }
diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
index 0f0502e..4087705 100644
--- a/arch/powerpc/mm/mmap.c
+++ b/arch/powerpc/mm/mmap.c
@@ -59,9 +59,9 @@ unsigned long arch_mmap_rnd(void)
 
 	/* 8MB for 32bit, 1GB for 64bit */
 	if (is_32bit_task())
-		rnd = (unsigned long)get_random_int() % (1<<(23-PAGE_SHIFT));
+		rnd = get_random_long() % (1<<(23-PAGE_SHIFT));
 	else
-		rnd = (unsigned long)get_random_int() % (1<<(30-PAGE_SHIFT));
+		rnd = get_random_long() % (1UL<<(30-PAGE_SHIFT));
 
 	return rnd << PAGE_SHIFT;
 }
diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
index c690c8e..b489e97 100644
--- a/arch/sparc/kernel/sys_sparc_64.c
+++ b/arch/sparc/kernel/sys_sparc_64.c
@@ -264,7 +264,7 @@ static unsigned long mmap_rnd(void)
 	unsigned long rnd = 0UL;
 
 	if (current->flags & PF_RANDOMIZE) {
-		unsigned long val = get_random_int();
+		unsigned long val = get_random_long();
 		if (test_thread_flag(TIF_32BIT))
 			rnd = (val % (1UL << (23UL-PAGE_SHIFT)));
 		else
diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index 96bd1e2..72bb52f 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -71,12 +71,12 @@ unsigned long arch_mmap_rnd(void)
 
 	if (mmap_is_ia32())
 #ifdef CONFIG_COMPAT
-		rnd = (unsigned long)get_random_int() & ((1 << mmap_rnd_compat_bits) - 1);
+		rnd = get_random_long() & ((1UL << mmap_rnd_compat_bits) - 1);
 #else
-		rnd = (unsigned long)get_random_int() & ((1 << mmap_rnd_bits) - 1);
+		rnd = get_random_long() & ((1UL << mmap_rnd_bits) - 1);
 #endif
 	else
-		rnd = (unsigned long)get_random_int() & ((1 << mmap_rnd_bits) - 1);
+		rnd = get_random_long() & ((1UL << mmap_rnd_bits) - 1);
 
 	return rnd << PAGE_SHIFT;
 }
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 051ea48..7d914c6 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -653,7 +653,7 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
 
 	if ((current->flags & PF_RANDOMIZE) &&
 		!(current->personality & ADDR_NO_RANDOMIZE)) {
-		random_variable = (unsigned long) get_random_int();
+		random_variable = get_random_long();
 		random_variable &= STACK_RND_MASK;
 		random_variable <<= PAGE_SHIFT;
 	}
-- 
2.7.0.rc3.207.g0ac5344

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] Fix mmap_base entropy for >31 bits.
  2016-02-04 22:06 [PATCH 0/2] Fix mmap_base entropy for >31 bits Daniel Cashman
  2016-02-04 22:06 ` [PATCH 1/2] drivers: char: random: Add get_random_long() Daniel Cashman
@ 2016-02-04 22:29 ` Kees Cook
  2016-02-24 20:40   ` Daniel Cashman
  1 sibling, 1 reply; 7+ messages in thread
From: Kees Cook @ 2016-02-04 22:29 UTC (permalink / raw)
  To: Daniel Cashman, Andrew Morton
  Cc: LKML, Russell King - ARM Linux, Theodore Ts'o, Arnd Bergmann,
	Greg KH, Catalin Marinas, Will Deacon, Ralf Baechle, benh,
	Paul Mackerras, Michael Ellerman, David S. Miller,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Al Viro,
	Nick Kralevich, Jeffrey Vander Stoep, Mark Salyzyn

On Thu, Feb 4, 2016 at 2:06 PM, Daniel Cashman <dcashman@android.com> wrote:
> Upstream commit: d07e22597d1d355829b7b18ac19afa912cf758d1 added the
> ability to choose from a range of values to use for entropy count in
> generating the random offset to the mmap_base address.  The
> maximum value on this range was set to 32 bits for 64-bit x86 systems,
> but this value could be increased further, requiring more than the 32
> bits of randomness provided by get_random_int(), as is already possible
> for arm64. Add a new function: get_random_long() which more naturally
> fits with the mmap usage of get_random_int() but operates exactly the
> same as get_random_int().
>
> Also, fix the shifting constant in mmap_rnd() to be an unsigned long so
> that values greater than 31 bits generate an appropriate mask without
> overflow.  This is especially important on x86, as its shift instruction
> uses a 5-bit mask for the shift operand, which meant that any value for
> mmap_rnd_bits over 31 acts as a no-op and effectively disables mmap_base
> randomization.
>
> Finally, replace calls to get_random_int() with get_random_long() where
> appropriate.
>
> Daniel Cashman (2):
>   drivers: char: random: Add get_random_long().
>   use get_random_long().
>
>  arch/arm/mm/mmap.c               |  2 +-
>  arch/arm64/mm/mmap.c             |  4 ++--
>  arch/mips/mm/mmap.c              |  4 ++--
>  arch/powerpc/kernel/process.c    |  4 ++--
>  arch/powerpc/mm/mmap.c           |  4 ++--
>  arch/sparc/kernel/sys_sparc_64.c |  2 +-
>  arch/x86/mm/mmap.c               |  6 +++---
>  drivers/char/random.c            | 22 ++++++++++++++++++++++
>  fs/binfmt_elf.c                  |  2 +-
>  include/linux/random.h           |  1 +
>  10 files changed, 37 insertions(+), 14 deletions(-)

Thanks for fixing this!

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] use get_random_long().
  2016-02-04 22:06   ` [PATCH 2/2] use get_random_long() Daniel Cashman
@ 2016-02-04 22:38     ` Kees Cook
  0 siblings, 0 replies; 7+ messages in thread
From: Kees Cook @ 2016-02-04 22:38 UTC (permalink / raw)
  To: Daniel Cashman, Andrew Morton, Theodore Ts'o
  Cc: LKML, Russell King - ARM Linux, Arnd Bergmann, Greg KH,
	Catalin Marinas, Will Deacon, Ralf Baechle, benh, Paul Mackerras,
	Michael Ellerman, David S. Miller, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Al Viro, Nick Kralevich,
	Jeffrey Vander Stoep, Mark Salyzyn

On Thu, Feb 4, 2016 at 2:06 PM, Daniel Cashman <dcashman@android.com> wrote:
> Replace calls to get_random_int() followed by a cast to (unsigned long)
> with calls to get_random_long().  Also address shifting bug which, in
> case of x86 removed entropy mask for mmap_rnd_bits values > 31 bits.

I wonder if randomize_range() should be using get_random_long()? Right
now, a large range would get truncated:

unsigned long
randomize_range(unsigned long start, unsigned long end, unsigned long len)
{
        unsigned long range = end - len - start;

        if (end <= start + len)
                return 0;
        return PAGE_ALIGN(get_random_int() % range + start);
}

For example, randomize_range(0, 0x7ffffffff000, 4096) will never
return 0x700000000000... no current callers use a >MAX_UINT range, so
nothing is bugged now, but it seems like we should fix this too
(separately)?

-Kees

>
> Signed-off-by: Daniel Cashman <dcashman@android.com>
> ---
>  arch/arm/mm/mmap.c               | 2 +-
>  arch/arm64/mm/mmap.c             | 4 ++--
>  arch/mips/mm/mmap.c              | 4 ++--
>  arch/powerpc/kernel/process.c    | 4 ++--
>  arch/powerpc/mm/mmap.c           | 4 ++--
>  arch/sparc/kernel/sys_sparc_64.c | 2 +-
>  arch/x86/mm/mmap.c               | 6 +++---
>  fs/binfmt_elf.c                  | 2 +-
>  8 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/mm/mmap.c b/arch/arm/mm/mmap.c
> index 4b4058d..66353ca 100644
> --- a/arch/arm/mm/mmap.c
> +++ b/arch/arm/mm/mmap.c
> @@ -173,7 +173,7 @@ unsigned long arch_mmap_rnd(void)
>  {
>         unsigned long rnd;
>
> -       rnd = (unsigned long)get_random_int() & ((1 << mmap_rnd_bits) - 1);
> +       rnd = get_random_long() & ((1UL << mmap_rnd_bits) - 1);
>
>         return rnd << PAGE_SHIFT;
>  }
> diff --git a/arch/arm64/mm/mmap.c b/arch/arm64/mm/mmap.c
> index 4c893b5..232f787 100644
> --- a/arch/arm64/mm/mmap.c
> +++ b/arch/arm64/mm/mmap.c
> @@ -53,10 +53,10 @@ unsigned long arch_mmap_rnd(void)
>
>  #ifdef CONFIG_COMPAT
>         if (test_thread_flag(TIF_32BIT))
> -               rnd = (unsigned long)get_random_int() & ((1 << mmap_rnd_compat_bits) - 1);
> +               rnd = get_random_long() & ((1UL << mmap_rnd_compat_bits) - 1);
>         else
>  #endif
> -               rnd = (unsigned long)get_random_int() & ((1 << mmap_rnd_bits) - 1);
> +               rnd = get_random_long() & ((1UL << mmap_rnd_bits) - 1);
>         return rnd << PAGE_SHIFT;
>  }
>
> diff --git a/arch/mips/mm/mmap.c b/arch/mips/mm/mmap.c
> index 5c81fdd..3530376 100644
> --- a/arch/mips/mm/mmap.c
> +++ b/arch/mips/mm/mmap.c
> @@ -146,7 +146,7 @@ unsigned long arch_mmap_rnd(void)
>  {
>         unsigned long rnd;
>
> -       rnd = (unsigned long)get_random_int();
> +       rnd = get_random_long();
>         rnd <<= PAGE_SHIFT;
>         if (TASK_IS_32BIT_ADDR)
>                 rnd &= 0xfffffful;
> @@ -174,7 +174,7 @@ void arch_pick_mmap_layout(struct mm_struct *mm)
>
>  static inline unsigned long brk_rnd(void)
>  {
> -       unsigned long rnd = get_random_int();
> +       unsigned long rnd = get_random_long();
>
>         rnd = rnd << PAGE_SHIFT;
>         /* 8MB for 32bit, 256MB for 64bit */
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index dccc87e..3c5736e 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1768,9 +1768,9 @@ static inline unsigned long brk_rnd(void)
>
>         /* 8MB for 32bit, 1GB for 64bit */
>         if (is_32bit_task())
> -               rnd = (long)(get_random_int() % (1<<(23-PAGE_SHIFT)));
> +               rnd = (get_random_long() % (1UL<<(23-PAGE_SHIFT)));
>         else
> -               rnd = (long)(get_random_int() % (1<<(30-PAGE_SHIFT)));
> +               rnd = (get_random_long() % (1UL<<(30-PAGE_SHIFT)));
>
>         return rnd << PAGE_SHIFT;
>  }
> diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
> index 0f0502e..4087705 100644
> --- a/arch/powerpc/mm/mmap.c
> +++ b/arch/powerpc/mm/mmap.c
> @@ -59,9 +59,9 @@ unsigned long arch_mmap_rnd(void)
>
>         /* 8MB for 32bit, 1GB for 64bit */
>         if (is_32bit_task())
> -               rnd = (unsigned long)get_random_int() % (1<<(23-PAGE_SHIFT));
> +               rnd = get_random_long() % (1<<(23-PAGE_SHIFT));
>         else
> -               rnd = (unsigned long)get_random_int() % (1<<(30-PAGE_SHIFT));
> +               rnd = get_random_long() % (1UL<<(30-PAGE_SHIFT));
>
>         return rnd << PAGE_SHIFT;
>  }
> diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
> index c690c8e..b489e97 100644
> --- a/arch/sparc/kernel/sys_sparc_64.c
> +++ b/arch/sparc/kernel/sys_sparc_64.c
> @@ -264,7 +264,7 @@ static unsigned long mmap_rnd(void)
>         unsigned long rnd = 0UL;
>
>         if (current->flags & PF_RANDOMIZE) {
> -               unsigned long val = get_random_int();
> +               unsigned long val = get_random_long();
>                 if (test_thread_flag(TIF_32BIT))
>                         rnd = (val % (1UL << (23UL-PAGE_SHIFT)));
>                 else
> diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> index 96bd1e2..72bb52f 100644
> --- a/arch/x86/mm/mmap.c
> +++ b/arch/x86/mm/mmap.c
> @@ -71,12 +71,12 @@ unsigned long arch_mmap_rnd(void)
>
>         if (mmap_is_ia32())
>  #ifdef CONFIG_COMPAT
> -               rnd = (unsigned long)get_random_int() & ((1 << mmap_rnd_compat_bits) - 1);
> +               rnd = get_random_long() & ((1UL << mmap_rnd_compat_bits) - 1);
>  #else
> -               rnd = (unsigned long)get_random_int() & ((1 << mmap_rnd_bits) - 1);
> +               rnd = get_random_long() & ((1UL << mmap_rnd_bits) - 1);
>  #endif
>         else
> -               rnd = (unsigned long)get_random_int() & ((1 << mmap_rnd_bits) - 1);
> +               rnd = get_random_long() & ((1UL << mmap_rnd_bits) - 1);
>
>         return rnd << PAGE_SHIFT;
>  }
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 051ea48..7d914c6 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -653,7 +653,7 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
>
>         if ((current->flags & PF_RANDOMIZE) &&
>                 !(current->personality & ADDR_NO_RANDOMIZE)) {
> -               random_variable = (unsigned long) get_random_int();
> +               random_variable = get_random_long();
>                 random_variable &= STACK_RND_MASK;
>                 random_variable <<= PAGE_SHIFT;
>         }
> --
> 2.7.0.rc3.207.g0ac5344
>



-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] Fix mmap_base entropy for >31 bits.
  2016-02-04 22:29 ` [PATCH 0/2] Fix mmap_base entropy for >31 bits Kees Cook
@ 2016-02-24 20:40   ` Daniel Cashman
  2016-02-24 21:50     ` Kees Cook
  0 siblings, 1 reply; 7+ messages in thread
From: Daniel Cashman @ 2016-02-24 20:40 UTC (permalink / raw)
  To: Kees Cook, Andrew Morton
  Cc: LKML, Russell King - ARM Linux, Theodore Ts'o, Arnd Bergmann,
	Greg KH, Catalin Marinas, Will Deacon, Ralf Baechle, benh,
	Paul Mackerras, Michael Ellerman, David S. Miller,
	Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Al Viro,
	Nick Kralevich, Jeffrey Vander Stoep, Mark Salyzyn

On 02/04/2016 02:29 PM, Kees Cook wrote:
> On Thu, Feb 4, 2016 at 2:06 PM, Daniel Cashman <dcashman@android.com> wrote:
>> Upstream commit: d07e22597d1d355829b7b18ac19afa912cf758d1 added the
>> ability to choose from a range of values to use for entropy count in
>> generating the random offset to the mmap_base address.  The
>> maximum value on this range was set to 32 bits for 64-bit x86 systems,
>> but this value could be increased further, requiring more than the 32
>> bits of randomness provided by get_random_int(), as is already possible
>> for arm64. Add a new function: get_random_long() which more naturally
>> fits with the mmap usage of get_random_int() but operates exactly the
>> same as get_random_int().
>>
>> Also, fix the shifting constant in mmap_rnd() to be an unsigned long so
>> that values greater than 31 bits generate an appropriate mask without
>> overflow.  This is especially important on x86, as its shift instruction
>> uses a 5-bit mask for the shift operand, which meant that any value for
>> mmap_rnd_bits over 31 acts as a no-op and effectively disables mmap_base
>> randomization.
>>
>> Finally, replace calls to get_random_int() with get_random_long() where
>> appropriate.
>>
>> Daniel Cashman (2):
>>   drivers: char: random: Add get_random_long().
>>   use get_random_long().
>>
>>  arch/arm/mm/mmap.c               |  2 +-
>>  arch/arm64/mm/mmap.c             |  4 ++--
>>  arch/mips/mm/mmap.c              |  4 ++--
>>  arch/powerpc/kernel/process.c    |  4 ++--
>>  arch/powerpc/mm/mmap.c           |  4 ++--
>>  arch/sparc/kernel/sys_sparc_64.c |  2 +-
>>  arch/x86/mm/mmap.c               |  6 +++---
>>  drivers/char/random.c            | 22 ++++++++++++++++++++++
>>  fs/binfmt_elf.c                  |  2 +-
>>  include/linux/random.h           |  1 +
>>  10 files changed, 37 insertions(+), 14 deletions(-)
> 
> Thanks for fixing this!
> 
> Acked-by: Kees Cook <keescook@chromium.org>
> 
> -Kees
> 

This has now been in linux-next for awhile. Could we please submit this
for the 4.5 rc window so that it fixes the issue in the final 4.5
release?  Sorry if this is a protocol breach, but I'm not sure what the
best way is for me to indicate that this is a "fix" that should go out
in the same release as the original feature.

Thank You,
Dan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] Fix mmap_base entropy for >31 bits.
  2016-02-24 20:40   ` Daniel Cashman
@ 2016-02-24 21:50     ` Kees Cook
  0 siblings, 0 replies; 7+ messages in thread
From: Kees Cook @ 2016-02-24 21:50 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Daniel Cashman, LKML, Russell King - ARM Linux,
	Theodore Ts'o, Arnd Bergmann, Greg KH, Catalin Marinas,
	Will Deacon, Ralf Baechle, benh, Paul Mackerras,
	Michael Ellerman, David S. Miller, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, x86, Al Viro, Nick Kralevich,
	Jeffrey Vander Stoep, Mark Salyzyn

On Wed, Feb 24, 2016 at 12:40 PM, Daniel Cashman <dcashman@android.com> wrote:
> On 02/04/2016 02:29 PM, Kees Cook wrote:
>> On Thu, Feb 4, 2016 at 2:06 PM, Daniel Cashman <dcashman@android.com> wrote:
>>> Upstream commit: d07e22597d1d355829b7b18ac19afa912cf758d1 added the
>>> ability to choose from a range of values to use for entropy count in
>>> generating the random offset to the mmap_base address.  The
>>> maximum value on this range was set to 32 bits for 64-bit x86 systems,
>>> but this value could be increased further, requiring more than the 32
>>> bits of randomness provided by get_random_int(), as is already possible
>>> for arm64. Add a new function: get_random_long() which more naturally
>>> fits with the mmap usage of get_random_int() but operates exactly the
>>> same as get_random_int().
>>>
>>> Also, fix the shifting constant in mmap_rnd() to be an unsigned long so
>>> that values greater than 31 bits generate an appropriate mask without
>>> overflow.  This is especially important on x86, as its shift instruction
>>> uses a 5-bit mask for the shift operand, which meant that any value for
>>> mmap_rnd_bits over 31 acts as a no-op and effectively disables mmap_base
>>> randomization.
>>>
>>> Finally, replace calls to get_random_int() with get_random_long() where
>>> appropriate.
>>>
>>> Daniel Cashman (2):
>>>   drivers: char: random: Add get_random_long().
>>>   use get_random_long().
>>>
>>>  arch/arm/mm/mmap.c               |  2 +-
>>>  arch/arm64/mm/mmap.c             |  4 ++--
>>>  arch/mips/mm/mmap.c              |  4 ++--
>>>  arch/powerpc/kernel/process.c    |  4 ++--
>>>  arch/powerpc/mm/mmap.c           |  4 ++--
>>>  arch/sparc/kernel/sys_sparc_64.c |  2 +-
>>>  arch/x86/mm/mmap.c               |  6 +++---
>>>  drivers/char/random.c            | 22 ++++++++++++++++++++++
>>>  fs/binfmt_elf.c                  |  2 +-
>>>  include/linux/random.h           |  1 +
>>>  10 files changed, 37 insertions(+), 14 deletions(-)
>>
>> Thanks for fixing this!
>>
>> Acked-by: Kees Cook <keescook@chromium.org>
>>
>> -Kees
>>
>
> This has now been in linux-next for awhile. Could we please submit this
> for the 4.5 rc window so that it fixes the issue in the final 4.5
> release?  Sorry if this is a protocol breach, but I'm not sure what the
> best way is for me to indicate that this is a "fix" that should go out
> in the same release as the original feature.

Yes please! :) Andrew, can you push these for 4.5? Best to keep the
feature from releasing broken.

Thanks!

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-02-24 21:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-04 22:06 [PATCH 0/2] Fix mmap_base entropy for >31 bits Daniel Cashman
2016-02-04 22:06 ` [PATCH 1/2] drivers: char: random: Add get_random_long() Daniel Cashman
2016-02-04 22:06   ` [PATCH 2/2] use get_random_long() Daniel Cashman
2016-02-04 22:38     ` Kees Cook
2016-02-04 22:29 ` [PATCH 0/2] Fix mmap_base entropy for >31 bits Kees Cook
2016-02-24 20:40   ` Daniel Cashman
2016-02-24 21:50     ` Kees Cook

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.