Re: [PATCH REBASE v4 05/14] arm64, mm: Make randomization selected by generic topdown mmap layout

[Alexandre Ghiti]

On 7/24/19 7:11 PM, Luis Chamberlain wrote:

On Wed, Jul 24, 2019 at 01:58:41AM -0400, Alexandre Ghiti wrote:

diff --git a/mm/util.c b/mm/util.c
index 0781e5575cb3..16f1e56e2996 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -321,7 +321,15 @@ unsigned long randomize_stack_top(unsigned long stack_top)
  }
  
  #ifdef CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT

-#ifdef CONFIG_ARCH_HAS_ELF_RANDOMIZE
+unsigned long arch_randomize_brk(struct mm_struct *mm)
+{
+   /* Is the current task 32bit ? */
+   if (!IS_ENABLED(CONFIG_64BIT) || is_compat_task())
+   return randomize_page(mm->brk, SZ_32M);
+
+   return randomize_page(mm->brk, SZ_1G);
+}
+
  unsigned long arch_mmap_rnd(void)
  {
unsigned long rnd;
@@ -335,7 +343,6 @@ unsigned long arch_mmap_rnd(void)
  
  	return rnd << PAGE_SHIFT;

  }

So arch_randomize_brk is no longer ifdef'd around
CONFIG_ARCH_HAS_ELF_RANDOMIZE either and yet the header
still has it. Is that intentional?



Yes, CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT selects 
CONFIG_ARCH_HAS_ELF_RANDOMIZE, that's what's new about v4: the generic

functions proposed in this series come with elf randomization.


Alex



   Luis

___
linux-riscv mailing list
linux-ri...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH REBASE v4 05/14] arm64, mm: Make randomization selected by generic topdown mmap layout

[Luis Chamberlain]

On Wed, Jul 24, 2019 at 01:58:41AM -0400, Alexandre Ghiti wrote:
> diff --git a/mm/util.c b/mm/util.c
> index 0781e5575cb3..16f1e56e2996 100644
> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -321,7 +321,15 @@ unsigned long randomize_stack_top(unsigned long 
> stack_top)
>  }
>  
>  #ifdef CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
> -#ifdef CONFIG_ARCH_HAS_ELF_RANDOMIZE
> +unsigned long arch_randomize_brk(struct mm_struct *mm)
> +{
> + /* Is the current task 32bit ? */
> + if (!IS_ENABLED(CONFIG_64BIT) || is_compat_task())
> + return randomize_page(mm->brk, SZ_32M);
> +
> + return randomize_page(mm->brk, SZ_1G);
> +}
> +
>  unsigned long arch_mmap_rnd(void)
>  {
>   unsigned long rnd;
> @@ -335,7 +343,6 @@ unsigned long arch_mmap_rnd(void)
>  
>   return rnd << PAGE_SHIFT;
>  }

So arch_randomize_brk is no longer ifdef'd around
CONFIG_ARCH_HAS_ELF_RANDOMIZE either and yet the header
still has it. Is that intentional?

  Luis

[PATCH REBASE v4 05/14] arm64, mm: Make randomization selected by generic topdown mmap layout

[Alexandre Ghiti]

This commits selects ARCH_HAS_ELF_RANDOMIZE when an arch uses the generic
topdown mmap layout functions so that this security feature is on by
default.
Note that this commit also removes the possibility for arm64 to have elf
randomization and no MMU: without MMU, the security added by randomization
is worth nothing.

Signed-off-by: Alexandre Ghiti 
Acked-by: Catalin Marinas 
Acked-by: Kees Cook 
Reviewed-by: Christoph Hellwig 
---
 arch/Kconfig|  1 +
 arch/arm64/Kconfig  |  1 -
 arch/arm64/kernel/process.c |  8 
 mm/util.c   | 11 +--
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index a0bb6fa4d381..d4c1f0551dfe 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -705,6 +705,7 @@ config HAVE_ARCH_COMPAT_MMAP_BASES
 config ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
bool
depends on MMU
+   select ARCH_HAS_ELF_RANDOMIZE
 
 config HAVE_COPY_THREAD_TLS
bool
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 14a194e63458..399f595ef852 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -16,7 +16,6 @@ config ARM64
select ARCH_HAS_DMA_MMAP_PGPROT
select ARCH_HAS_DMA_PREP_COHERENT
select ARCH_HAS_ACPI_TABLE_UPGRADE if ACPI
-   select ARCH_HAS_ELF_RANDOMIZE
select ARCH_HAS_FAST_MULTIPLIER
select ARCH_HAS_FORTIFY_SOURCE
select ARCH_HAS_GCOV_PROFILE_ALL
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 6a869d9f304f..3f59d0d1632e 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -524,14 +524,6 @@ unsigned long arch_align_stack(unsigned long sp)
return sp & ~0xf;
 }
 
-unsigned long arch_randomize_brk(struct mm_struct *mm)
-{
-   if (is_compat_task())
-   return randomize_page(mm->brk, SZ_32M);
-   else
-   return randomize_page(mm->brk, SZ_1G);
-}
-
 /*
  * Called from setup_new_exec() after (COMPAT_)SET_PERSONALITY.
  */
diff --git a/mm/util.c b/mm/util.c
index 0781e5575cb3..16f1e56e2996 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -321,7 +321,15 @@ unsigned long randomize_stack_top(unsigned long stack_top)
 }
 
 #ifdef CONFIG_ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT
-#ifdef CONFIG_ARCH_HAS_ELF_RANDOMIZE
+unsigned long arch_randomize_brk(struct mm_struct *mm)
+{
+   /* Is the current task 32bit ? */
+   if (!IS_ENABLED(CONFIG_64BIT) || is_compat_task())
+   return randomize_page(mm->brk, SZ_32M);
+
+   return randomize_page(mm->brk, SZ_1G);
+}
+
 unsigned long arch_mmap_rnd(void)
 {
unsigned long rnd;
@@ -335,7 +343,6 @@ unsigned long arch_mmap_rnd(void)
 
return rnd << PAGE_SHIFT;
 }
-#endif /* CONFIG_ARCH_HAS_ELF_RANDOMIZE */
 
 static int mmap_is_legacy(struct rlimit *rlim_stack)
 {
-- 
2.20.1