Fix mmap_base entropy for >31 bits.
Subsystems (10)
cover Fix mmap_base entropy for >31 bits.
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(-)
Signature · 1 line
2.7.0.rc3.207.g0ac5344
PATCH 1/2 drivers: char: random: Add get_random_long().
--- drivers/char/random.c | 22 ++++++++++++++++++++++ include/linux/random.h | 1 + 2 files changed, 23 insertions(+)
drivers/char/random.c +22 -0
@@ -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> .....]
include/linux/random.h +1 -0
@@ -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 );
Signature · 1 line
2.7.0.rc3.207.g0ac5344
PATCH 2/2 use get_random_long().
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.
--- 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(-)
arch/arm/mm/mmap.c +1 -1
@@ -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 ; }
arch/arm64/mm/mmap.c +2 -2
@@ -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 ; }
arch/mips/mm/mmap.c +2 -2
@@ -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 */
arch/powerpc/kernel/process.c +2 -2
@@ -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 ; }
arch/powerpc/mm/mmap.c +2 -2
@@ -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 ; }
arch/sparc/kernel/sys_sparc_64.c +1 -1
@@ -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
arch/x86/mm/mmap.c +3 -3
@@ -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 ; }
fs/binfmt_elf.c +1 -1
@@ -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 ; }
Signature · 1 line
2.7.0.rc3.207.g0ac5344
Re: [PATCH 2/2] use get_random_long().
Quoted 3 lines · Daniel Cashman
Replace calls to
get_random_int()followed by a cast to (unsigned long) with calls toget_random_long(). Also address shifting bug which, in case of x86 removed entropy mask formmap_rnd_bitsvalues > 31 bits.
I wonder if randomize_range() should be using get_random_long()? Right now, a large range would get truncated:
unsigned longrandomize_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
Quoted 146 lines
Signed-off-by Daniel Cashman <[email protected]>--- 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(-)arch/arm/mm/mmap.c +1
-1@@ -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 ; }arch/arm64/mm/mmap.c +2
-2@@ -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 ; }arch/mips/mm/mmap.c +2
-2@@ -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 */ arch/powerpc/kernel/process.c +2
-2@@ -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 ; }arch/powerpc/mm/mmap.c +2
-2@@ -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 ; }arch/sparc/kernel/sys_sparc_64.c +1
-1@@ -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 arch/x86/mm/mmap.c +3
-3@@ -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 ; }fs/binfmt_elf.c +1
-2@@ -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
Signature · 2 lines
Kees Cook Chrome OS & Brillo Security
Quoted 24 lines · Daniel Cashman
Upstream commit:
d07e22597d1d355829b7b18ac19afa912cf758d1added the ability to choose from a range of values to use for entropy count in generating the random offset to themmap_baseaddress. 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 byget_random_int(), as is already possible for arm64. Add a new function:get_random_long()which more naturally fits with the mmap usage ofget_random_int()but operates exactly the same asget_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 formmap_rnd_bitsover 31 acts as a no-op and effectively disablesmmap_baserandomization.Finally, replace calls to
get_random_int()withget_random_long()where appropriate.Daniel Cashman (2):
drivers: char: random: Addget_random_long().
useget_random_long().
Quoted 11 lines
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!
-Kees
Signature · 2 lines
Kees Cook Chrome OS & Brillo Security
Quoted 37 lines · Kees Cook
Quoted 24 lines · Daniel Cashman
Upstream commit:
d07e22597d1d355829b7b18ac19afa912cf758d1added the ability to choose from a range of values to use for entropy count in generating the random offset to themmap_baseaddress. 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 byget_random_int(), as is already possible for arm64. Add a new function:get_random_long()which more naturally fits with the mmap usage ofget_random_int()but operates exactly the same asget_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 formmap_rnd_bitsover 31 acts as a no-op and effectively disablesmmap_baserandomization.Finally, replace calls to
get_random_int()withget_random_long()where appropriate.Daniel Cashman (2):
drivers: char: random: Addget_random_long().
useget_random_long().Quoted 11 lines
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(-)
Quoted 6 lines
Thanks for fixing this!
Acked-by Kees Cook <[email protected]>-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
Quoted 42 lines · Daniel Cashman
Quoted 41 lines · Kees Cook
Quoted 24 lines · Daniel Cashman
Upstream commit:
d07e22597d1d355829b7b18ac19afa912cf758d1added the ability to choose from a range of values to use for entropy count in generating the random offset to themmap_baseaddress. 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 byget_random_int(), as is already possible for arm64. Add a new function:get_random_long()which more naturally fits with the mmap usage ofget_random_int()but operates exactly the same asget_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 formmap_rnd_bitsover 31 acts as a no-op and effectively disablesmmap_baserandomization.Finally, replace calls to
get_random_int()withget_random_long()where appropriate.Daniel Cashman (2):
drivers: char: random: Addget_random_long().
useget_random_long().Quoted 11 lines
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 <[email protected]>
Quoted 8 lines
Quoted 2 lines
-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
Signature · 2 lines
Kees Cook Chrome OS & Brillo Security