[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 8 Feb 2017 18:23:21 +0530
From: Bhupesh Sharma <bhsharma@...hat.com>
To: Michael Ellerman <mpe@...erman.id.au>
Cc: linuxppc-dev@...ts.ozlabs.org, kernel-hardening@...ts.openwall.com,
Daniel Cashman <dcashman@...gle.com>, Bhupesh SHARMA <bhupesh.linux@...il.com>,
Kees Cook <keescook@...omium.org>, Alexander Graf <agraf@...e.com>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>, Paul Mackerras <paulus@...ba.org>,
Anatolij Gustschin <agust@...x.de>, Alistair Popple <alistair@...ple.id.au>,
Matt Porter <mporter@...nel.crashing.org>, Vitaly Bordug <vitb@...nel.crashing.org>,
Scott Wood <oss@...error.net>, Kumar Gala <galak@...nel.crashing.org>,
Daniel Cashman <dcashman@...roid.com>
Subject: Re: [PATCH 1/2] powerpc: mm: support ARCH_MMAP_RND_BITS
HI Michael,
On Thu, Feb 2, 2017 at 3:53 PM, Michael Ellerman <mpe@...erman.id.au> wrote:
> Bhupesh Sharma <bhsharma@...hat.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.
Sure :)
>> 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
>
>
>
>> 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
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
