From: hpa@zytor.com (H. Peter Anvin)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v6 4/4] x86: mm: support ARCH_MMAP_RND_BITS.
Date: Mon, 14 Dec 2015 10:58:28 -0800 [thread overview]
Message-ID: <566F1154.7030703@zytor.com> (raw)
In-Reply-To: <1449856338-30984-5-git-send-email-dcashman@android.com>
On 12/11/15 09:52, Daniel Cashman wrote:
> From: dcashman <dcashman@google.com>
>
> x86: arch_mmap_rnd() uses hard-coded values, 8 for 32-bit and 28 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.
>
> Signed-off-by: Daniel Cashman <dcashman@android.com>
OK, this is around the time when I make a lecture about the danger of
expecting the compiler to make certain transformations:
> diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> index 844b06d..647fecf 100644
> --- a/arch/x86/mm/mmap.c
> +++ b/arch/x86/mm/mmap.c
> @@ -69,14 +69,14 @@ unsigned long arch_mmap_rnd(void)
> {
> unsigned long rnd;
>
> - /*
> - * 8 bits of randomness in 32bit mmaps, 20 address space bits
> - * 28 bits of randomness in 64bit mmaps, 40 address space bits
> - */
> if (mmap_is_ia32())
> - rnd = (unsigned long)get_random_int() % (1<<8);
> +#ifdef CONFIG_COMPAT
> + rnd = (unsigned long)get_random_int() % (1 << mmap_rnd_compat_bits);
> +#else
> + rnd = (unsigned long)get_random_int() % (1 << mmap_rnd_bits);
> +#endif
> else
> - rnd = (unsigned long)get_random_int() % (1<<28);
> + rnd = (unsigned long)get_random_int() % (1 << mmap_rnd_bits);
>
> return rnd << PAGE_SHIFT;
> }
>
Now, you and I know that both variants can be implemented with a simple
AND, but I have a strong suspicion that once this is turned into a
variable, this will in fact be changed from an AND to a divide.
So I'd prefer to use the
"get_random_int() & ((1UL << mmap_rnd_bits) - 1)" construct instead.
-hpa
WARNING: multiple messages have this Message-ID (diff)
From: "H. Peter Anvin" <hpa@zytor.com>
To: Daniel Cashman <dcashman@android.com>, linux-kernel@vger.kernel.org
Cc: linux@arm.linux.org.uk, akpm@linux-foundation.org,
keescook@chromium.org, mingo@kernel.org,
linux-arm-kernel@lists.infradead.org, corbet@lwn.net,
dzickus@redhat.com, ebiederm@xmission.com, xypron.glpk@gmx.de,
jpoimboe@redhat.com, kirill.shutemov@linux.intel.com,
n-horiguchi@ah.jp.nec.com, aarcange@redhat.com, mgorman@suse.de,
tglx@linutronix.de, rientjes@google.com, linux-mm@kvack.org,
linux-doc@vger.kernel.org, salyzyn@android.com, jeffv@google.com,
nnk@google.com, catalin.marinas@arm.com, will.deacon@arm.com,
x86@kernel.org, hecmargi@upv.es, bp@suse.de, dcashman@google.com,
arnd@arndb.de, jonathanh@nvidia.com
Subject: Re: [PATCH v6 4/4] x86: mm: support ARCH_MMAP_RND_BITS.
Date: Mon, 14 Dec 2015 10:58:28 -0800 [thread overview]
Message-ID: <566F1154.7030703@zytor.com> (raw)
In-Reply-To: <1449856338-30984-5-git-send-email-dcashman@android.com>
On 12/11/15 09:52, Daniel Cashman wrote:
> From: dcashman <dcashman@google.com>
>
> x86: arch_mmap_rnd() uses hard-coded values, 8 for 32-bit and 28 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.
>
> Signed-off-by: Daniel Cashman <dcashman@android.com>
OK, this is around the time when I make a lecture about the danger of
expecting the compiler to make certain transformations:
> diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> index 844b06d..647fecf 100644
> --- a/arch/x86/mm/mmap.c
> +++ b/arch/x86/mm/mmap.c
> @@ -69,14 +69,14 @@ unsigned long arch_mmap_rnd(void)
> {
> unsigned long rnd;
>
> - /*
> - * 8 bits of randomness in 32bit mmaps, 20 address space bits
> - * 28 bits of randomness in 64bit mmaps, 40 address space bits
> - */
> if (mmap_is_ia32())
> - rnd = (unsigned long)get_random_int() % (1<<8);
> +#ifdef CONFIG_COMPAT
> + rnd = (unsigned long)get_random_int() % (1 << mmap_rnd_compat_bits);
> +#else
> + rnd = (unsigned long)get_random_int() % (1 << mmap_rnd_bits);
> +#endif
> else
> - rnd = (unsigned long)get_random_int() % (1<<28);
> + rnd = (unsigned long)get_random_int() % (1 << mmap_rnd_bits);
>
> return rnd << PAGE_SHIFT;
> }
>
Now, you and I know that both variants can be implemented with a simple
AND, but I have a strong suspicion that once this is turned into a
variable, this will in fact be changed from an AND to a divide.
So I'd prefer to use the
"get_random_int() & ((1UL << mmap_rnd_bits) - 1)" construct instead.
-hpa
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
WARNING: multiple messages have this Message-ID (diff)
From: "H. Peter Anvin" <hpa@zytor.com>
To: Daniel Cashman <dcashman@android.com>, linux-kernel@vger.kernel.org
Cc: linux@arm.linux.org.uk, akpm@linux-foundation.org,
keescook@chromium.org, mingo@kernel.org,
linux-arm-kernel@lists.infradead.org, corbet@lwn.net,
dzickus@redhat.com, ebiederm@xmission.com, xypron.glpk@gmx.de,
jpoimboe@redhat.com, kirill.shutemov@linux.intel.com,
n-horiguchi@ah.jp.nec.com, aarcange@redhat.com, mgorman@suse.de,
tglx@linutronix.de, rientjes@google.com, linux-mm@kvack.org,
linux-doc@vger.kernel.org, salyzyn@android.com, jeffv@google.com,
nnk@google.com, catalin.marinas@arm.com, will.deacon@arm.com,
x86@kernel.org, hecmargi@upv.es, bp@suse.de, dcashman@google.com,
arnd@arndb.de, jonathanh@nvidia.com
Subject: Re: [PATCH v6 4/4] x86: mm: support ARCH_MMAP_RND_BITS.
Date: Mon, 14 Dec 2015 10:58:28 -0800 [thread overview]
Message-ID: <566F1154.7030703@zytor.com> (raw)
In-Reply-To: <1449856338-30984-5-git-send-email-dcashman@android.com>
On 12/11/15 09:52, Daniel Cashman wrote:
> From: dcashman <dcashman@google.com>
>
> x86: arch_mmap_rnd() uses hard-coded values, 8 for 32-bit and 28 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.
>
> Signed-off-by: Daniel Cashman <dcashman@android.com>
OK, this is around the time when I make a lecture about the danger of
expecting the compiler to make certain transformations:
> diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> index 844b06d..647fecf 100644
> --- a/arch/x86/mm/mmap.c
> +++ b/arch/x86/mm/mmap.c
> @@ -69,14 +69,14 @@ unsigned long arch_mmap_rnd(void)
> {
> unsigned long rnd;
>
> - /*
> - * 8 bits of randomness in 32bit mmaps, 20 address space bits
> - * 28 bits of randomness in 64bit mmaps, 40 address space bits
> - */
> if (mmap_is_ia32())
> - rnd = (unsigned long)get_random_int() % (1<<8);
> +#ifdef CONFIG_COMPAT
> + rnd = (unsigned long)get_random_int() % (1 << mmap_rnd_compat_bits);
> +#else
> + rnd = (unsigned long)get_random_int() % (1 << mmap_rnd_bits);
> +#endif
> else
> - rnd = (unsigned long)get_random_int() % (1<<28);
> + rnd = (unsigned long)get_random_int() % (1 << mmap_rnd_bits);
>
> return rnd << PAGE_SHIFT;
> }
>
Now, you and I know that both variants can be implemented with a simple
AND, but I have a strong suspicion that once this is turned into a
variable, this will in fact be changed from an AND to a divide.
So I'd prefer to use the
"get_random_int() & ((1UL << mmap_rnd_bits) - 1)" construct instead.
-hpa
next prev parent reply other threads:[~2015-12-14 18:58 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-11 17:52 [PATCH v6 0/4] Allow customizable random offset to mmap_base address Daniel Cashman
2015-12-11 17:52 ` Daniel Cashman
2015-12-11 17:52 ` Daniel Cashman
2015-12-11 17:52 ` [PATCH v6 1/4] mm: mmap: Add new /proc tunable for mmap_base ASLR Daniel Cashman
2015-12-11 17:52 ` Daniel Cashman
2015-12-11 17:52 ` Daniel Cashman
2015-12-11 17:52 ` [PATCH v6 2/4] arm: mm: support ARCH_MMAP_RND_BITS Daniel Cashman
2015-12-11 17:52 ` Daniel Cashman
2015-12-11 17:52 ` Daniel Cashman
2015-12-11 17:52 ` [PATCH v6 3/4] arm64: " Daniel Cashman
2015-12-11 17:52 ` Daniel Cashman
2015-12-11 17:52 ` Daniel Cashman
2015-12-11 17:52 ` [PATCH v6 4/4] x86: " Daniel Cashman
2015-12-11 17:52 ` Daniel Cashman
2015-12-11 17:52 ` Daniel Cashman
2015-12-14 18:58 ` H. Peter Anvin [this message]
2015-12-14 18:58 ` H. Peter Anvin
2015-12-14 18:58 ` H. Peter Anvin
2015-12-14 20:51 ` Daniel Cashman
2015-12-14 20:51 ` Daniel Cashman
2015-12-14 20:51 ` Daniel Cashman
2015-12-14 11:19 ` [PATCH v6 3/4] arm64: " Will Deacon
2015-12-14 11:19 ` Will Deacon
2015-12-14 11:19 ` Will Deacon
2015-12-14 20:45 ` Daniel Cashman
2015-12-14 20:45 ` Daniel Cashman
2015-12-14 20:45 ` Daniel Cashman
2015-12-11 21:02 ` [PATCH v6 0/4] Allow customizable random offset to mmap_base address Kees Cook
2015-12-11 21:02 ` Kees Cook
2015-12-11 21:02 ` Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=566F1154.7030703@zytor.com \
--to=hpa@zytor.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.