Fix offset2lib issue for x86*, ARM*, PowerPC and MIPS

[PATCH] Fix offset2lib issue for x86*, ARM*, PowerPC and MIPS

The issue appears on PIE linked executables when all memory areas of a process are randomized. In this case, the attack "offset2lib" de-randomizes all library areas on 64 bit Linux systems in less than one second.

Further details of the PoC attack at:
http://cybersecurity.upv.es/attacks/offset2lib/offset2lib.html

This patch loads the PIE linked executable in a different area than the libraries. The successful fix can be tested with a simple pie compiled application:

$ ./show_mmaps_pie
54859ccd6000-54859ccd7000 r-xp  ...  /tmp/show_mmaps_pie
54859ced6000-54859ced7000 r--p  ...  /tmp/show_mmaps_pie
54859ced7000-54859ced8000 rw-p  ...  /tmp/show_mmaps_pie
7f75be764000-7f75be91f000 r-xp  ...  /lib/x86_64-linux-gnu/libc.so.6
7f75be91f000-7f75beb1f000 ---p  ...  /lib/x86_64-linux-gnu/libc.so.6
7f75beb1f000-7f75beb23000 r--p  ...  /lib/x86_64-linux-gnu/libc.so.6
7f75beb23000-7f75beb25000 rw-p  ...  /lib/x86_64-linux-gnu/libc.so.6
7f75beb25000-7f75beb2a000 rw-p  ...
7f75beb2a000-7f75beb4d000 r-xp  ...  /lib64/ld-linux-x86-64.so.2
7f75bed45000-7f75bed46000 rw-p  ...
7f75bed46000-7f75bed47000 r-xp  ...
7f75bed47000-7f75bed4c000 rw-p  ...
7f75bed4c000-7f75bed4d000 r--p  ...  /lib64/ld-linux-x86-64.so.2
7f75bed4d000-7f75bed4e000 rw-p  ...  /lib64/ld-linux-x86-64.so.2
7f75bed4e000-7f75bed4f000 rw-p  ...
7fffb3741000-7fffb3762000 rw-p  ...  [stack]
7fffb377b000-7fffb377d000 r--p  ...  [vvar]
7fffb377d000-7fffb377f000 r-xp  ...  [vdso]

Once corrected, the PIE linked application is loaded in a different area.

We updated the "Fixing Offset2lib weakness" page:
http://cybersecurity.upv.es/solutions/aslrv2/aslrv2.html

Signed-off-by Hector Marco-Gisbert <[email protected]>

Signed-off-by Ismael Ripoll <[email protected]>

arch/arm/Kconfig +0 -1

index 97d07ed..ee7ea7e 100644
@@ -1,7 +1,6 @@
 config ARM
        bool
        default y
-       select ARCH_BINFMT_ELF_RANDOMIZE_PIE
        select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
        select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
        select ARCH_HAVE_CUSTOM_GPIO_H

arch/arm/include/asm/elf.h +0 -0

index afb9caf..6755cd8 100644
@@ -115,7 +115,8 @@ int dump_task_regs(struct task_struct *t, elf_gregset_t
 *elfregs);
    the loader.  We need to make sure that it is out of the way of the

program
that it will "exec", and that there is sufficient room for the brk. */

arch/arm/include/asm/elf.h +2 -1

-#define ELF_ET_DYN_BASE        (2 * TASK_SIZE / 3)
+extern unsigned long randomize_et_dyn(unsigned long base);
+#define ELF_ET_DYN_BASE        (randomize_et_dyn(2 * TASK_SIZE / 3))
 
 /* When the program starts, a1 contains a pointer to a function to be
    registered with atexit, as per the SVR4 ABI.  A value of 0 means we

arch/arm/mm/mmap.c +21 -0

index 5e85ed3..9177100 100644
@@ -30,6 +30,17 @@ static int mmap_is_legacy(void)
        return sysctl_legacy_va_layout;
 }
 
+static unsigned long mmap_rnd(void)
+{
+       unsigned long rnd = 0;
+
+       /* 8 bits of randomness in 20 address space bits */
+       if (current->flags & PF_RANDOMIZE)
+               rnd = (long)get_random_int() % (1 << 8);
+
+       return rnd << PAGE_SHIFT;
+}
+
 static unsigned long mmap_base(unsigned long rnd)
 {
        unsigned long gap = rlimit(RLIMIT_STACK);

@@ -230,3 +241,13 @@ int devmem_is_allowed(unsigned long pfn)
 }
 
 #endif
+
+unsigned long randomize_et_dyn(unsigned long base)
+{
+       unsigned long ret;
+       if ((current->personality & ADDR_NO_RANDOMIZE) ||
+               !(current->flags & PF_RANDOMIZE))
+               return base;
+       ret = base + mmap_rnd();
+       return (ret > base) ? ret : base;
+}

arch/arm64/Kconfig +0 -1

index b1f9a20..5580d90 100644
@@ -1,6 +1,5 @@
 config ARM64
        def_bool y
-       select ARCH_BINFMT_ELF_RANDOMIZE_PIE
        select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
        select ARCH_HAS_GCOV_PROFILE_ALL
        select ARCH_HAS_SG_CHAIN

arch/arm64/include/asm/elf.h +2 -2

index 1f65be3..01d3aab 100644
@@ -126,7 +126,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
  * that it will "exec", and that there is sufficient room for the brk.
  */
 extern unsigned long randomize_et_dyn(unsigned long base);
-#define ELF_ET_DYN_BASE        (2 * TASK_SIZE_64 / 3)
+#define ELF_ET_DYN_BASE        (randomize_et_dyn(2 * TASK_SIZE_64 / 3))
 
 /*
  * When the program starts, a1 contains a pointer to a function to be

@@ -169,7 +169,7 @@ extern unsigned long arch_randomize_brk(struct mm_struct
 *mm);
 #define COMPAT_ELF_PLATFORM            ("v8l")
 #endif
 
-#define COMPAT_ELF_ET_DYN_BASE         (2 * TASK_SIZE_32 / 3)
+#define COMPAT_ELF_ET_DYN_BASE         (randomize_et_dyn(2 * TASK_SIZE_32 /
 3))
 

/* AArch32 registers. */
#define COMPAT_ELF_NGREG 18

arch/arm64/mm/mmap.c +10 -0

index 54922d1..980110c50 100644
@@ -89,6 +89,16 @@ void arch_pick_mmap_layout(struct mm_struct *mm)
 }
 EXPORT_SYMBOL_GPL(arch_pick_mmap_layout);
 
+unsigned long randomize_et_dyn(unsigned long base)
+{
+       unsigned long ret;
+       if ((current->personality & ADDR_NO_RANDOMIZE) ||
+               !(current->flags & PF_RANDOMIZE))
+               return base;
+       ret = base + mmap_rnd();
+       return (ret > base) ? ret : base;
+}
+
 
 /*
  * You really shouldn't be using read() or write() on /dev/mem.  This might

go

arch/mips/Kconfig +0 -1

index 3289969..31cc248 100644
@@ -23,7 +23,6 @@ config MIPS
        select HAVE_KRETPROBES
        select HAVE_DEBUG_KMEMLEAK
        select HAVE_SYSCALL_TRACEPOINTS
-       select ARCH_BINFMT_ELF_RANDOMIZE_PIE
        select HAVE_ARCH_TRANSPARENT_HUGEPAGE if CPU_SUPPORTS_HUGEPAGES &&

64BIT
select RTC_LIB if !MACH_LOONGSON
select GENERIC_ATOMIC64 if !64BIT

arch/mips/include/asm/elf.h +2 -1

index eb4d95d..fcac4c99 100644
@@ -402,7 +402,8 @@ extern const char *__elf_platform;
    that it will "exec", and that there is sufficient room for the brk. */
 
 #ifndef ELF_ET_DYN_BASE
-#define ELF_ET_DYN_BASE                (TASK_SIZE / 3 * 2)
+extern unsigned long randomize_et_dyn(unsigned long base);
+#define ELF_ET_DYN_BASE                (randomize_et_dyn(TASK_SIZE / 3 *
 2))
 #endif
 

#define ARCH_HAS_SETUP_ADDITIONAL_PAGES 1

arch/mips/mm/mmap.c +10 -0

index f1baadd..20ad644 100644
@@ -196,3 +196,13 @@ int __virt_addr_valid(const volatile void *kaddr)
        return pfn_valid(PFN_DOWN(virt_to_phys(kaddr)));
 }
 EXPORT_SYMBOL_GPL(__virt_addr_valid);
+
+unsigned long randomize_et_dyn(unsigned long base)
+{
+       unsigned long ret;
+       if ((current->personality & ADDR_NO_RANDOMIZE) ||
+               !(current->flags & PF_RANDOMIZE))
+               return base;
+       ret = base + brk_rnd();
+       return (ret > base) ? ret : base;
+}

arch/powerpc/Kconfig +0 -1

index a2a168e..fa4c877 100644
@@ -88,7 +88,6 @@ config PPC
        select ARCH_MIGHT_HAVE_PC_PARPORT
        select ARCH_MIGHT_HAVE_PC_SERIO
        select BINFMT_ELF
-       select ARCH_BINFMT_ELF_RANDOMIZE_PIE
        select OF
        select OF_EARLY_FLATTREE
        select OF_RESERVED_MEM

arch/powerpc/include/asm/elf.h +0 -0

index 57d289a..4080425 100644
@@ -28,7 +28,8 @@
    the loader.  We need to make sure that it is out of the way of the

program
that it will "exec", and that there is sufficient room for the brk. */

arch/powerpc/include/asm/elf.h +2 -1

-#define ELF_ET_DYN_BASE        0x20000000
+extern unsigned long randomize_et_dyn(unsigned long base);
+#define ELF_ET_DYN_BASE        (randomize_et_dyn(0x20000000))
 
 #define ELF_CORE_EFLAGS (is_elf2_task() ? 2 : 0)

arch/powerpc/mm/mmap.c +10 -0

index cb8bdbe..3e642e7 100644
@@ -97,3 +97,13 @@ void arch_pick_mmap_layout(struct mm_struct *mm)
                mm->get_unmapped_area = arch_get_unmapped_area_topdown;
        }
 }
+
+unsigned long randomize_et_dyn(unsigned long base)
+{
+       unsigned long ret;
+       if ((current->personality & ADDR_NO_RANDOMIZE) ||
+               !(current->flags & PF_RANDOMIZE))
+               return base;
+       ret = base + mmap_rnd();
+       return (ret > base) ? ret : base;
+}

arch/x86/Kconfig +0 -1

index ba397bd..dcfe16c 100644
@@ -85,7 +85,6 @@ config X86
        select HAVE_CMPXCHG_DOUBLE
        select HAVE_ARCH_KMEMCHECK
        select HAVE_USER_RETURN_NOTIFIER
-       select ARCH_BINFMT_ELF_RANDOMIZE_PIE
        select HAVE_ARCH_JUMP_LABEL
        select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
        select SPARSE_IRQ

arch/x86/include/asm/elf.h +0 -0

index ca3347a..92c6ac4 100644
@@ -249,7 +249,8 @@ extern int force_personality32;
    the loader.  We need to make sure that it is out of the way of the

program
that it will "exec", and that there is sufficient room for the brk. */

arch/x86/include/asm/elf.h +2 -1

-#define ELF_ET_DYN_BASE                (TASK_SIZE / 3 * 2)
+extern unsigned long randomize_et_dyn(unsigned long base);
+#define ELF_ET_DYN_BASE                (randomize_et_dyn(TASK_SIZE / 3 *

2))

/* This yields a mask that user programs can use to figure out what
instruction set this CPU supports. This could be done in user space,

arch/x86/mm/mmap.c +9 -0

index 919b912..7b86605 100644
@@ -122,3 +122,12 @@ void arch_pick_mmap_layout(struct mm_struct *mm)
                mm->get_unmapped_area = arch_get_unmapped_area_topdown;
        }
 }
+unsigned long randomize_et_dyn(unsigned long base)
+{
+       unsigned long ret;
+       if ((current->personality & ADDR_NO_RANDOMIZE) ||
+               !(current->flags & PF_RANDOMIZE))
+               return base;
+       ret = base + mmap_rnd();
+       return (ret > base) ? ret : base;
+}

fs/Kconfig.binfmt +0 -2

index c055d56..1186190 100644
@@ -27,8 +27,6 @@ config COMPAT_BINFMT_ELF
        bool
        depends on COMPAT && BINFMT_ELF
 
-config ARCH_BINFMT_ELF_RANDOMIZE_PIE
-       bool
 
 config ARCH_BINFMT_ELF_STATE
        bool

fs/binfmt_elf.c +0 -0

index 02b1691..72f7ff5 100644
@@ -908,21 +908,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
                         * default mmap base, as well as whatever program

they
* might try to exec. This is because the brk will
* follow the loader, and is not movable. */

fs/binfmt_elf.c +0 -12

-#ifdef CONFIG_ARCH_BINFMT_ELF_RANDOMIZE_PIE
-                       /* Memory randomization might have been switched off
-                        * in runtime via sysctl or explicit setting of
-                        * personality flags.
-                        * If that is the case, retain the original non-zero
-                        * load_bias value in order to establish proper
-                        * non-randomized mappings.
-                        */
-                       if (current->flags & PF_RANDOMIZE)
-                               load_bias = 0;
-                       else
-                               load_bias = ELF_PAGESTART(ELF_ET_DYN_BASE -

vaddr);

(I've added some additional CCs to make sure the arch maintainers notice this patch.)

This patch seems white-space damaged to me. I had to do a lot of manual editing to get it to apply. Please use "git format-patch", if you're not already. What version of the kernel was this based on?

Quoted 37 lines · Hector Marco

[PATCH] Fix offset2lib issue for x86*, ARM*, PowerPC and MIPS

The issue appears on PIE linked executables when all memory areas of a process are randomized. In this case, the attack "offset2lib" de-randomizes all library areas on 64 bit Linux systems in less than one second.

Further details of the PoC attack at:
http://cybersecurity.upv.es/attacks/offset2lib/offset2lib.html

This patch loads the PIE linked executable in a different area than the libraries. The successful fix can be tested with a simple pie compiled application:

$ ./show_mmaps_pie
54859ccd6000-54859ccd7000 r-xp  ...  /tmp/show_mmaps_pie
54859ced6000-54859ced7000 r--p  ...  /tmp/show_mmaps_pie
54859ced7000-54859ced8000 rw-p  ...  /tmp/show_mmaps_pie
7f75be764000-7f75be91f000 r-xp  ...  /lib/x86_64-linux-gnu/libc.so.6
7f75be91f000-7f75beb1f000 ---p  ...  /lib/x86_64-linux-gnu/libc.so.6
7f75beb1f000-7f75beb23000 r--p  ...  /lib/x86_64-linux-gnu/libc.so.6
7f75beb23000-7f75beb25000 rw-p  ...  /lib/x86_64-linux-gnu/libc.so.6
7f75beb25000-7f75beb2a000 rw-p  ...
7f75beb2a000-7f75beb4d000 r-xp  ...  /lib64/ld-linux-x86-64.so.2
7f75bed45000-7f75bed46000 rw-p  ...
7f75bed46000-7f75bed47000 r-xp  ...
7f75bed47000-7f75bed4c000 rw-p  ...
7f75bed4c000-7f75bed4d000 r--p  ...  /lib64/ld-linux-x86-64.so.2
7f75bed4d000-7f75bed4e000 rw-p  ...  /lib64/ld-linux-x86-64.so.2
7f75bed4e000-7f75bed4f000 rw-p  ...
7fffb3741000-7fffb3762000 rw-p  ...  [stack]
7fffb377b000-7fffb377d000 r--p  ...  [vvar]
7fffb377d000-7fffb377f000 r-xp  ...  [vdso]

Quoted 1 line

Once corrected, the PIE linked application is loaded in a different area.

Thanks for working on this!

Quoted 7 lines

We updated the "Fixing Offset2lib weakness" page:
http://cybersecurity.upv.es/solutions/aslrv2/aslrv2.html

Signed-off-by Hector Marco-Gisbert <[email protected]>

Signed-off-by Ismael Ripoll <[email protected]>

Acked-by Kees Cook <[email protected]>

Quoted 294 lines

arch/arm/Kconfig +0 -1

index 97d07ed..ee7ea7e 100644
@@ -1,7 +1,6 @@
 config ARM
        bool
        default y
-       select ARCH_BINFMT_ELF_RANDOMIZE_PIE
        select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
        select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
        select ARCH_HAVE_CUSTOM_GPIO_H

arch/arm/include/asm/elf.h +0 -0

index afb9caf..6755cd8 100644
@@ -115,7 +115,8 @@ int dump_task_regs(struct task_struct *t, elf_gregset_t
 *elfregs);
    the loader.  We need to make sure that it is out of the way of the

program
that it will "exec", and that there is sufficient room for the brk. */

arch/arm/include/asm/elf.h +2 -1

-#define ELF_ET_DYN_BASE        (2 * TASK_SIZE / 3)
+extern unsigned long randomize_et_dyn(unsigned long base);
+#define ELF_ET_DYN_BASE        (randomize_et_dyn(2 * TASK_SIZE / 3))
 
 /* When the program starts, a1 contains a pointer to a function to be
    registered with atexit, as per the SVR4 ABI.  A value of 0 means we

arch/arm/mm/mmap.c +21 -0

index 5e85ed3..9177100 100644
@@ -30,6 +30,17 @@ static int mmap_is_legacy(void)
        return sysctl_legacy_va_layout;
 }
 
+static unsigned long mmap_rnd(void)
+{
+       unsigned long rnd = 0;
+
+       /* 8 bits of randomness in 20 address space bits */
+       if (current->flags & PF_RANDOMIZE)
+               rnd = (long)get_random_int() % (1 << 8);
+
+       return rnd << PAGE_SHIFT;
+}
+
 static unsigned long mmap_base(unsigned long rnd)
 {
        unsigned long gap = rlimit(RLIMIT_STACK);

@@ -230,3 +241,13 @@ int devmem_is_allowed(unsigned long pfn)
 }
 
 #endif
+
+unsigned long randomize_et_dyn(unsigned long base)
+{
+       unsigned long ret;
+       if ((current->personality & ADDR_NO_RANDOMIZE) ||
+               !(current->flags & PF_RANDOMIZE))
+               return base;
+       ret = base + mmap_rnd();
+       return (ret > base) ? ret : base;
+}

arch/arm64/Kconfig +0 -1

index b1f9a20..5580d90 100644
@@ -1,6 +1,5 @@
 config ARM64
        def_bool y
-       select ARCH_BINFMT_ELF_RANDOMIZE_PIE
        select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
        select ARCH_HAS_GCOV_PROFILE_ALL
        select ARCH_HAS_SG_CHAIN

arch/arm64/include/asm/elf.h +2 -2

index 1f65be3..01d3aab 100644
@@ -126,7 +126,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
  * that it will "exec", and that there is sufficient room for the brk.
  */
 extern unsigned long randomize_et_dyn(unsigned long base);
-#define ELF_ET_DYN_BASE        (2 * TASK_SIZE_64 / 3)
+#define ELF_ET_DYN_BASE        (randomize_et_dyn(2 * TASK_SIZE_64 / 3))
 
 /*
  * When the program starts, a1 contains a pointer to a function to be

@@ -169,7 +169,7 @@ extern unsigned long arch_randomize_brk(struct mm_struct
 *mm);
 #define COMPAT_ELF_PLATFORM            ("v8l")
 #endif
 
-#define COMPAT_ELF_ET_DYN_BASE         (2 * TASK_SIZE_32 / 3)
+#define COMPAT_ELF_ET_DYN_BASE         (randomize_et_dyn(2 * TASK_SIZE_32 /
 3))
 

/* AArch32 registers. */
#define COMPAT_ELF_NGREG 18

arch/arm64/mm/mmap.c +10 -0

index 54922d1..980110c50 100644
@@ -89,6 +89,16 @@ void arch_pick_mmap_layout(struct mm_struct *mm)
 }
 EXPORT_SYMBOL_GPL(arch_pick_mmap_layout);
 
+unsigned long randomize_et_dyn(unsigned long base)
+{
+       unsigned long ret;
+       if ((current->personality & ADDR_NO_RANDOMIZE) ||
+               !(current->flags & PF_RANDOMIZE))
+               return base;
+       ret = base + mmap_rnd();
+       return (ret > base) ? ret : base;
+}
+
 
 /*
  * You really shouldn't be using read() or write() on /dev/mem.  This might

go

arch/mips/Kconfig +0 -1

index 3289969..31cc248 100644
@@ -23,7 +23,6 @@ config MIPS
        select HAVE_KRETPROBES
        select HAVE_DEBUG_KMEMLEAK
        select HAVE_SYSCALL_TRACEPOINTS
-       select ARCH_BINFMT_ELF_RANDOMIZE_PIE
        select HAVE_ARCH_TRANSPARENT_HUGEPAGE if CPU_SUPPORTS_HUGEPAGES &&

64BIT
select RTC_LIB if !MACH_LOONGSON
select GENERIC_ATOMIC64 if !64BIT

arch/mips/include/asm/elf.h +2 -1

index eb4d95d..fcac4c99 100644
@@ -402,7 +402,8 @@ extern const char *__elf_platform;
    that it will "exec", and that there is sufficient room for the brk. */
 
 #ifndef ELF_ET_DYN_BASE
-#define ELF_ET_DYN_BASE                (TASK_SIZE / 3 * 2)
+extern unsigned long randomize_et_dyn(unsigned long base);
+#define ELF_ET_DYN_BASE                (randomize_et_dyn(TASK_SIZE / 3 *
 2))
 #endif
 

#define ARCH_HAS_SETUP_ADDITIONAL_PAGES 1

arch/mips/mm/mmap.c +10 -0

index f1baadd..20ad644 100644
@@ -196,3 +196,13 @@ int __virt_addr_valid(const volatile void *kaddr)
        return pfn_valid(PFN_DOWN(virt_to_phys(kaddr)));
 }
 EXPORT_SYMBOL_GPL(__virt_addr_valid);
+
+unsigned long randomize_et_dyn(unsigned long base)
+{
+       unsigned long ret;
+       if ((current->personality & ADDR_NO_RANDOMIZE) ||
+               !(current->flags & PF_RANDOMIZE))
+               return base;
+       ret = base + brk_rnd();
+       return (ret > base) ? ret : base;
+}

arch/powerpc/Kconfig +0 -1

index a2a168e..fa4c877 100644
@@ -88,7 +88,6 @@ config PPC
        select ARCH_MIGHT_HAVE_PC_PARPORT
        select ARCH_MIGHT_HAVE_PC_SERIO
        select BINFMT_ELF
-       select ARCH_BINFMT_ELF_RANDOMIZE_PIE
        select OF
        select OF_EARLY_FLATTREE
        select OF_RESERVED_MEM

arch/powerpc/include/asm/elf.h +0 -0

index 57d289a..4080425 100644
@@ -28,7 +28,8 @@
    the loader.  We need to make sure that it is out of the way of the

program
that it will "exec", and that there is sufficient room for the brk. */

arch/powerpc/include/asm/elf.h +2 -1

-#define ELF_ET_DYN_BASE        0x20000000
+extern unsigned long randomize_et_dyn(unsigned long base);
+#define ELF_ET_DYN_BASE        (randomize_et_dyn(0x20000000))
 
 #define ELF_CORE_EFLAGS (is_elf2_task() ? 2 : 0)

arch/powerpc/mm/mmap.c +10 -0

index cb8bdbe..3e642e7 100644
@@ -97,3 +97,13 @@ void arch_pick_mmap_layout(struct mm_struct *mm)
                mm->get_unmapped_area = arch_get_unmapped_area_topdown;
        }
 }
+
+unsigned long randomize_et_dyn(unsigned long base)
+{
+       unsigned long ret;
+       if ((current->personality & ADDR_NO_RANDOMIZE) ||
+               !(current->flags & PF_RANDOMIZE))
+               return base;
+       ret = base + mmap_rnd();
+       return (ret > base) ? ret : base;
+}

arch/x86/Kconfig +0 -1

index ba397bd..dcfe16c 100644
@@ -85,7 +85,6 @@ config X86
        select HAVE_CMPXCHG_DOUBLE
        select HAVE_ARCH_KMEMCHECK
        select HAVE_USER_RETURN_NOTIFIER
-       select ARCH_BINFMT_ELF_RANDOMIZE_PIE
        select HAVE_ARCH_JUMP_LABEL
        select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
        select SPARSE_IRQ

arch/x86/include/asm/elf.h +0 -0

index ca3347a..92c6ac4 100644
@@ -249,7 +249,8 @@ extern int force_personality32;
    the loader.  We need to make sure that it is out of the way of the

program
that it will "exec", and that there is sufficient room for the brk. */

arch/x86/include/asm/elf.h +2 -1

-#define ELF_ET_DYN_BASE                (TASK_SIZE / 3 * 2)
+extern unsigned long randomize_et_dyn(unsigned long base);
+#define ELF_ET_DYN_BASE                (randomize_et_dyn(TASK_SIZE / 3 *

2))

/* This yields a mask that user programs can use to figure out what
instruction set this CPU supports. This could be done in user space,

arch/x86/mm/mmap.c +9 -0

index 919b912..7b86605 100644
@@ -122,3 +122,12 @@ void arch_pick_mmap_layout(struct mm_struct *mm)
                mm->get_unmapped_area = arch_get_unmapped_area_topdown;
        }
 }
+unsigned long randomize_et_dyn(unsigned long base)
+{
+       unsigned long ret;
+       if ((current->personality & ADDR_NO_RANDOMIZE) ||
+               !(current->flags & PF_RANDOMIZE))
+               return base;
+       ret = base + mmap_rnd();
+       return (ret > base) ? ret : base;
+}

fs/Kconfig.binfmt +0 -2

index c055d56..1186190 100644
@@ -27,8 +27,6 @@ config COMPAT_BINFMT_ELF
        bool
        depends on COMPAT && BINFMT_ELF
 
-config ARCH_BINFMT_ELF_RANDOMIZE_PIE
-       bool
 
 config ARCH_BINFMT_ELF_STATE
        bool

fs/binfmt_elf.c +0 -0

index 02b1691..72f7ff5 100644
@@ -908,21 +908,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
                         * default mmap base, as well as whatever program

they
* might try to exec. This is because the brk will
* follow the loader, and is not movable. */

fs/binfmt_elf.c +0 -12

-#ifdef CONFIG_ARCH_BINFMT_ELF_RANDOMIZE_PIE
-                       /* Memory randomization might have been switched off
-                        * in runtime via sysctl or explicit setting of
-                        * personality flags.
-                        * If that is the case, retain the original non-zero
-                        * load_bias value in order to establish proper
-                        * non-randomized mappings.
-                        */
-                       if (current->flags & PF_RANDOMIZE)
-                               load_bias = 0;
-                       else
-                               load_bias = ELF_PAGESTART(ELF_ET_DYN_BASE -

vaddr);

fs/binfmt_elf.c

Quoted 6 lines

fs/binfmt_elf.c +0 -2

-#else
                        load_bias = ELF_PAGESTART(ELF_ET_DYN_BASE - vaddr);
-#endif
                }
 
                error = elf_map(bprm->file, load_bias + vaddr, elf_ppnt,

I think this is much cleaner now without ARCH_BINFMT_ELF_RANDOMIZE_PIE. I imagine there could be some follow-up cleanups to standardize (or at least clearly document) the intended levels of entropy in the 4 ASLR regions on each architecture, as it currently varies a bit.

Thanks!

-Kees

--
Kees Cook
Chrome OS Security

(quoted diff) +36 -0

+unsigned long randomize_et_dyn(unsigned long base)
+{
+	unsigned long ret;
+	if ((current->personality & ADDR_NO_RANDOMIZE) ||
+		!(current->flags & PF_RANDOMIZE))
+		return base;
+	ret = base + mmap_rnd();
+	return (ret > base) ? ret : base;
+}
 
+unsigned long randomize_et_dyn(unsigned long base)
+{
+	unsigned long ret;
+	if ((current->personality & ADDR_NO_RANDOMIZE) ||
+		!(current->flags & PF_RANDOMIZE))
+		return base;
+	ret = base + mmap_rnd();
+	return (ret > base) ? ret : base;
+}
 
+unsigned long randomize_et_dyn(unsigned long base)
+{
+	unsigned long ret;
+	if ((current->personality & ADDR_NO_RANDOMIZE) ||
+		!(current->flags & PF_RANDOMIZE))
+		return base;
+	ret = base + brk_rnd();
+	return (ret > base) ? ret : base;
+}
 
+unsigned long randomize_et_dyn(unsigned long base)
+{
+	unsigned long ret;
+	if ((current->personality & ADDR_NO_RANDOMIZE) ||
+		!(current->flags & PF_RANDOMIZE))
+		return base;
+	ret = base + mmap_rnd();
+	return (ret > base) ? ret : base;
+}

Quoted 39 lines · Hector Marco Gisbert

(quoted diff) +36 -0

+unsigned long randomize_et_dyn(unsigned long base)
+{
+	unsigned long ret;
+	if ((current->personality & ADDR_NO_RANDOMIZE) ||
+		!(current->flags & PF_RANDOMIZE))
+		return base;
+	ret = base + mmap_rnd();
+	return (ret > base) ? ret : base;
+}
 
+unsigned long randomize_et_dyn(unsigned long base)
+{
+	unsigned long ret;
+	if ((current->personality & ADDR_NO_RANDOMIZE) ||
+		!(current->flags & PF_RANDOMIZE))
+		return base;
+	ret = base + mmap_rnd();
+	return (ret > base) ? ret : base;
+}
 
+unsigned long randomize_et_dyn(unsigned long base)
+{
+	unsigned long ret;
+	if ((current->personality & ADDR_NO_RANDOMIZE) ||
+		!(current->flags & PF_RANDOMIZE))
+		return base;
+	ret = base + brk_rnd();
+	return (ret > base) ? ret : base;
+}
 
+unsigned long randomize_et_dyn(unsigned long base)
+{
+	unsigned long ret;
+	if ((current->personality & ADDR_NO_RANDOMIZE) ||
+		!(current->flags & PF_RANDOMIZE))
+		return base;
+	ret = base + mmap_rnd();
+	return (ret > base) ? ret : base;
+}

Review

Quoted 9 lines

(quoted diff) +9 -0

+unsigned long randomize_et_dyn(unsigned long base)
+{
+	unsigned long ret;
+	if ((current->personality & ADDR_NO_RANDOMIZE) ||
+		!(current->flags & PF_RANDOMIZE))
+		return base;
+	ret = base + mmap_rnd();
+	return (ret > base) ? ret : base;
+}

That pointless repetition should be avoided.

From: Andrew Morton<[email protected]>
Subject: fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix

Consolidate randomize_et_dyn() implementations into fs/binfmt_elf.c.

There doesn't seem to be a compile-time way of making randomize_et_dyn() go away on architectures which don't need it, so mark it __weak to cause it to be discarded at link time.

Cc "H. Peter Anvin"<[email protected]>

Cc Benjamin Herrenschmidt<[email protected]>

Cc Catalin Marinas<[email protected]>

Cc Hector Marco Gisbert<[email protected]>

Cc Hector Marco-Gisbert<[email protected]>

Cc Ingo Molnar<[email protected]>

Cc Ismael Ripoll<[email protected]>

Cc Kees Cook<[email protected]>

Cc Ralf Baechle<[email protected]>

Cc Russell King<[email protected]>

Cc Thomas Gleixner<[email protected]>

Cc Will Deacon<[email protected]>

Signed-off-by Andrew Morton<[email protected]>

arch/arm64/include/asm/elf.h renamed from arch/arm64/include/asm/elf.h~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +0 -0

   *
   * This program is free software; you can redistribute it and/or modify
   * it under the terms of the GNU General Public License version 2 as

Does that __weak trick even work?

arch/arm64/Kconfig line 1

Quoted 6 lines · Andrew Morton

diff -puN arch/arm64/Kconfig~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix arch/arm64/Kconfig

arch/arm64/Kconfig renamed from arch/arm64/Kconfig~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +1 -1

@@ -1,4 +1,4 @@
-config ARM64
+qconfig ARM64

Is this a typo?

diff -puN fs/binfmt_elf.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix fs/binfmt_elf.c

fs/binfmt_elf.c renamed from fs/binfmt_elf.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +11 -0

@@ -22,6 +22,7 @@
 #include <linux/slab.h>
 #include <linux/personality.h>
 #include <linux/elfcore.h>
+#include <linux/elf-randomization.h>
 #include <linux/init.h>
 #include <linux/highuid.h>
 #include <linux/compiler.h>

@@ -2300,6 +2301,26 @@ out:
 
 #endif		/* CONFIG_ELF_CORE */
 
+/* Not all architectures implement mmap_rnd() */
+unsigned long __weak mmap_rnd(void)
+{
+}
+
+/*
+ * Not all architectures use randomize_et_dyn(), so use __weak to let the
+ * linker omit it from vmlinux
+ */
+unsigned long __weak randomize_et_dyn(unsigned long base)

diff -puN fs/binfmt_elf.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix fs/binfmt_elf.c

fs/binfmt_elf.c renamed from fs/binfmt_elf.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +11 -0

@@ -22,6 +22,7 @@
 #include <linux/slab.h>
 #include <linux/personality.h>
 #include <linux/elfcore.h>
+#include <linux/elf-randomization.h>
 #include <linux/init.h>
 #include <linux/highuid.h>
 #include <linux/compiler.h>

@@ -2300,6 +2301,26 @@ out:
 
 #endif		/* CONFIG_ELF_CORE */
 
+/* Not all architectures implement mmap_rnd() */
+unsigned long __weak mmap_rnd(void)
+{
+}
+
+/*
+ * Not all architectures use randomize_et_dyn(), so use __weak to let the
+ * linker omit it from vmlinux
+ */
+unsigned long __weak randomize_et_dyn(unsigned long base)

Quoted 43 lines · On Tue, 24 Feb 2015 08:39:06 +0100 Ingo Molnar <[email protected]> wrote:

Quoted 39 lines · Hector Marco Gisbert

(quoted diff) +36 -0

+unsigned long randomize_et_dyn(unsigned long base)
+{
+   unsigned long ret;
+   if ((current->personality & ADDR_NO_RANDOMIZE) ||
+           !(current->flags & PF_RANDOMIZE))
+           return base;
+   ret = base + mmap_rnd();
+   return (ret > base) ? ret : base;
+}
 
+unsigned long randomize_et_dyn(unsigned long base)
+{
+   unsigned long ret;
+   if ((current->personality & ADDR_NO_RANDOMIZE) ||
+           !(current->flags & PF_RANDOMIZE))
+           return base;
+   ret = base + mmap_rnd();
+   return (ret > base) ? ret : base;
+}
 
+unsigned long randomize_et_dyn(unsigned long base)
+{
+   unsigned long ret;
+   if ((current->personality & ADDR_NO_RANDOMIZE) ||
+           !(current->flags & PF_RANDOMIZE))
+           return base;
+   ret = base + brk_rnd();
+   return (ret > base) ? ret : base;
+}
 
+unsigned long randomize_et_dyn(unsigned long base)
+{
+   unsigned long ret;
+   if ((current->personality & ADDR_NO_RANDOMIZE) ||
+           !(current->flags & PF_RANDOMIZE))
+           return base;
+   ret = base + mmap_rnd();
+   return (ret > base) ? ret : base;
+}

Quoted 11 lines

fs/binfmt_elf.c out: · line 2322

Quoted 9 lines

fs/binfmt_elf.c renamed from fs/binfmt_elf.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +9 -0

+unsigned long randomize_et_dyn(unsigned long base)
+{
+   unsigned long ret;
+   if ((current->personality & ADDR_NO_RANDOMIZE) ||
+           !(current->flags & PF_RANDOMIZE))
+           return base;
+   ret = base + mmap_rnd();
+   return (ret > base) ? ret : base;
+}

That pointless repetition should be avoided.

That's surprisingly hard!

After renaming mips brk_rnd() to mmap_rnd() I had a shot. I'm not very confident in the result. Does that __weak trick even work?

Someone tell me how important Hector's patch is?

From: Andrew Morton <[email protected]>
Subject: fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix

Consolidate randomize_et_dyn() implementations into fs/binfmt_elf.c.

There doesn't seem to be a compile-time way of making randomize_et_dyn() go away on architectures which don't need it, so mark it __weak to cause it to be discarded at link time.

Cc "H. Peter Anvin" <[email protected]>

Cc Benjamin Herrenschmidt <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Hector Marco Gisbert <[email protected]>

Cc Hector Marco-Gisbert <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Ismael Ripoll <[email protected]>

Cc Kees Cook <[email protected]>

Cc Ralf Baechle <[email protected]>

Cc Russell King <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Will Deacon <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

---

 arch/arm/include/asm/elf.h        |    3 +--
 arch/arm/mm/mmap.c                |   13 ++-----------
 arch/arm64/Kconfig                |    2 +-
 arch/arm64/include/asm/elf.h      |    3 +--
 arch/arm64/mm/mmap.c              |   14 ++------------
 arch/mips/include/asm/elf.h       |    1 -
 arch/mips/mm/mmap.c               |   13 ++-----------
 arch/powerpc/include/asm/elf.h    |    2 --
 arch/powerpc/mm/mmap.c            |   13 ++-----------
 arch/x86/include/asm/elf.h        |    2 --
 arch/x86/mm/mmap.c                |   12 ++----------
 fs/binfmt_elf.c                   |   21 +++++++++++++++++++++
 include/linux/elf-randomization.h |    7 +++++++
 13 files changed, 41 insertions(+), 65 deletions(-)

diff -puN arch/arm/Kconfig~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix arch/arm/Kconfig diff -puN arch/arm/include/asm/elf.h~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix arch/arm/include/asm/elf.h

arch/arm/include/asm/elf.h renamed from arch/arm/include/asm/elf.h~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +1 -2

@@ -2,7 +2,7 @@
 #define __ASMARM_ELF_H
 
 #include <asm/hwcap.h>
-
+#include <linux/elf-randomization.h>
 /*
  * ELF register definitions..
  */

@@ -115,7 +115,6 @@ int dump_task_regs(struct task_struct *t
    the loader.  We need to make sure that it is out of the way of the program
    that it will "exec", and that there is sufficient room for the brk.  */
 
-extern unsigned long randomize_et_dyn(unsigned long base);
 #define ELF_ET_DYN_BASE        (randomize_et_dyn(2 * TASK_SIZE / 3))
 
 /* When the program starts, a1 contains a pointer to a function to be

diff -puN arch/arm/mm/mmap.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix arch/arm/mm/mmap.c

arch/arm/mm/mmap.c renamed from arch/arm/mm/mmap.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +2 -11

@@ -7,6 +7,7 @@
 #include <linux/shm.h>
 #include <linux/sched.h>
 #include <linux/io.h>
+#include <linux/elf-randomization.h>
 #include <linux/personality.h>
 #include <linux/random.h>
 #include <asm/cachetype.h>

@@ -30,7 +31,7 @@ static int mmap_is_legacy(void)
        return sysctl_legacy_va_layout;
 }
 
-static unsigned long mmap_rnd(void)
+unsigned long mmap_rnd(void)
 {
        unsigned long rnd = 0;
 

@@ -241,13 +242,3 @@ int devmem_is_allowed(unsigned long pfn)
 }
 
 #endif
-
-unsigned long randomize_et_dyn(unsigned long base)
-{
-       unsigned long ret;
-       if ((current->personality & ADDR_NO_RANDOMIZE) ||
-               !(current->flags & PF_RANDOMIZE))
-               return base;
-       ret = base + mmap_rnd();
-       return (ret > base) ? ret : base;
-}

diff -puN arch/arm64/Kconfig~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix arch/arm64/Kconfig

arch/arm64/Kconfig renamed from arch/arm64/Kconfig~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +1 -1

@@ -1,4 +1,4 @@
-config ARM64
+qconfig ARM64
        def_bool y
        select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
        select ARCH_HAS_GCOV_PROFILE_ALL

diff -puN arch/arm64/include/asm/elf.h~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix arch/arm64/include/asm/elf.h

arch/arm64/include/asm/elf.h renamed from arch/arm64/include/asm/elf.h~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +1 -2

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012 ARM Ltd.
+ * Copyright (C) 20q12 ARM Ltd.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as

@@ -125,7 +125,6 @@ typedef struct user_fpsimd_state elf_fpr
  * the loader.  We need to make sure that it is out of the way of the program
  * that it will "exec", and that there is sufficient room for the brk.
  */
-extern unsigned long randomize_et_dyn(unsigned long base);
 #define ELF_ET_DYN_BASE        (randomize_et_dyn(2 * TASK_SIZE_64 / 3))
 
 /*

diff -puN arch/arm64/mm/mmap.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix arch/arm64/mm/mmap.c

arch/arm64/mm/mmap.c renamed from arch/arm64/mm/mmap.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +2 -12

@@ -17,6 +17,7 @@
  */
 
 #include <linux/elf.h>
+#include <linux/elf-randomization.h>
 #include <linux/fs.h>
 #include <linux/mm.h>
 #include <linux/mman.h>

@@ -47,7 +48,7 @@ static int mmap_is_legacy(void)
        return sysctl_legacy_va_layout;
 }
 
-static unsigned long mmap_rnd(void)
+unsigned long mmap_rnd(void)
 {
        unsigned long rnd = 0;
 

@@ -89,17 +90,6 @@ void arch_pick_mmap_layout(struct mm_str
 }
 EXPORT_SYMBOL_GPL(arch_pick_mmap_layout);
 
-unsigned long randomize_et_dyn(unsigned long base)
-{
-       unsigned long ret;
-       if ((current->personality & ADDR_NO_RANDOMIZE) ||
-               !(current->flags & PF_RANDOMIZE))
-               return base;
-       ret = base + mmap_rnd();
-       return (ret > base) ? ret : base;
-}
-
-
 /*
  * You really shouldn't be using read() or write() on /dev/mem.  This might go
  * away in the future.

diff -puN arch/mips/Kconfig~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix arch/mips/Kconfig diff -puN arch/mips/include/asm/elf.h~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix arch/mips/include/asm/elf.h

arch/mips/include/asm/elf.h renamed from arch/mips/include/asm/elf.h~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +0 -1

@@ -402,7 +402,6 @@ extern const char *__elf_platform;
    that it will "exec", and that there is sufficient room for the brk. */
 
 #ifndef ELF_ET_DYN_BASE
-extern unsigned long randomize_et_dyn(unsigned long base);
 #define ELF_ET_DYN_BASE                (randomize_et_dyn(TASK_SIZE / 3 * 2))
 #endif
 

diff -puN arch/mips/mm/mmap.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix arch/mips/mm/mmap.c

arch/mips/mm/mmap.c renamed from arch/mips/mm/mmap.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +2 -11

@@ -9,6 +9,7 @@
 #include <linux/compiler.h>
 #include <linux/errno.h>
 #include <linux/mm.h>
+#include <linux/elf-randomization.h>
 #include <linux/mman.h>
 #include <linux/module.h>
 #include <linux/personality.h>

@@ -164,7 +165,7 @@ void arch_pick_mmap_layout(struct mm_str
        }
 }
 
-static inline unsigned long mmap_rnd(void)
+unsigned long mmap_rnd(void)
 {
        unsigned long rnd = get_random_int();
 

@@ -196,13 +197,3 @@ int __virt_addr_valid(const volatile voi
        return pfn_valid(PFN_DOWN(virt_to_phys(kaddr)));
 }
 EXPORT_SYMBOL_GPL(__virt_addr_valid);
-
-unsigned long randomize_et_dyn(unsigned long base)
-{
-       unsigned long ret;
-       if ((current->personality & ADDR_NO_RANDOMIZE) ||
-               !(current->flags & PF_RANDOMIZE))
-               return base;
-       ret = base + brk_rnd();
-       return (ret > base) ? ret : base;
-}

diff -puN arch/powerpc/Kconfig~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix arch/powerpc/Kconfig diff -puN arch/powerpc/include/asm/elf.h~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix arch/powerpc/include/asm/elf.h

arch/powerpc/include/asm/elf.h renamed from arch/powerpc/include/asm/elf.h~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +0 -2

@@ -27,8 +27,6 @@
    use of this is to invoke "./ld.so someprog" to test out a new version of
    the loader.  We need to make sure that it is out of the way of the program
    that it will "exec", and that there is sufficient room for the brk.  */
-
-extern unsigned long randomize_et_dyn(unsigned long base);
 #define ELF_ET_DYN_BASE        (randomize_et_dyn(0x20000000))
 
 #define ELF_CORE_EFLAGS (is_elf2_task() ? 2 : 0)

diff -puN arch/powerpc/mm/mmap.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix arch/powerpc/mm/mmap.c

arch/powerpc/mm/mmap.c renamed from arch/powerpc/mm/mmap.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +2 -11

@@ -24,6 +24,7 @@
 
 #include <linux/personality.h>
 #include <linux/mm.h>
+#include <linux/elf-randomization.h>
 #include <linux/random.h>
 #include <linux/sched.h>
 

@@ -53,7 +54,7 @@ static inline int mmap_is_legacy(void)
        return sysctl_legacy_va_layout;
 }
 
-static unsigned long mmap_rnd(void)
+unsigned long mmap_rnd(void)
 {
        unsigned long rnd = 0;
 

@@ -97,13 +98,3 @@ void arch_pick_mmap_layout(struct mm_str
                mm->get_unmapped_area = arch_get_unmapped_area_topdown;
        }
 }
-
-unsigned long randomize_et_dyn(unsigned long base)
-{
-       unsigned long ret;
-       if ((current->personality & ADDR_NO_RANDOMIZE) ||
-               !(current->flags & PF_RANDOMIZE))
-               return base;
-       ret = base + mmap_rnd();
-       return (ret > base) ? ret : base;
-}

diff -puN arch/x86/Kconfig~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix arch/x86/Kconfig diff -puN arch/x86/include/asm/elf.h~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix arch/x86/include/asm/elf.h

arch/x86/include/asm/elf.h renamed from arch/x86/include/asm/elf.h~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +0 -2

@@ -248,8 +248,6 @@ extern int force_personality32;
    use of this is to invoke "./ld.so someprog" to test out a new version of
    the loader.  We need to make sure that it is out of the way of the program
    that it will "exec", and that there is sufficient room for the brk.  */
-
-extern unsigned long randomize_et_dyn(unsigned long base);
 #define ELF_ET_DYN_BASE                (randomize_et_dyn(TASK_SIZE / 3 * 2))
 
 /* This yields a mask that user programs can use to figure out what

diff -puN arch/x86/mm/mmap.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix arch/x86/mm/mmap.c

arch/x86/mm/mmap.c renamed from arch/x86/mm/mmap.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +2 -10

@@ -29,6 +29,7 @@
 #include <linux/random.h>
 #include <linux/limits.h>
 #include <linux/sched.h>
+#include <linux/elf-randomization.h>
 #include <asm/elf.h>
 
 struct va_alignment __read_mostly va_align = {

@@ -65,7 +66,7 @@ static int mmap_is_legacy(void)
        return sysctl_legacy_va_layout;
 }
 
-static unsigned long mmap_rnd(void)
+unsigned long mmap_rnd(void)
 {
        unsigned long rnd = 0;
 

@@ -122,12 +123,3 @@ void arch_pick_mmap_layout(struct mm_str
                mm->get_unmapped_area = arch_get_unmapped_area_topdown;
        }
 }
-unsigned long randomize_et_dyn(unsigned long base)
-{
-       unsigned long ret;
-       if ((current->personality & ADDR_NO_RANDOMIZE) ||
-               !(current->flags & PF_RANDOMIZE))
-               return base;
-       ret = base + mmap_rnd();
-       return (ret > base) ? ret : base;
-}

diff -puN fs/Kconfig.binfmt~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix fs/Kconfig.binfmt diff -puN fs/binfmt_elf.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix fs/binfmt_elf.c

fs/binfmt_elf.c renamed from fs/binfmt_elf.c~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix +21 -0

@@ -22,6 +22,7 @@
 #include <linux/slab.h>
 #include <linux/personality.h>
 #include <linux/elfcore.h>
+#include <linux/elf-randomization.h>
 #include <linux/init.h>
 #include <linux/highuid.h>
 #include <linux/compiler.h>

@@ -2300,6 +2301,26 @@ out:
 
 #endif         /* CONFIG_ELF_CORE */
 
+/* Not all architectures implement mmap_rnd() */
+unsigned long __weak mmap_rnd(void)
+{
+}
+
+/*
+ * Not all architectures use randomize_et_dyn(), so use __weak to let the
+ * linker omit it from vmlinux
+ */
+unsigned long __weak randomize_et_dyn(unsigned long base)
+{
+       unsigned long ret;
+
+       if ((current->personality & ADDR_NO_RANDOMIZE) ||
+               !(current->flags & PF_RANDOMIZE))
+               return base;
+       ret = base + mmap_rnd();
+       return max(ret, base);
+}
+
 static int __init init_elf_binfmt(void)
 {
        register_binfmt(&elf_format);

diff -puN include/linux/elf.h~fix-offset2lib-issue-for-x86-arm-powerpc-and-mips-fix include/linux/elf.h diff -puN /dev/null include/linux/elf-randomization.h

include/linux/elf-randomization.h new file +7 -0

@@ -0,0 +1,7 @@
+#ifndef __ELF_RANDOMIZATION_H
+#define __ELF_RANDOMIZATION_H
+
+unsigned long randomize_et_dyn(unsigned long base);
+unsigned long mmap_rnd(void);
+
+#endif /* __ELF_RANDOMIZATION_H */

_

Quoted 16 lines

(quoted diff) +16 -0

+/* Not all architectures implement mmap_rnd() */
+unsigned long __weak mmap_rnd(void)
+{
+}
+
+/*
+ * Not all architectures use randomize_et_dyn(), so use __weak to let the
+ * linker omit it from vmlinux
+ */
+unsigned long __weak randomize_et_dyn(unsigned long base)
+{
+	unsigned long ret;
+
+	if ((current->personality & ADDR_NO_RANDOMIZE) ||
+		!(current->flags & PF_RANDOMIZE))
+		return base;

Review

Quoted 4 lines

(quoted diff) +4 -0

+	ret = base + mmap_rnd();
+	return max(ret, base);
+}
+

Didn't we have some trouble with some compilers when the weak function (mmap_rnd) was defined and used in the same file i.e. the wrong one was used?

Quoted 6 lines

Quoted 1 line

That pointless repetition should be avoided.

That's surprisingly hard!

After renaming mips brk_rnd() to mmap_rnd() I had a shot. I'm not very confident in the result. Does that __weak trick even work?

In theory, it shouldn't be needed since only randomize_et_dyn will call mmap_rnd, and only architectures that use randomize_et_dyn will call it ... and will define mmap_rnd.

Agh, no, please let's avoid the CONFIG addition.

Hector mentioned in private mail that he was looking at an alternative that adds exec_base to struct mm which would avoid all this insanity.

Can't we do something like:

#ifndef mmap_rnd

# define mmap_rnd 0
#endif

Quoted 19 lines · Andrew Morton

Quoted 1 line · On Thu, 26 Feb 2015 15:37:37 -0800 Kees Cook <[email protected]> wrote:

Agh, no, please let's avoid the CONFIG addition.

That is precisely how we do this.

Quoted 8 lines

Hector mentioned in private mail that he was looking at an alternative that adds exec_base to struct mm which would avoid all this insanity.

Can't we do something like:

#ifndef mmap_rnd

# define mmap_rnd 0
#endif

Sure, and sprinkle

#define mmap_rnd mmap_rnd

Quoted 3 lines

in five arch header files where nobody thinks to look.

For better or for worse, we are consolidating such things into arch/*/Kconfig.

Okay, fair enough. Even with your configs (though shouldn't they be ARCH_HAS or just HAVE?) I've now stumbled over the issue that we can't put randomize_et_dyn in binfmt_elf because it conflicts with linking against compat_binfmt_elf.