[PATCH 0/2] RFC: Adjust powerpc ASLR elf randomness

This RFC patchset tries to make the powerpc ASLR elf randomness
implementation similar to other ARCHs (like x86).

The 1st patch introduces the support of ARCH_MMAP_RND_BITS in powerpc
mmap implementation to allow a sane balance between increased randomness
in the mmap address of ASLR elfs and increased address space
fragmentation.

The 2nd patch increases the ELF_ET_DYN_BASE value from the current
hardcoded value of 0x2000_0000 to something more practical,
i.e. TASK_SIZE - PAGE_SHIFT (which makes sense especially for
64-bit platforms which would like to utilize more randomization
in the load address of a PIE elf).

I have tested this patchset on 64-bit Fedora and RHEL7 machines/VMs.
Here are the test results and details of the test environment:

1. Create a test PIE program which shows its own memory map:

$ cat show_mmap_pie.c
#include <stdlib.h>
#include <stdio.h>

int main(void){
    char command[1024];
    sprintf(command,"cat /proc/%d/maps",getpid());
    system(command);
    return 0;
}

2. Compile it as a PIE:

$ gcc -o show_mmap_pie -fpie -pie show_mmap_pie.c

3. Before this patchset (on a Fedora-25 PPC64 POWER7 machine):

# ./show_mmap_pie
33dd0000-33de0000 r-xp 00000000 fd:00 1724816                            /root/git/linux/show_mmap_pie
33de0000-33df0000 r--p 00000000 fd:00 1724816                            /root/git/linux/show_mmap_pie
33df0000-33e00000 rw-p 00010000 fd:00 1724816                            /root/git/linux/show_mmap_pie
3fff9d750000-3fff9d940000 r-xp 00000000 fd:00 2753176                    /usr/lib64/power7/libc-2.23.so
3fff9d940000-3fff9d950000 ---p 001f0000 fd:00 2753176                    /usr/lib64/power7/libc-2.23.so
3fff9d950000-3fff9d960000 r--p 001f0000 fd:00 2753176                    /usr/lib64/power7/libc-2.23.so
3fff9d960000-3fff9d970000 rw-p 00200000 fd:00 2753176                    /usr/lib64/power7/libc-2.23.so
3fff9d980000-3fff9d9a0000 r-xp 00000000 00:00 0                          [vdso]
3fff9d9a0000-3fff9d9e0000 r-xp 00000000 fd:00 2625136                    /usr/lib64/ld-2.23.so
3fff9d9e0000-3fff9d9f0000 r--p 00030000 fd:00 2625136                    /usr/lib64/ld-2.23.so
3fff9d9f0000-3fff9da00000 rw-p 00040000 fd:00 2625136                    /usr/lib64/ld-2.23.so
3ffff5280000-3ffff52b0000 rw-p 00000000 00:00 0                          [stack]

As one can notice, the load address even for a 64-bit binary
(show_mmap_pie), is within the 32-bit range.

4. After this patchset (on a Fedora-25 PPC64 POWER7 machine):

# ./show_mmap_pie
3fffad250000-3fffad440000 r-xp 00000000 fd:00 2753176                    /usr/lib64/power7/libc-2.23.so
3fffad440000-3fffad450000 ---p 001f0000 fd:00 2753176                    /usr/lib64/power7/libc-2.23.so
3fffad450000-3fffad460000 r--p 001f0000 fd:00 2753176                    /usr/lib64/power7/libc-2.23.so
3fffad460000-3fffad470000 rw-p 00200000 fd:00 2753176                    /usr/lib64/power7/libc-2.23.so
3fffad480000-3fffad4a0000 r-xp 00000000 00:00 0                          [vdso]
3fffad4a0000-3fffad4e0000 r-xp 00000000 fd:00 2625136                    /usr/lib64/ld-2.23.so
3fffad4e0000-3fffad4f0000 r--p 00030000 fd:00 2625136                    /usr/lib64/ld-2.23.so
3fffad4f0000-3fffad500000 rw-p 00040000 fd:00 2625136                    /usr/lib64/ld-2.23.so
3fffad500000-3fffad510000 r-xp 00000000 fd:00 1724816                    /root/git/linux/show_mmap_pie
3fffad510000-3fffad520000 r--p 00000000 fd:00 1724816                    /root/git/linux/show_mmap_pie
3fffad520000-3fffad530000 rw-p 00010000 fd:00 1724816                    /root/git/linux/show_mmap_pie
3fffe3110000-3fffe3140000 rw-p 00000000 00:00 0                          [stack]

The load address of the elf is now pushed to be in a 64-bit range.

As I have access to limited number of powerpc machines, request folks
having powerpc platforms to try this patchset and share their
test results/issues as well.

Cc: Alexander Graf <redacted>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Anatolij Gustschin <agust@denx.de>
Cc: Alistair Popple <redacted>
Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Vitaly Bordug <redacted>
Cc: Scott Wood <oss@buserror.net>
Cc: Kumar Gala <redacted>
Cc: Daniel Cashman <redacted>
Cc: Kees Cook <redacted>

Bhupesh Sharma (2):
  powerpc: mm: support ARCH_MMAP_RND_BITS
  powerpc: Redefine ELF_ET_DYN_BASE

 arch/powerpc/Kconfig           | 34 ++++++++++++++++++++++++++++++++++
 arch/powerpc/include/asm/elf.h |  2 +-
 arch/powerpc/mm/mmap.c         |  7 ++++---
 3 files changed, 39 insertions(+), 4 deletions(-)

-- 
2.7.4

powerpc: arch_mmap_rnd() uses hard-coded values, (23-PAGE_SHIFT) for
32-bit and (30-PAGE_SHIFT) for 64-bit, to generate the random offset
for the mmap base address.

This value represents a compromise between increased
ASLR effectiveness and avoiding address-space fragmentation.
Replace it with a Kconfig option, which is sensibly bounded, so that
platform developers may choose where to place this compromise.
Keep default values as new minimums.

This patch makes sure that now powerpc mmap arch_mmap_rnd() approach
is similar to other ARCHs like x86, arm64 and arm.

Cc: Alexander Graf <redacted>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Anatolij Gustschin <agust@denx.de>
Cc: Alistair Popple <redacted>
Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Vitaly Bordug <redacted>
Cc: Scott Wood <oss@buserror.net>
Cc: Kumar Gala <redacted>
Cc: Daniel Cashman <redacted>
Cc: Kees Cook <redacted>
Signed-off-by: Bhupesh Sharma <redacted>
---
 arch/powerpc/Kconfig   | 34 ++++++++++++++++++++++++++++++++++
 arch/powerpc/mm/mmap.c |  7 ++++---
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index a8ee573fe610..b4a843f68705 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig

@@ -22,6 +22,38 @@ config MMU
 	bool
 	default y
 
+config ARCH_MMAP_RND_BITS_MIN
+       default 5 if PPC_256K_PAGES && 32BIT
+       default 12 if PPC_256K_PAGES && 64BIT
+       default 7 if PPC_64K_PAGES && 32BIT
+       default 14 if PPC_64K_PAGES && 64BIT
+       default 9 if PPC_16K_PAGES && 32BIT
+       default 16 if PPC_16K_PAGES && 64BIT
+       default 11 if PPC_4K_PAGES && 32BIT
+       default 18 if PPC_4K_PAGES && 64BIT
+
+# max bits determined by the following formula:
+#  VA_BITS - PAGE_SHIFT - 4
+#  for e.g for 64K page and 64BIT = 48 - 16 - 4 = 28
+config ARCH_MMAP_RND_BITS_MAX
+       default 10 if PPC_256K_PAGES && 32BIT
+       default 26 if PPC_256K_PAGES && 64BIT
+       default 12 if PPC_64K_PAGES && 32BIT
+       default 28 if PPC_64K_PAGES && 64BIT
+       default 14 if PPC_16K_PAGES && 32BIT
+       default 30 if PPC_16K_PAGES && 64BIT
+       default 16 if PPC_4K_PAGES && 32BIT
+       default 32 if PPC_4K_PAGES && 64BIT
+
+config ARCH_MMAP_RND_COMPAT_BITS_MIN
+       default 5 if PPC_256K_PAGES
+       default 7 if PPC_64K_PAGES
+       default 9 if PPC_16K_PAGES
+       default 11
+
+config ARCH_MMAP_RND_COMPAT_BITS_MAX
+       default 16
+
 config HAVE_SETUP_PER_CPU_AREA
 	def_bool PPC64
 

@@ -100,6 +132,8 @@ config PPC
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS if !(CPU_LITTLE_ENDIAN && POWER7_CPU)
 	select HAVE_KPROBES
 	select HAVE_ARCH_KGDB
+	select HAVE_ARCH_MMAP_RND_BITS
+	select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
 	select HAVE_KRETPROBES
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_MEMBLOCK

diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
index 2f1e44362198..babf59faab3b 100644
--- a/arch/powerpc/mm/mmap.c
+++ b/arch/powerpc/mm/mmap.c

@@ -60,11 +60,12 @@ unsigned long arch_mmap_rnd(void)
 {
 	unsigned long rnd;
 
-	/* 8MB for 32bit, 1GB for 64bit */
+#ifdef CONFIG_COMPAT
 	if (is_32bit_task())
-		rnd = get_random_long() % (1<<(23-PAGE_SHIFT));
+		rnd = get_random_long() & ((1UL << mmap_rnd_compat_bits) - 1);
 	else
-		rnd = get_random_long() % (1UL<<(30-PAGE_SHIFT));
+#endif
+		rnd = get_random_long() & ((1UL << mmap_rnd_bits) - 1);
 
 	return rnd << PAGE_SHIFT;
 }

-- 
2.7.4

Currently the powerpc arch uses a ELF_ET_DYN_BASE value of 0x20000000
which ends up pushing an elf to a load address which is 32-bit.

On 64-bit platforms, this might be too less especially when one is
trying to increase the randomness of the load address of the ASLR elfs
on such platforms.

This patch makes the powerpc platforms mimic the x86 ones, by ensuring
that the ELF_ET_DYN_BASE is calculated on basis of the current task's
TASK_SIZE.

Cc: Alexander Graf <redacted>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Anatolij Gustschin <agust@denx.de>
Cc: Alistair Popple <redacted>
Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Vitaly Bordug <redacted>
Cc: Scott Wood <oss@buserror.net>
Cc: Kumar Gala <redacted>
Cc: Daniel Cashman <redacted>
Cc: Kees Cook <redacted>
Signed-off-by: Bhupesh Sharma <redacted>
---
 arch/powerpc/include/asm/elf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h
index ee46ffef608e..dd035f6dd782 100644
--- a/arch/powerpc/include/asm/elf.h
+++ b/arch/powerpc/include/asm/elf.h

@@ -28,7 +28,7 @@
    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.  */
 
-#define ELF_ET_DYN_BASE	0x20000000
+#define ELF_ET_DYN_BASE	(TASK_SIZE - PAGE_SIZE)
 
 #define ELF_CORE_EFLAGS (is_elf2_task() ? 2 : 0)
 

-- 
2.7.4

On Thu, Feb 02, 2017 at 11:12:46AM +0530, Bhupesh Sharma wrote:

This RFC patchset tries to make the powerpc ASLR elf randomness
implementation similar to other ARCHs (like x86).

The 1st patch introduces the support of ARCH_MMAP_RND_BITS in powerpc
mmap implementation to allow a sane balance between increased randomness
in the mmap address of ASLR elfs and increased address space
fragmentation.

From what I see we get 28 bits of entropy right for 64k pages

bits as compared to 14 bits earlier?

The 2nd patch increases the ELF_ET_DYN_BASE value from the current
hardcoded value of 0x2000_0000 to something more practical,
i.e. TASK_SIZE - PAGE_SHIFT (which makes sense especially for
64-bit platforms which would like to utilize more randomization
in the load address of a PIE elf).

This helps PIE executables as such and leaves other not impacted?

Balbir Singh

COMPAT is on for ppc64 by default, so we'll end up with COMPAT_BITS same
as before all the time.

Balbir Singh.

Bhupesh Sharma [off-list ref] writes:

powerpc: arch_mmap_rnd() uses hard-coded values, (23-PAGE_SHIFT) for
32-bit and (30-PAGE_SHIFT) for 64-bit, to generate the random offset
for the mmap base address.

This value represents a compromise between increased
ASLR effectiveness and avoiding address-space fragmentation.
Replace it with a Kconfig option, which is sensibly bounded, so that
platform developers may choose where to place this compromise.
Keep default values as new minimums.

This patch makes sure that now powerpc mmap arch_mmap_rnd() approach
is similar to other ARCHs like x86, arm64 and arm.

Thanks for looking at this, it's been on my TODO for a while.

I have a half completed version locally, but never got around to testing
it thoroughly.

This is what I have below, which is a bit neater I think because each
value is only there once (by defaulting to the COMPAT value).

My max values are different to yours, I don't really remember why I
chose those values, so we can argue about which is right.

+config ARCH_MMAP_RND_BITS_MIN
+	# On 64-bit up to 1G of address space (2^30)
+	default 12 if 64BIT && PPC_256K_PAGES 	# 256K (2^18), = 30 - 18 = 12
+	default 14 if 64BIT && PPC_64K_PAGES  	# 64K  (2^16), = 30 - 16 = 14
+	default 16 if 64BIT && PPC_16K_PAGES  	# 16K  (2^14), = 30 - 14 = 16
+	default 18 if 64BIT			# 4K   (2^12), = 30 - 12 = 18
+	default ARCH_MMAP_RND_COMPAT_BITS_MIN
+
+config ARCH_MMAP_RND_BITS_MAX
+	# On 64-bit up to 32T of address space (2^45)
+	default 27 if 64BIT && PPC_256K_PAGES   # 256K (2^18), = 45 - 18 = 27
+	default 29 if 64BIT && PPC_64K_PAGES    # 64K  (2^16), = 45 - 16 = 29
+	default 31 if 64BIT && PPC_16K_PAGES    # 16K  (2^14), = 45 - 14 = 31
+	default 33 if 64BIT			# 4K   (2^12), = 45 - 12 = 33
+	default ARCH_MMAP_RND_COMPAT_BITS_MAX
+
+config ARCH_MMAP_RND_COMPAT_BITS_MIN
+	# Up to 8MB of address space (2^23)
+	default 5 if PPC_256K_PAGES             # 256K (2^18), = 23 - 18 = 5
+	default 7 if PPC_64K_PAGES              # 64K  (2^16), = 23 - 16 = 7
+	default 9 if PPC_16K_PAGES              # 16K  (2^14), = 23 - 14 = 9
+	default 11				# 4K   (2^12), = 23 - 12 = 11
+
+config ARCH_MMAP_RND_COMPAT_BITS_MAX
+	# Up to 2G of address space (2^31)
+	default 13 if PPC_256K_PAGES            # 256K (2^18), = 31 - 18 = 13
+	default 15 if PPC_64K_PAGES             # 64K  (2^16), = 31 - 16 = 15
+	default 17 if PPC_16K_PAGES             # 16K  (2^14), = 31 - 14 = 17
+	default 19				# 4K   (2^12), = 31 - 12 = 19

I also have what I think is a better hunk for that:

 unsigned long arch_mmap_rnd(void)
 {
-	unsigned long rnd;
+	unsigned long shift, rnd;
 
-	/* 8MB for 32bit, 1GB for 64bit */
+	shift = mmap_rnd_bits;
+#ifdef CONFIG_COMPAT
 	if (is_32bit_task())
-		rnd = (unsigned long)get_random_int() % (1<<(23-PAGE_SHIFT));
-	else
-		rnd = (unsigned long)get_random_int() % (1<<(30-PAGE_SHIFT));
+		shift = mmap_rnd_compat_bits;
+#endif
+
+	rnd = (unsigned long)get_random_int() % (1 << shift);

But I'm just nit picking I guess :)

cheers

On Thu, Feb 02, 2017 at 09:23:33PM +1100, Michael Ellerman wrote:

+config ARCH_MMAP_RND_BITS_MIN
+	# On 64-bit up to 1G of address space (2^30)
+	default 12 if 64BIT && PPC_256K_PAGES 	# 256K (2^18), = 30 - 18 = 12
+	default 14 if 64BIT && PPC_64K_PAGES  	# 64K  (2^16), = 30 - 16 = 14
+	default 16 if 64BIT && PPC_16K_PAGES  	# 16K  (2^14), = 30 - 14 = 16
+	default 18 if 64BIT			# 4K   (2^12), = 30 - 12 = 18
+	default ARCH_MMAP_RND_COMPAT_BITS_MIN
+
+config ARCH_MMAP_RND_BITS_MAX
+	# On 64-bit up to 32T of address space (2^45)

I thought it was 64T, TASK_SIZE_USER64 is 2^46?

<snip>

I also have what I think is a better hunk for that:

 unsigned long arch_mmap_rnd(void)
 {
-	unsigned long rnd;
+	unsigned long shift, rnd;
 
-	/* 8MB for 32bit, 1GB for 64bit */
+	shift = mmap_rnd_bits;
+#ifdef CONFIG_COMPAT
 	if (is_32bit_task())
-		rnd = (unsigned long)get_random_int() % (1<<(23-PAGE_SHIFT));
-	else
-		rnd = (unsigned long)get_random_int() % (1<<(30-PAGE_SHIFT));
+		shift = mmap_rnd_compat_bits;
+#endif
+
+	rnd = (unsigned long)get_random_int() % (1 << shift);

But I'm just nit picking I guess :)

No.. the version above is nicer IMHO

Balbir 

On Wed, Feb 1, 2017 at 9:42 PM, Bhupesh Sharma [off-list ref] wrote:

This RFC patchset tries to make the powerpc ASLR elf randomness
implementation similar to other ARCHs (like x86).

The 1st patch introduces the support of ARCH_MMAP_RND_BITS in powerpc
mmap implementation to allow a sane balance between increased randomness
in the mmap address of ASLR elfs and increased address space
fragmentation.

The 2nd patch increases the ELF_ET_DYN_BASE value from the current
hardcoded value of 0x2000_0000 to something more practical,
i.e. TASK_SIZE - PAGE_SHIFT (which makes sense especially for
64-bit platforms which would like to utilize more randomization
in the load address of a PIE elf).

I don't think you want this second patch. Moving ELF_ET_DYN_BASE to
the top of TASK_SIZE means you'll be constantly colliding with stack
and mmap randomization. 0x20000000 is way better since it randomizes
up from there towards the mmap area.

Is there a reason to avoid the 32-bit memory range for the ELF addresses?

-Kees

-- 
Kees Cook
Pixel Security

On Wed, Feb 1, 2017 at 9:42 PM, Bhupesh Sharma [off-list ref] wrote:

Awesome! This looks good to me based on my earlier analysis.

Reviewed-by: Kees Cook <redacted>

-Kees

-- 
Kees Cook
Pixel Security

Hi Kees,

On Thu, Feb 2, 2017 at 7:55 PM, Kees Cook [off-list ref] wrote:

On Wed, Feb 1, 2017 at 9:42 PM, Bhupesh Sharma [off-list ref] wrote:

quoted

powerpc: arch_mmap_rnd() uses hard-coded values, (23-PAGE_SHIFT) for
32-bit and (30-PAGE_SHIFT) for 64-bit, to generate the random offset
for the mmap base address.

This value represents a compromise between increased
ASLR effectiveness and avoiding address-space fragmentation.
Replace it with a Kconfig option, which is sensibly bounded, so that
platform developers may choose where to place this compromise.
Keep default values as new minimums.

This patch makes sure that now powerpc mmap arch_mmap_rnd() approach
is similar to other ARCHs like x86, arm64 and arm.

Cc: Alexander Graf <redacted>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Anatolij Gustschin <agust@denx.de>
Cc: Alistair Popple <redacted>
Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Vitaly Bordug <redacted>
Cc: Scott Wood <oss@buserror.net>
Cc: Kumar Gala <redacted>
Cc: Daniel Cashman <redacted>
Cc: Kees Cook <redacted>
Signed-off-by: Bhupesh Sharma <redacted>
---
 arch/powerpc/Kconfig   | 34 ++++++++++++++++++++++++++++++++++
 arch/powerpc/mm/mmap.c |  7 ++++---
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index a8ee573fe610..b4a843f68705 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig

@@ -22,6 +22,38 @@ config MMU
        bool
        default y

+config ARCH_MMAP_RND_BITS_MIN
+       default 5 if PPC_256K_PAGES && 32BIT
+       default 12 if PPC_256K_PAGES && 64BIT
+       default 7 if PPC_64K_PAGES && 32BIT
+       default 14 if PPC_64K_PAGES && 64BIT
+       default 9 if PPC_16K_PAGES && 32BIT
+       default 16 if PPC_16K_PAGES && 64BIT
+       default 11 if PPC_4K_PAGES && 32BIT
+       default 18 if PPC_4K_PAGES && 64BIT
+
+# max bits determined by the following formula:
+#  VA_BITS - PAGE_SHIFT - 4
+#  for e.g for 64K page and 64BIT = 48 - 16 - 4 = 28
+config ARCH_MMAP_RND_BITS_MAX
+       default 10 if PPC_256K_PAGES && 32BIT
+       default 26 if PPC_256K_PAGES && 64BIT
+       default 12 if PPC_64K_PAGES && 32BIT
+       default 28 if PPC_64K_PAGES && 64BIT
+       default 14 if PPC_16K_PAGES && 32BIT
+       default 30 if PPC_16K_PAGES && 64BIT
+       default 16 if PPC_4K_PAGES && 32BIT
+       default 32 if PPC_4K_PAGES && 64BIT
+
+config ARCH_MMAP_RND_COMPAT_BITS_MIN
+       default 5 if PPC_256K_PAGES
+       default 7 if PPC_64K_PAGES
+       default 9 if PPC_16K_PAGES
+       default 11
+
+config ARCH_MMAP_RND_COMPAT_BITS_MAX
+       default 16
+
 config HAVE_SETUP_PER_CPU_AREA
        def_bool PPC64

@@ -100,6 +132,8 @@ config PPC
        select HAVE_EFFICIENT_UNALIGNED_ACCESS if !(CPU_LITTLE_ENDIAN && POWER7_CPU)
        select HAVE_KPROBES
        select HAVE_ARCH_KGDB
+       select HAVE_ARCH_MMAP_RND_BITS
+       select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT
        select HAVE_KRETPROBES
        select HAVE_ARCH_TRACEHOOK
        select HAVE_MEMBLOCK

diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
index 2f1e44362198..babf59faab3b 100644
--- a/arch/powerpc/mm/mmap.c
+++ b/arch/powerpc/mm/mmap.c

@@ -60,11 +60,12 @@ unsigned long arch_mmap_rnd(void)
 {
        unsigned long rnd;

-       /* 8MB for 32bit, 1GB for 64bit */
+#ifdef CONFIG_COMPAT
        if (is_32bit_task())
-               rnd = get_random_long() % (1<<(23-PAGE_SHIFT));
+               rnd = get_random_long() & ((1UL << mmap_rnd_compat_bits) - 1);
        else
-               rnd = get_random_long() % (1UL<<(30-PAGE_SHIFT));
+#endif
+               rnd = get_random_long() & ((1UL << mmap_rnd_bits) - 1);

        return rnd << PAGE_SHIFT;
 }

Awesome! This looks good to me based on my earlier analysis.

Reviewed-by: Kees Cook <redacted>

Many thanks.
Regards,
Bhupesh

Hi Kees,

Thanks for the review.
Please see my comments inline.

On Thu, Feb 2, 2017 at 7:51 PM, Kees Cook [off-list ref] wrote:

On Wed, Feb 1, 2017 at 9:42 PM, Bhupesh Sharma [off-list ref] wrote:

quoted

This RFC patchset tries to make the powerpc ASLR elf randomness
implementation similar to other ARCHs (like x86).

The 1st patch introduces the support of ARCH_MMAP_RND_BITS in powerpc
mmap implementation to allow a sane balance between increased randomness
in the mmap address of ASLR elfs and increased address space
fragmentation.

The 2nd patch increases the ELF_ET_DYN_BASE value from the current
hardcoded value of 0x2000_0000 to something more practical,
i.e. TASK_SIZE - PAGE_SHIFT (which makes sense especially for
64-bit platforms which would like to utilize more randomization
in the load address of a PIE elf).

I don't think you want this second patch. Moving ELF_ET_DYN_BASE to
the top of TASK_SIZE means you'll be constantly colliding with stack
and mmap randomization. 0x20000000 is way better since it randomizes
up from there towards the mmap area.

Is there a reason to avoid the 32-bit memory range for the ELF addresses?

-Kees

I think you are right. Hmm, I think I was going by my particular use
case which might not be required for generic PPC platforms.

I have one doubt though - I have primarily worked on arm64 and x86
architectures and there I see there 64-bit user space applications
using the 64-bit load addresses/ranges. I am not sure why PPC64 is
different historically.

Regards,
Bhupesh

HI Balbir,

On Thu, Feb 2, 2017 at 2:41 PM, Balbir Singh [off-list ref] wrote:

quoted

@@ -100,6 +132,8 @@ config PPC
      select HAVE_EFFICIENT_UNALIGNED_ACCESS if !(CPU_LITTLE_ENDIAN && POWER7_CPU)
      select HAVE_KPROBES
      select HAVE_ARCH_KGDB
+     select HAVE_ARCH_MMAP_RND_BITS
+     select HAVE_ARCH_MMAP_RND_COMPAT_BITS if COMPAT

COMPAT is on for ppc64 by default, so we'll end up with COMPAT_BITS same
as before all the time.

No, actually the 'ARCH_MMAP_RND_COMPAT_BITS'  values can be changed
after boot using the '/proc/sys/vm/mmap_rnd_compat_bits' tunable:

http://lxr.free-electrons.com/source/arch/Kconfig#L624

Regards,
Bhupesh

Hi Balbir,

On Thu, Feb 2, 2017 at 12:14 PM, Balbir Singh [off-list ref] wrote:

On Thu, Feb 02, 2017 at 11:12:46AM +0530, Bhupesh Sharma wrote:

quoted

This RFC patchset tries to make the powerpc ASLR elf randomness
implementation similar to other ARCHs (like x86).

The 1st patch introduces the support of ARCH_MMAP_RND_BITS in powerpc
mmap implementation to allow a sane balance between increased randomness
in the mmap address of ASLR elfs and increased address space
fragmentation.

From what I see we get 28 bits of entropy right for 64k pages
bits as compared to 14 bits earlier?

That's correct. We can go upto 28-bits of entropy for 64BIT platforms
using 64K pages with the current approach. I see arm64 using > 28 bits
of entropy randomness in some cases, but I think 28-bit MAX entropy is
sensible for 64BIT/64K combination on PPC.

quoted

The 2nd patch increases the ELF_ET_DYN_BASE value from the current
hardcoded value of 0x2000_0000 to something more practical,
i.e. TASK_SIZE - PAGE_SHIFT (which makes sense especially for
64-bit platforms which would like to utilize more randomization
in the load address of a PIE elf).

This helps PIE executables as such and leaves other not impacted?

It basically affects all shared object files (as noted in [1]).
However as Kees noted in one of his reviews, I think this 2nd patch
might not be needed for all generic ppc platforms.

[1] http://lxr.free-electrons.com/source/arch/powerpc/include/asm/elf.h#L26.

Regards,
Bhupesh

On Thu, Feb 2, 2017 at 10:08 AM, Bhupesh Sharma [off-list ref] wrote:

On Thu, Feb 2, 2017 at 7:51 PM, Kees Cook [off-list ref] wrote:

quoted

On Wed, Feb 1, 2017 at 9:42 PM, Bhupesh Sharma [off-list ref] wrote:

quoted

The 2nd patch increases the ELF_ET_DYN_BASE value from the current
hardcoded value of 0x2000_0000 to something more practical,
i.e. TASK_SIZE - PAGE_SHIFT (which makes sense especially for
64-bit platforms which would like to utilize more randomization
in the load address of a PIE elf).

I don't think you want this second patch. Moving ELF_ET_DYN_BASE to
the top of TASK_SIZE means you'll be constantly colliding with stack
and mmap randomization. 0x20000000 is way better since it randomizes
up from there towards the mmap area.

Is there a reason to avoid the 32-bit memory range for the ELF addresses?

I think you are right. Hmm, I think I was going by my particular use
case which might not be required for generic PPC platforms.

I have one doubt though - I have primarily worked on arm64 and x86
architectures and there I see there 64-bit user space applications
using the 64-bit load addresses/ranges. I am not sure why PPC64 is
different historically.

x86's ELF_ET_DYN_BASE is (TASK_SIZE / 3 * 2), so it puts it ET_DYN
base at the top third of the address space. (In theory, this is to
avoid interpreter collisions, but I'm working on removing that
restriction, as it seems pointless.)

Other architectures have small ELF_ET_DYN_BASEs, which is good: it
allows them to have larger entropy for ET_DYN.

-Kees

-- 
Kees Cook
Pixel Security

On 3 Feb 2017 00:49, "Kees Cook" [off-list ref] wrote:

On Thu, Feb 2, 2017 at 10:08 AM, Bhupesh Sharma [off-list ref] wrote:

On Thu, Feb 2, 2017 at 7:51 PM, Kees Cook [off-list ref] wrote:

quoted

On Wed, Feb 1, 2017 at 9:42 PM, Bhupesh Sharma [off-list ref]

wrote:

quoted

quoted

The 2nd patch increases the ELF_ET_DYN_BASE value from the current
hardcoded value of 0x2000_0000 to something more practical,
i.e. TASK_SIZE - PAGE_SHIFT (which makes sense especially for
64-bit platforms which would like to utilize more randomization
in the load address of a PIE elf).

I don't think you want this second patch. Moving ELF_ET_DYN_BASE to
the top of TASK_SIZE means you'll be constantly colliding with stack
and mmap randomization. 0x20000000 is way better since it randomizes
up from there towards the mmap area.

Is there a reason to avoid the 32-bit memory range for the ELF addresses?

I think you are right. Hmm, I think I was going by my particular use
case which might not be required for generic PPC platforms.

I have one doubt though - I have primarily worked on arm64 and x86
architectures and there I see there 64-bit user space applications
using the 64-bit load addresses/ranges. I am not sure why PPC64 is
different historically.

x86's ELF_ET_DYN_BASE is (TASK_SIZE / 3 * 2), so it puts it ET_DYN
base at the top third of the address space. (In theory, this is to
avoid interpreter collisions, but I'm working on removing that
restriction, as it seems pointless.)

Other architectures have small ELF_ET_DYN_BASEs, which is good: it
allows them to have larger entropy for ET_DYN.


Fair enough. I will drop the 2nd patch then and spin a v2.

Regards,
Bhupesh

Balbir Singh [off-list ref] writes:

On Thu, Feb 02, 2017 at 09:23:33PM +1100, Michael Ellerman wrote:

quoted

+config ARCH_MMAP_RND_BITS_MIN
+	# On 64-bit up to 1G of address space (2^30)
+	default 12 if 64BIT && PPC_256K_PAGES 	# 256K (2^18), = 30 - 18 = 12
+	default 14 if 64BIT && PPC_64K_PAGES  	# 64K  (2^16), = 30 - 16 = 14
+	default 16 if 64BIT && PPC_16K_PAGES  	# 16K  (2^14), = 30 - 14 = 16
+	default 18 if 64BIT			# 4K   (2^12), = 30 - 12 = 18
+	default ARCH_MMAP_RND_COMPAT_BITS_MIN
+
+config ARCH_MMAP_RND_BITS_MAX
+	# On 64-bit up to 32T of address space (2^45)

I thought it was 64T, TASK_SIZE_USER64 is 2^46?

The virtual address space is 64T. The comment is talking about how much
can be taking up by the randomisation factor.

cheers

HI Michael,

On Thu, Feb 2, 2017 at 3:53 PM, Michael Ellerman [off-list ref] wrote:

Bhupesh Sharma [off-list ref] writes:

quoted

powerpc: arch_mmap_rnd() uses hard-coded values, (23-PAGE_SHIFT) for
32-bit and (30-PAGE_SHIFT) for 64-bit, to generate the random offset
for the mmap base address.

This value represents a compromise between increased
ASLR effectiveness and avoiding address-space fragmentation.
Replace it with a Kconfig option, which is sensibly bounded, so that
platform developers may choose where to place this compromise.
Keep default values as new minimums.

This patch makes sure that now powerpc mmap arch_mmap_rnd() approach
is similar to other ARCHs like x86, arm64 and arm.

Thanks for looking at this, it's been on my TODO for a while.

I have a half completed version locally, but never got around to testing
it thoroughly.

Sure :)

quoted

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index a8ee573fe610..b4a843f68705 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig

@@ -22,6 +22,38 @@ config MMU
      bool
      default y

+config ARCH_MMAP_RND_BITS_MIN
+       default 5 if PPC_256K_PAGES && 32BIT
+       default 12 if PPC_256K_PAGES && 64BIT
+       default 7 if PPC_64K_PAGES && 32BIT
+       default 14 if PPC_64K_PAGES && 64BIT
+       default 9 if PPC_16K_PAGES && 32BIT
+       default 16 if PPC_16K_PAGES && 64BIT
+       default 11 if PPC_4K_PAGES && 32BIT
+       default 18 if PPC_4K_PAGES && 64BIT
+
+# max bits determined by the following formula:
+#  VA_BITS - PAGE_SHIFT - 4
+#  for e.g for 64K page and 64BIT = 48 - 16 - 4 = 28
+config ARCH_MMAP_RND_BITS_MAX
+       default 10 if PPC_256K_PAGES && 32BIT
+       default 26 if PPC_256K_PAGES && 64BIT
+       default 12 if PPC_64K_PAGES && 32BIT
+       default 28 if PPC_64K_PAGES && 64BIT
+       default 14 if PPC_16K_PAGES && 32BIT
+       default 30 if PPC_16K_PAGES && 64BIT
+       default 16 if PPC_4K_PAGES && 32BIT
+       default 32 if PPC_4K_PAGES && 64BIT
+
+config ARCH_MMAP_RND_COMPAT_BITS_MIN
+       default 5 if PPC_256K_PAGES
+       default 7 if PPC_64K_PAGES
+       default 9 if PPC_16K_PAGES
+       default 11
+
+config ARCH_MMAP_RND_COMPAT_BITS_MAX
+       default 16
+

This is what I have below, which is a bit neater I think because each
value is only there once (by defaulting to the COMPAT value).

My max values are different to yours, I don't really remember why I
chose those values, so we can argue about which is right.

I am not sure how you derived these values, but I am not sure there
should be differences between 64-BIT x86/ARM64 and PPC values for the
MAX values.

+config ARCH_MMAP_RND_BITS_MIN
+       # On 64-bit up to 1G of address space (2^30)
+       default 12 if 64BIT && PPC_256K_PAGES   # 256K (2^18), = 30 - 18 = 12
+       default 14 if 64BIT && PPC_64K_PAGES    # 64K  (2^16), = 30 - 16 = 14
+       default 16 if 64BIT && PPC_16K_PAGES    # 16K  (2^14), = 30 - 14 = 16
+       default 18 if 64BIT                     # 4K   (2^12), = 30 - 12 = 18
+       default ARCH_MMAP_RND_COMPAT_BITS_MIN
+
+config ARCH_MMAP_RND_BITS_MAX
+       # On 64-bit up to 32T of address space (2^45)
+       default 27 if 64BIT && PPC_256K_PAGES   # 256K (2^18), = 45 - 18 = 27
+       default 29 if 64BIT && PPC_64K_PAGES    # 64K  (2^16), = 45 - 16 = 29
+       default 31 if 64BIT && PPC_16K_PAGES    # 16K  (2^14), = 45 - 14 = 31
+       default 33 if 64BIT                     # 4K   (2^12), = 45 - 12 = 33
+       default ARCH_MMAP_RND_COMPAT_BITS_MAX
+
+config ARCH_MMAP_RND_COMPAT_BITS_MIN
+       # Up to 8MB of address space (2^23)
+       default 5 if PPC_256K_PAGES             # 256K (2^18), = 23 - 18 = 5
+       default 7 if PPC_64K_PAGES              # 64K  (2^16), = 23 - 16 = 7
+       default 9 if PPC_16K_PAGES              # 16K  (2^14), = 23 - 14 = 9
+       default 11                              # 4K   (2^12), = 23 - 12 = 11
+
+config ARCH_MMAP_RND_COMPAT_BITS_MAX
+       # Up to 2G of address space (2^31)
+       default 13 if PPC_256K_PAGES            # 256K (2^18), = 31 - 18 = 13
+       default 15 if PPC_64K_PAGES             # 64K  (2^16), = 31 - 16 = 15
+       default 17 if PPC_16K_PAGES             # 16K  (2^14), = 31 - 14 = 17
+       default 19                              # 4K   (2^12), = 31 - 12 = 19

quoted

diff --git a/arch/powerpc/mm/mmap.c b/arch/powerpc/mm/mmap.c
index 2f1e44362198..babf59faab3b 100644
--- a/arch/powerpc/mm/mmap.c
+++ b/arch/powerpc/mm/mmap.c

@@ -60,11 +60,12 @@ unsigned long arch_mmap_rnd(void)
 {
      unsigned long rnd;

-     /* 8MB for 32bit, 1GB for 64bit */
+#ifdef CONFIG_COMPAT
      if (is_32bit_task())
-             rnd = get_random_long() % (1<<(23-PAGE_SHIFT));
+             rnd = get_random_long() & ((1UL << mmap_rnd_compat_bits) - 1);
      else
-             rnd = get_random_long() % (1UL<<(30-PAGE_SHIFT));
+#endif
+             rnd = get_random_long() & ((1UL << mmap_rnd_bits) - 1);

I also have what I think is a better hunk for that:

 unsigned long arch_mmap_rnd(void)
 {
-       unsigned long rnd;
+       unsigned long shift, rnd;

-       /* 8MB for 32bit, 1GB for 64bit */
+       shift = mmap_rnd_bits;
+#ifdef CONFIG_COMPAT
        if (is_32bit_task())
-               rnd = (unsigned long)get_random_int() % (1<<(23-PAGE_SHIFT));
-       else
-               rnd = (unsigned long)get_random_int() % (1<<(30-PAGE_SHIFT));
+               shift = mmap_rnd_compat_bits;
+#endif
+
+       rnd = (unsigned long)get_random_int() % (1 << shift);

But I'm just nit picking I guess :)

I am trying to reuse the existing hunk available for x86 MMAP randomization.
So I am not sure if we really need the hunk mentioned above.

I think we can get this applied to upstream unless someone sees a
breakage on their platform (I have tested this on PPC64 and PPC64LE
setups at my end, but there might be some aberrations on some custom
PPC platforms)

Regards,
Bhupesh

Bhupesh Sharma [off-list ref] writes:

HI Michael,

On Thu, Feb 2, 2017 at 3:53 PM, Michael Ellerman [off-list ref] wrote:

quoted

Bhupesh Sharma [off-list ref] writes:

quoted

powerpc: arch_mmap_rnd() uses hard-coded values, (23-PAGE_SHIFT) for
32-bit and (30-PAGE_SHIFT) for 64-bit, to generate the random offset
for the mmap base address.

This value represents a compromise between increased
ASLR effectiveness and avoiding address-space fragmentation.
Replace it with a Kconfig option, which is sensibly bounded, so that
platform developers may choose where to place this compromise.
Keep default values as new minimums.

This patch makes sure that now powerpc mmap arch_mmap_rnd() approach
is similar to other ARCHs like x86, arm64 and arm.

Thanks for looking at this, it's been on my TODO for a while.

I have a half completed version locally, but never got around to testing
it thoroughly.

Sure :)

quoted

quoted

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index a8ee573fe610..b4a843f68705 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig

@@ -22,6 +22,38 @@ config MMU
      bool
      default y

+config ARCH_MMAP_RND_BITS_MIN
+       default 5 if PPC_256K_PAGES && 32BIT
+       default 12 if PPC_256K_PAGES && 64BIT
+       default 7 if PPC_64K_PAGES && 32BIT
+       default 14 if PPC_64K_PAGES && 64BIT
+       default 9 if PPC_16K_PAGES && 32BIT
+       default 16 if PPC_16K_PAGES && 64BIT
+       default 11 if PPC_4K_PAGES && 32BIT
+       default 18 if PPC_4K_PAGES && 64BIT
+
+# max bits determined by the following formula:
+#  VA_BITS - PAGE_SHIFT - 4
+#  for e.g for 64K page and 64BIT = 48 - 16 - 4 = 28
+config ARCH_MMAP_RND_BITS_MAX
+       default 10 if PPC_256K_PAGES && 32BIT
+       default 26 if PPC_256K_PAGES && 64BIT
+       default 12 if PPC_64K_PAGES && 32BIT
+       default 28 if PPC_64K_PAGES && 64BIT
+       default 14 if PPC_16K_PAGES && 32BIT
+       default 30 if PPC_16K_PAGES && 64BIT
+       default 16 if PPC_4K_PAGES && 32BIT
+       default 32 if PPC_4K_PAGES && 64BIT
+
+config ARCH_MMAP_RND_COMPAT_BITS_MIN
+       default 5 if PPC_256K_PAGES
+       default 7 if PPC_64K_PAGES
+       default 9 if PPC_16K_PAGES
+       default 11
+
+config ARCH_MMAP_RND_COMPAT_BITS_MAX
+       default 16
+

This is what I have below, which is a bit neater I think because each
value is only there once (by defaulting to the COMPAT value).

My max values are different to yours, I don't really remember why I
chose those values, so we can argue about which is right.

I am not sure how you derived these values, but I am not sure there
should be differences between 64-BIT x86/ARM64 and PPC values for the
MAX values.

But your values *are* different to x86 and arm64.

And why would they be the same anyway? x86 has a 47 bit address space,
64-bit powerpc is 46 bits, and arm64 is configurable from 36 to 48 bits.

So your calculations above using VA_BITS = 48 should be using 46 bits.

But if you fixed that, your formula basically gives 1/16th of the
address space as the maximum range. Why is that the right amount?

x86 uses 1/8th, and arm64 uses a mixture of 1/8th and 1/32nd (though
those might be bugs).

My values were more liberal, giving up to half the address space for 32
& 64-bit. Maybe that's too generous, but my rationale was it's up to the
sysadmin to tweak the values and they get to keep the pieces if it
breaks.

quoted

+config ARCH_MMAP_RND_BITS_MAX
+       # On 64-bit up to 32T of address space (2^45)
+       default 27 if 64BIT && PPC_256K_PAGES   # 256K (2^18), = 45 - 18 = 27
+       default 29 if 64BIT && PPC_64K_PAGES    # 64K  (2^16), = 45 - 16 = 29
+       default 31 if 64BIT && PPC_16K_PAGES    # 16K  (2^14), = 45 - 14 = 31
+       default 33 if 64BIT                     # 4K   (2^12), = 45 - 12 = 33
+       default ARCH_MMAP_RND_COMPAT_BITS_MAX

I played with my values a bit and allowing 32T is a little bit nuts. It
means you can actually end up with the adjusted ET_DYN_BASE *above* 32T,
followed by the heap growing up, and the mmap base *below* 32T, growing
down. Which is kinda fun, but definitely breaks a lot of assumptions.

So limiting it to a max of 16T is probably more sensible.

Anyway late here, will think about it some more over the weekend.

cheers

On Fri, Feb 10, 2017 at 4:31 PM, Michael Ellerman [off-list ref] wrote:

Bhupesh Sharma [off-list ref] writes:

quoted

HI Michael,

On Thu, Feb 2, 2017 at 3:53 PM, Michael Ellerman [off-list ref] wrote:

quoted

Bhupesh Sharma [off-list ref] writes:

quoted

powerpc: arch_mmap_rnd() uses hard-coded values, (23-PAGE_SHIFT) for
32-bit and (30-PAGE_SHIFT) for 64-bit, to generate the random offset
for the mmap base address.

This value represents a compromise between increased
ASLR effectiveness and avoiding address-space fragmentation.
Replace it with a Kconfig option, which is sensibly bounded, so that
platform developers may choose where to place this compromise.
Keep default values as new minimums.

This patch makes sure that now powerpc mmap arch_mmap_rnd() approach
is similar to other ARCHs like x86, arm64 and arm.

Thanks for looking at this, it's been on my TODO for a while.

I have a half completed version locally, but never got around to testing
it thoroughly.

Sure :)

quoted

quoted

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index a8ee573fe610..b4a843f68705 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig

@@ -22,6 +22,38 @@ config MMU
      bool
      default y

+config ARCH_MMAP_RND_BITS_MIN
+       default 5 if PPC_256K_PAGES && 32BIT
+       default 12 if PPC_256K_PAGES && 64BIT
+       default 7 if PPC_64K_PAGES && 32BIT
+       default 14 if PPC_64K_PAGES && 64BIT
+       default 9 if PPC_16K_PAGES && 32BIT
+       default 16 if PPC_16K_PAGES && 64BIT
+       default 11 if PPC_4K_PAGES && 32BIT
+       default 18 if PPC_4K_PAGES && 64BIT
+
+# max bits determined by the following formula:
+#  VA_BITS - PAGE_SHIFT - 4
+#  for e.g for 64K page and 64BIT = 48 - 16 - 4 = 28
+config ARCH_MMAP_RND_BITS_MAX
+       default 10 if PPC_256K_PAGES && 32BIT
+       default 26 if PPC_256K_PAGES && 64BIT
+       default 12 if PPC_64K_PAGES && 32BIT
+       default 28 if PPC_64K_PAGES && 64BIT
+       default 14 if PPC_16K_PAGES && 32BIT
+       default 30 if PPC_16K_PAGES && 64BIT
+       default 16 if PPC_4K_PAGES && 32BIT
+       default 32 if PPC_4K_PAGES && 64BIT
+
+config ARCH_MMAP_RND_COMPAT_BITS_MIN
+       default 5 if PPC_256K_PAGES
+       default 7 if PPC_64K_PAGES
+       default 9 if PPC_16K_PAGES
+       default 11
+
+config ARCH_MMAP_RND_COMPAT_BITS_MAX
+       default 16
+

This is what I have below, which is a bit neater I think because each
value is only there once (by defaulting to the COMPAT value).

My max values are different to yours, I don't really remember why I
chose those values, so we can argue about which is right.

I am not sure how you derived these values, but I am not sure there
should be differences between 64-BIT x86/ARM64 and PPC values for the
MAX values.

But your values *are* different to x86 and arm64.

And why would they be the same anyway? x86 has a 47 bit address space,
64-bit powerpc is 46 bits, and arm64 is configurable from 36 to 48 bits.

So your calculations above using VA_BITS = 48 should be using 46 bits.

But if you fixed that, your formula basically gives 1/16th of the
address space as the maximum range. Why is that the right amount?

x86 uses 1/8th, and arm64 uses a mixture of 1/8th and 1/32nd (though
those might be bugs).

My values were more liberal, giving up to half the address space for 32
& 64-bit. Maybe that's too generous, but my rationale was it's up to the
sysadmin to tweak the values and they get to keep the pieces if it
breaks.

I am not sure why would one want to use more than the practical limits
of 1/8th used by x86 - this causes additional burden of address space
fragmentation.

So we need to balance between the randomness increase and the address
space fragmentation.

quoted

quoted

+config ARCH_MMAP_RND_BITS_MAX
+       # On 64-bit up to 32T of address space (2^45)
+       default 27 if 64BIT && PPC_256K_PAGES   # 256K (2^18), = 45 - 18 = 27
+       default 29 if 64BIT && PPC_64K_PAGES    # 64K  (2^16), = 45 - 16 = 29
+       default 31 if 64BIT && PPC_16K_PAGES    # 16K  (2^14), = 45 - 14 = 31
+       default 33 if 64BIT                     # 4K   (2^12), = 45 - 12 = 33
+       default ARCH_MMAP_RND_COMPAT_BITS_MAX

I played with my values a bit and allowing 32T is a little bit nuts. It
means you can actually end up with the adjusted ET_DYN_BASE *above* 32T,
followed by the heap growing up, and the mmap base *below* 32T, growing
down. Which is kinda fun, but definitely breaks a lot of assumptions.

So limiting it to a max of 16T is probably more sensible.

Anyway late here, will think about it some more over the weekend.

A user is always free to tweak the maximum values via specific Kconfig
+ defconfig combinations for their platforms, but why have such large
max values as default for say a embedded PPC64 board which only
supports say 16GB of DDR.

A default max of 33bits for such platforms might be an overkill, while
it might be fine for servers which might have greater DDR
availability.

Regards,
Bhupesh

Hi Michael,

On Fri, Feb 10, 2017 at 4:41 PM, Bhupesh Sharma [off-list ref] wrote:

On Fri, Feb 10, 2017 at 4:31 PM, Michael Ellerman [off-list ref] wrote:

quoted

Bhupesh Sharma [off-list ref] writes:

quoted

HI Michael,

On Thu, Feb 2, 2017 at 3:53 PM, Michael Ellerman [off-list ref] wrote:

quoted

Bhupesh Sharma [off-list ref] writes:

quoted

powerpc: arch_mmap_rnd() uses hard-coded values, (23-PAGE_SHIFT) for
32-bit and (30-PAGE_SHIFT) for 64-bit, to generate the random offset
for the mmap base address.

This value represents a compromise between increased
ASLR effectiveness and avoiding address-space fragmentation.
Replace it with a Kconfig option, which is sensibly bounded, so that
platform developers may choose where to place this compromise.
Keep default values as new minimums.

This patch makes sure that now powerpc mmap arch_mmap_rnd() approach
is similar to other ARCHs like x86, arm64 and arm.

Thanks for looking at this, it's been on my TODO for a while.

I have a half completed version locally, but never got around to testing
it thoroughly.

Sure :)

quoted

quoted

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index a8ee573fe610..b4a843f68705 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig

@@ -22,6 +22,38 @@ config MMU
      bool
      default y

+config ARCH_MMAP_RND_BITS_MIN
+       default 5 if PPC_256K_PAGES && 32BIT
+       default 12 if PPC_256K_PAGES && 64BIT
+       default 7 if PPC_64K_PAGES && 32BIT
+       default 14 if PPC_64K_PAGES && 64BIT
+       default 9 if PPC_16K_PAGES && 32BIT
+       default 16 if PPC_16K_PAGES && 64BIT
+       default 11 if PPC_4K_PAGES && 32BIT
+       default 18 if PPC_4K_PAGES && 64BIT
+
+# max bits determined by the following formula:
+#  VA_BITS - PAGE_SHIFT - 4
+#  for e.g for 64K page and 64BIT = 48 - 16 - 4 = 28
+config ARCH_MMAP_RND_BITS_MAX
+       default 10 if PPC_256K_PAGES && 32BIT
+       default 26 if PPC_256K_PAGES && 64BIT
+       default 12 if PPC_64K_PAGES && 32BIT
+       default 28 if PPC_64K_PAGES && 64BIT
+       default 14 if PPC_16K_PAGES && 32BIT
+       default 30 if PPC_16K_PAGES && 64BIT
+       default 16 if PPC_4K_PAGES && 32BIT
+       default 32 if PPC_4K_PAGES && 64BIT
+
+config ARCH_MMAP_RND_COMPAT_BITS_MIN
+       default 5 if PPC_256K_PAGES
+       default 7 if PPC_64K_PAGES
+       default 9 if PPC_16K_PAGES
+       default 11
+
+config ARCH_MMAP_RND_COMPAT_BITS_MAX
+       default 16
+

This is what I have below, which is a bit neater I think because each
value is only there once (by defaulting to the COMPAT value).

My max values are different to yours, I don't really remember why I
chose those values, so we can argue about which is right.

I am not sure how you derived these values, but I am not sure there
should be differences between 64-BIT x86/ARM64 and PPC values for the
MAX values.

But your values *are* different to x86 and arm64.

And why would they be the same anyway? x86 has a 47 bit address space,
64-bit powerpc is 46 bits, and arm64 is configurable from 36 to 48 bits.

So your calculations above using VA_BITS = 48 should be using 46 bits.

But if you fixed that, your formula basically gives 1/16th of the
address space as the maximum range. Why is that the right amount?

x86 uses 1/8th, and arm64 uses a mixture of 1/8th and 1/32nd (though
those might be bugs).

My values were more liberal, giving up to half the address space for 32
& 64-bit. Maybe that's too generous, but my rationale was it's up to the
sysadmin to tweak the values and they get to keep the pieces if it
breaks.

I am not sure why would one want to use more than the practical limits
of 1/8th used by x86 - this causes additional burden of address space
fragmentation.

So we need to balance between the randomness increase and the address
space fragmentation.

quoted

quoted

quoted

+config ARCH_MMAP_RND_BITS_MAX
+       # On 64-bit up to 32T of address space (2^45)
+       default 27 if 64BIT && PPC_256K_PAGES   # 256K (2^18), = 45 - 18 = 27
+       default 29 if 64BIT && PPC_64K_PAGES    # 64K  (2^16), = 45 - 16 = 29
+       default 31 if 64BIT && PPC_16K_PAGES    # 16K  (2^14), = 45 - 14 = 31
+       default 33 if 64BIT                     # 4K   (2^12), = 45 - 12 = 33
+       default ARCH_MMAP_RND_COMPAT_BITS_MAX

I played with my values a bit and allowing 32T is a little bit nuts. It
means you can actually end up with the adjusted ET_DYN_BASE *above* 32T,
followed by the heap growing up, and the mmap base *below* 32T, growing
down. Which is kinda fun, but definitely breaks a lot of assumptions.

So limiting it to a max of 16T is probably more sensible.

Anyway late here, will think about it some more over the weekend.

A user is always free to tweak the maximum values via specific Kconfig
+ defconfig combinations for their platforms, but why have such large
max values as default for say a embedded PPC64 board which only
supports say 16GB of DDR.

A default max of 33bits for such platforms might be an overkill, while
it might be fine for servers which might have greater DDR
availability.

Ping.

Regards,
Bhupesh

Hi Michael,

On Thu, Feb 16, 2017 at 10:19 AM, Bhupesh Sharma [off-list ref] wrote:

Hi Michael,

On Fri, Feb 10, 2017 at 4:41 PM, Bhupesh Sharma [off-list ref] wrote:

quoted

On Fri, Feb 10, 2017 at 4:31 PM, Michael Ellerman [off-list ref] wrote:

quoted

Bhupesh Sharma [off-list ref] writes:

quoted

HI Michael,

On Thu, Feb 2, 2017 at 3:53 PM, Michael Ellerman [off-list ref] wrote:

quoted

Bhupesh Sharma [off-list ref] writes:

quoted

powerpc: arch_mmap_rnd() uses hard-coded values, (23-PAGE_SHIFT) for
32-bit and (30-PAGE_SHIFT) for 64-bit, to generate the random offset
for the mmap base address.

This value represents a compromise between increased
ASLR effectiveness and avoiding address-space fragmentation.
Replace it with a Kconfig option, which is sensibly bounded, so that
platform developers may choose where to place this compromise.
Keep default values as new minimums.

This patch makes sure that now powerpc mmap arch_mmap_rnd() approach
is similar to other ARCHs like x86, arm64 and arm.

Thanks for looking at this, it's been on my TODO for a while.

I have a half completed version locally, but never got around to testing
it thoroughly.

Sure :)

quoted

quoted

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index a8ee573fe610..b4a843f68705 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig

@@ -22,6 +22,38 @@ config MMU
      bool
      default y

+config ARCH_MMAP_RND_BITS_MIN
+       default 5 if PPC_256K_PAGES && 32BIT
+       default 12 if PPC_256K_PAGES && 64BIT
+       default 7 if PPC_64K_PAGES && 32BIT
+       default 14 if PPC_64K_PAGES && 64BIT
+       default 9 if PPC_16K_PAGES && 32BIT
+       default 16 if PPC_16K_PAGES && 64BIT
+       default 11 if PPC_4K_PAGES && 32BIT
+       default 18 if PPC_4K_PAGES && 64BIT
+
+# max bits determined by the following formula:
+#  VA_BITS - PAGE_SHIFT - 4
+#  for e.g for 64K page and 64BIT = 48 - 16 - 4 = 28
+config ARCH_MMAP_RND_BITS_MAX
+       default 10 if PPC_256K_PAGES && 32BIT
+       default 26 if PPC_256K_PAGES && 64BIT
+       default 12 if PPC_64K_PAGES && 32BIT
+       default 28 if PPC_64K_PAGES && 64BIT
+       default 14 if PPC_16K_PAGES && 32BIT
+       default 30 if PPC_16K_PAGES && 64BIT
+       default 16 if PPC_4K_PAGES && 32BIT
+       default 32 if PPC_4K_PAGES && 64BIT
+
+config ARCH_MMAP_RND_COMPAT_BITS_MIN
+       default 5 if PPC_256K_PAGES
+       default 7 if PPC_64K_PAGES
+       default 9 if PPC_16K_PAGES
+       default 11
+
+config ARCH_MMAP_RND_COMPAT_BITS_MAX
+       default 16
+

This is what I have below, which is a bit neater I think because each
value is only there once (by defaulting to the COMPAT value).

My max values are different to yours, I don't really remember why I
chose those values, so we can argue about which is right.

I am not sure how you derived these values, but I am not sure there
should be differences between 64-BIT x86/ARM64 and PPC values for the
MAX values.

But your values *are* different to x86 and arm64.

And why would they be the same anyway? x86 has a 47 bit address space,
64-bit powerpc is 46 bits, and arm64 is configurable from 36 to 48 bits.

So your calculations above using VA_BITS = 48 should be using 46 bits.

But if you fixed that, your formula basically gives 1/16th of the
address space as the maximum range. Why is that the right amount?

x86 uses 1/8th, and arm64 uses a mixture of 1/8th and 1/32nd (though
those might be bugs).

My values were more liberal, giving up to half the address space for 32
& 64-bit. Maybe that's too generous, but my rationale was it's up to the
sysadmin to tweak the values and they get to keep the pieces if it
breaks.

I am not sure why would one want to use more than the practical limits
of 1/8th used by x86 - this causes additional burden of address space
fragmentation.

So we need to balance between the randomness increase and the address
space fragmentation.

quoted

quoted

quoted

+config ARCH_MMAP_RND_BITS_MAX
+       # On 64-bit up to 32T of address space (2^45)
+       default 27 if 64BIT && PPC_256K_PAGES   # 256K (2^18), = 45 - 18 = 27
+       default 29 if 64BIT && PPC_64K_PAGES    # 64K  (2^16), = 45 - 16 = 29
+       default 31 if 64BIT && PPC_16K_PAGES    # 16K  (2^14), = 45 - 14 = 31
+       default 33 if 64BIT                     # 4K   (2^12), = 45 - 12 = 33
+       default ARCH_MMAP_RND_COMPAT_BITS_MAX

I played with my values a bit and allowing 32T is a little bit nuts. It
means you can actually end up with the adjusted ET_DYN_BASE *above* 32T,
followed by the heap growing up, and the mmap base *below* 32T, growing
down. Which is kinda fun, but definitely breaks a lot of assumptions.

So limiting it to a max of 16T is probably more sensible.

Anyway late here, will think about it some more over the weekend.

A user is always free to tweak the maximum values via specific Kconfig
+ defconfig combinations for their platforms, but why have such large
max values as default for say a embedded PPC64 board which only
supports say 16GB of DDR.

A default max of 33bits for such platforms might be an overkill, while
it might be fine for servers which might have greater DDR
availability.

Ping.

Any updates on this?

Regards,
Bhupesh

On 24 February 2017 6:32:13 pm AEDT, Bhupesh Sharma <bhsharma@redhat=2Ecom=

wrote:
Hi Michael,

On Thu, Feb 16, 2017 at 10:19 AM, Bhupesh Sharma <bhsharma@redhat=2Ecom>
wrote:

quoted

Hi Michael,

On Fri, Feb 10, 2017 at 4:41 PM, Bhupesh Sharma <bhsharma@redhat=2Ecom>

wrote:

quoted

quoted

On Fri, Feb 10, 2017 at 4:31 PM, Michael Ellerman

<mpe@ellerman=2Eid=2Eau> wrote:

quoted

quoted

quoted

Bhupesh Sharma <bhsharma@redhat=2Ecom> writes:

quoted

HI Michael,

On Thu, Feb 2, 2017 at 3:53 PM, Michael Ellerman

<mpe@ellerman=2Eid=2Eau> wrote:

quoted

quoted

quoted

quoted

quoted

Bhupesh Sharma <bhsharma@redhat=2Ecom> writes:

quoted

powerpc: arch_mmap_rnd() uses hard-coded values, (23-PAGE_SHIFT)

for

quoted

quoted

quoted

quoted

quoted

quoted

32-bit and (30-PAGE_SHIFT) for 64-bit, to generate the random

offset

quoted

quoted

quoted

quoted

quoted

quoted

for the mmap base address=2E

This value represents a compromise between increased
ASLR effectiveness and avoiding address-space fragmentation=2E
Replace it with a Kconfig option, which is sensibly bounded, so

that

quoted

quoted

quoted

quoted

quoted

quoted

platform developers may choose where to place this compromise=2E
Keep default values as new minimums=2E

This patch makes sure that now powerpc mmap arch_mmap_rnd()

approach

quoted

quoted

quoted

quoted

quoted

quoted

is similar to other ARCHs like x86, arm64 and arm=2E

Thanks for looking at this, it's been on my TODO for a while=2E

I have a half completed version locally, but never got around to

testing

quoted

quoted

quoted

quoted

quoted

it thoroughly=2E

Sure :)

quoted

quoted

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig

index a8ee573fe610=2E=2Eb4a843f68705 100644

--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig

@@ -22,6 +22,38 @@ config MMU
      bool
      default y

+config ARCH_MMAP_RND_BITS_MIN
+       default 5 if PPC_256K_PAGES && 32BIT
+       default 12 if PPC_256K_PAGES && 64BIT
+       default 7 if PPC_64K_PAGES && 32BIT
+       default 14 if PPC_64K_PAGES && 64BIT
+       default 9 if PPC_16K_PAGES && 32BIT
+       default 16 if PPC_16K_PAGES && 64BIT
+       default 11 if PPC_4K_PAGES && 32BIT
+       default 18 if PPC_4K_PAGES && 64BIT
+
+# max bits determined by the following formula:
+#  VA_BITS - PAGE_SHIFT - 4
+#  for e=2Eg for 64K page and 64BIT =3D 48 - 16 - 4 =3D 28
+config ARCH_MMAP_RND_BITS_MAX
+       default 10 if PPC_256K_PAGES && 32BIT
+       default 26 if PPC_256K_PAGES && 64BIT
+       default 12 if PPC_64K_PAGES && 32BIT
+       default 28 if PPC_64K_PAGES && 64BIT
+       default 14 if PPC_16K_PAGES && 32BIT
+       default 30 if PPC_16K_PAGES && 64BIT
+       default 16 if PPC_4K_PAGES && 32BIT
+       default 32 if PPC_4K_PAGES && 64BIT
+
+config ARCH_MMAP_RND_COMPAT_BITS_MIN
+       default 5 if PPC_256K_PAGES
+       default 7 if PPC_64K_PAGES
+       default 9 if PPC_16K_PAGES
+       default 11
+
+config ARCH_MMAP_RND_COMPAT_BITS_MAX
+       default 16
+

This is what I have below, which is a bit neater I think because

each

quoted

quoted

quoted

quoted

quoted

value is only there once (by defaulting to the COMPAT value)=2E

My max values are different to yours, I don't really remember why

I

quoted

quoted

quoted

quoted

quoted

chose those values, so we can argue about which is right=2E

I am not sure how you derived these values, but I am not sure

there

quoted

quoted

quoted

quoted

should be differences between 64-BIT x86/ARM64 and PPC values for

the

quoted

quoted

quoted

quoted

MAX values=2E

But your values *are* different to x86 and arm64=2E

And why would they be the same anyway? x86 has a 47 bit address

space,

quoted

quoted

quoted

64-bit powerpc is 46 bits, and arm64 is configurable from 36 to 48

bits=2E

quoted

quoted

quoted

So your calculations above using VA_BITS =3D 48 should be using 46

bits=2E

quoted

quoted

quoted

But if you fixed that, your formula basically gives 1/16th of the
address space as the maximum range=2E Why is that the right amount?

x86 uses 1/8th, and arm64 uses a mixture of 1/8th and 1/32nd

(though

quoted

quoted

quoted

those might be bugs)=2E

My values were more liberal, giving up to half the address space

for 32

quoted

quoted

quoted

& 64-bit=2E Maybe that's too generous, but my rationale was it's up

to the

quoted

quoted

quoted

sysadmin to tweak the values and they get to keep the pieces if it
breaks=2E

I am not sure why would one want to use more than the practical

limits

quoted

quoted

of 1/8th used by x86 - this causes additional burden of address

space

quoted

quoted

fragmentation=2E

So we need to balance between the randomness increase and the

address

quoted

quoted

space fragmentation=2E

quoted

quoted

quoted

+config ARCH_MMAP_RND_BITS_MAX
+       # On 64-bit up to 32T of address space (2^45)
+       default 27 if 64BIT && PPC_256K_PAGES   # 256K (2^18), =3D

45 - 18 =3D 27

quoted

quoted

quoted

quoted

quoted

+       default 29 if 64BIT && PPC_64K_PAGES    # 64K  (2^16), =3D

45 - 16 =3D 29

quoted

quoted

quoted

quoted

quoted

+       default 31 if 64BIT && PPC_16K_PAGES    # 16K  (2^14), =3D

45 - 14 =3D 31

quoted

quoted

quoted

quoted

quoted

+       default 33 if 64BIT                     # 4K   (2^12), =3D

45 - 12 =3D 33

quoted

quoted

quoted

quoted

quoted

+       default ARCH_MMAP_RND_COMPAT_BITS_MAX

I played with my values a bit and allowing 32T is a little bit

nuts=2E It

quoted

quoted

quoted

means you can actually end up with the adjusted ET_DYN_BASE *above*

32T,

quoted

quoted

quoted

followed by the heap growing up, and the mmap base *below* 32T,

growing

quoted

quoted

quoted

down=2E Which is kinda fun, but definitely breaks a lot of

assumptions=2E

quoted

quoted

quoted

So limiting it to a max of 16T is probably more sensible=2E

Anyway late here, will think about it some more over the weekend=2E

A user is always free to tweak the maximum values via specific

Kconfig

quoted

quoted

+ defconfig combinations for their platforms, but why have such

large

quoted

quoted

max values as default for say a embedded PPC64 board which only
supports say 16GB of DDR=2E

A default max of 33bits for such platforms might be an overkill,

while

quoted

quoted

it might be fine for servers which might have greater DDR
availability=2E

Ping=2E

Any updates on this?

It missed 4=2E11, we'll get something in for 4=2E12=2E

Cheers
--=20
Sent from my Android phone with K-9 Mail=2E Please excuse my brevity=2E