Bhupesh Sharma <bhsha...@redhat.com> 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.
> 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.
+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
> 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 :)
cheers
