Messages in this thread Patch in this message | | | Date | Thu, 4 Feb 2016 14:38:02 -0800 | Subject | Re: [PATCH 2/2] use get_random_long(). | From | Kees Cook <> |
| |
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
| |