Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1258439 > unrolled thread

[PATCH 1/2] mm: mmap: Add new /proc tunable for mmap_base ASLR.

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/2] mm: mmap: Add new /proc tunable for mmap_base ASLR. Daniel Cashman <dcashman@android.com> - 2015-10-28 22:30 +0100
    [PATCH 2/2] arm: mm: support ARCH_MMAP_RND_BITS. Daniel Cashman <dcashman@android.com> - 2015-10-28 22:30 +0100
    Re: [PATCH 1/2] mm: mmap: Add new /proc tunable for mmap_base ASLR. ebiederm@xmission.com (Eric W. Biederman) - 2015-10-29 00:50 +0100
      Re: [PATCH 1/2] mm: mmap: Add new /proc tunable for mmap_base ASLR. Jeffrey Vander Stoep <jeffv@google.com> - 2015-10-29 01:10 +0100
        Re: [PATCH 1/2] mm: mmap: Add new /proc tunable for mmap_base ASLR. Dan Cashman <dcashman@android.com> - 2015-10-29 01:40 +0100
          Re: [PATCH 1/2] mm: mmap: Add new /proc tunable for mmap_base ASLR. ebiederm@xmission.com (Eric W. Biederman) - 2015-10-29 05:00 +0100
            Re: [PATCH 1/2] mm: mmap: Add new /proc tunable for mmap_base ASLR. Daniel Cashman <dcashman@android.com> - 2015-10-29 23:10 +0100

#1258439 — [PATCH 1/2] mm: mmap: Add new /proc tunable for mmap_base ASLR.

From: dcashman <dcashman@google.com>

ASLR currently only uses 8 bits to generate the random offset for the
mmap base address on 32 bit architectures. This value was chosen to
prevent a poorly chosen value from dividing the address space in such
a way as to prevent large allocations. This may not be an issue on all
platforms. Allow the specification of a minimum number of bits so that
platforms desiring greater ASLR protection may determine where to place
the trade-off.

Signed-off-by: Daniel Cashman <dcashman@google.com>
---
 Documentation/sysctl/kernel.txt | 14 ++++++++++++++
 include/linux/mm.h              |  6 ++++++
 kernel/sysctl.c                 | 11 +++++++++++
 3 files changed, 31 insertions(+)

diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index 6fccb69..0d4ca53 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -41,6 +41,7 @@ show up in /proc/sys/kernel:
 - kptr_restrict
 - kstack_depth_to_print       [ X86 only ]
 - l2cr                        [ PPC only ]
+- mmap_rnd_bits
 - modprobe                    ==> Documentation/debugging-modules.txt
 - modules_disabled
 - msg_next_id		      [ sysv ipc ]
@@ -391,6 +392,19 @@ This flag controls the L2 cache of G3 processor boards. If
 
 ==============================================================
 
+mmap_rnd_bits:
+
+This value can be used to select the number of bits to use to
+determine the random offset to the base address of vma regions
+resulting from mmap allocations on architectures which support
+tuning address space randomization.  This value will be bounded
+by the architecture's minimum and maximum supported values.
+
+This value can be changed after boot using the
+/proc/sys/kernel/mmap_rnd_bits tunable
+
+==============================================================
+
 modules_disabled:
 
 A toggle value indicating if modules are allowed to be loaded
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 80001de..15b083a 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -51,6 +51,12 @@ extern int sysctl_legacy_va_layout;
 #define sysctl_legacy_va_layout 0
 #endif
 
+#ifdef CONFIG_ARCH_MMAP_RND_BITS
+extern int mmap_rnd_bits_min;
+extern int mmap_rnd_bits_max;
+extern int mmap_rnd_bits;
+#endif
+
 #include <asm/page.h>
 #include <asm/pgtable.h>
 #include <asm/processor.h>
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index e69201d..37e657a 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1139,6 +1139,17 @@ static struct ctl_table kern_table[] = {
 		.proc_handler	= timer_migration_handler,
 	},
 #endif
+#ifdef CONFIG_ARCH_MMAP_RND_BITS
+	{
+		.procname	= "mmap_rnd_bits",
+		.data		= &mmap_rnd_bits,
+		.maxlen		= sizeof(mmap_rnd_bits),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &mmap_rnd_bits_min,
+		.extra2		= &mmap_rnd_bits_max,
+	},
+#endif
 	{ }
 };
 
-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1258441 — [PATCH 2/2] arm: mm: support ARCH_MMAP_RND_BITS.

From: dcashman <dcashman@google.com>

arm: arch_mmap_rnd() uses a hard-code value of 8 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 8 as the minimum acceptable value.

Signed-off-by: Daniel Cashman <dcashman@google.com>
---
 arch/arm/Kconfig   | 24 ++++++++++++++++++++++++
 arch/arm/mm/mmap.c |  7 +++++--
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 639411f..d61e7e2 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -306,6 +306,30 @@ config MMU
 	  Select if you want MMU-based virtualised addressing space
 	  support by paged memory management. If unsure, say 'Y'.
 
+config ARCH_MMAP_RND_BITS_MIN
+	int
+	default 8
+
+config ARCH_MMAP_RND_BITS_MAX
+	int
+	default 14 if MMU && PAGE_OFFSET=0x40000000
+	default 15 if MMU && PAGE_OFFSET=0x80000000
+	default 16 if MMU
+	default 8
+
+config ARCH_MMAP_RND_BITS
+	int "Number of bits to use for ASLR of mmap base address" if EXPERT
+	range ARCH_MMAP_RND_BITS_MIN ARCH_MMAP_RND_BITS_MAX
+	default ARCH_MMAP_RND_BITS_MIN
+	help
+	  This value can be used to select the number of bits to use to
+	  determine the random offset to the base address of vma regions
+	  resulting from mmap allocations. This value will be bounded
+	  by the architecture's minimum and maximum supported values.
+
+	  This value can be changed after boot using the
+	  /proc/sys/kernel/mmap_rnd_bits tunable
+
 #
 # The "ARM system type" choice list is ordered alphabetically by option
 # text.  Please add new entries in the option alphabetic order.
diff --git a/arch/arm/mm/mmap.c b/arch/arm/mm/mmap.c
index 407dc78..73ca3a7 100644
--- a/arch/arm/mm/mmap.c
+++ b/arch/arm/mm/mmap.c
@@ -11,6 +11,10 @@
 #include <linux/random.h>
 #include <asm/cachetype.h>
 
+int mmap_rnd_bits_min = CONFIG_ARCH_MMAP_RND_BITS_MIN;
+int mmap_rnd_bits_max = CONFIG_ARCH_MMAP_RND_BITS_MAX;
+int mmap_rnd_bits = CONFIG_ARCH_MMAP_RND_BITS;
+
 #define COLOUR_ALIGN(addr,pgoff)		\
 	((((addr)+SHMLBA-1)&~(SHMLBA-1)) +	\
 	 (((pgoff)<<PAGE_SHIFT) & (SHMLBA-1)))
@@ -173,8 +177,7 @@ unsigned long arch_mmap_rnd(void)
 {
 	unsigned long rnd;
 
-	/* 8 bits of randomness in 20 address space bits */
-	rnd = (unsigned long)get_random_int() % (1 << 8);
+	rnd = (unsigned long)get_random_int() % (1 << mmap_rnd_bits);
 
 	return rnd << PAGE_SHIFT;
 }
-- 
2.6.0.rc2.230.g3dd15c0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1258484

Daniel Cashman <dcashman@android.com> writes:

> From: dcashman <dcashman@google.com>
>
> ASLR currently only uses 8 bits to generate the random offset for the
> mmap base address on 32 bit architectures. This value was chosen to
> prevent a poorly chosen value from dividing the address space in such
> a way as to prevent large allocations. This may not be an issue on all
> platforms. Allow the specification of a minimum number of bits so that
> platforms desiring greater ASLR protection may determine where to place
> the trade-off.

This all would be much cleaner if the arm architecture code were just to
register the sysctl itself.

As it sits this looks like a patchset that does not meaninfully bisect,
and would result in code that is hard to trace and understand.

Eric

> Signed-off-by: Daniel Cashman <dcashman@google.com>
> ---
>  Documentation/sysctl/kernel.txt | 14 ++++++++++++++
>  include/linux/mm.h              |  6 ++++++
>  kernel/sysctl.c                 | 11 +++++++++++
>  3 files changed, 31 insertions(+)
>
> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
> index 6fccb69..0d4ca53 100644
> --- a/Documentation/sysctl/kernel.txt
> +++ b/Documentation/sysctl/kernel.txt
> @@ -41,6 +41,7 @@ show up in /proc/sys/kernel:
>  - kptr_restrict
>  - kstack_depth_to_print       [ X86 only ]
>  - l2cr                        [ PPC only ]
> +- mmap_rnd_bits
>  - modprobe                    ==> Documentation/debugging-modules.txt
>  - modules_disabled
>  - msg_next_id		      [ sysv ipc ]
> @@ -391,6 +392,19 @@ This flag controls the L2 cache of G3 processor boards. If
>  
>  ==============================================================
>  
> +mmap_rnd_bits:
> +
> +This value can be used to select the number of bits to use to
> +determine the random offset to the base address of vma regions
> +resulting from mmap allocations on architectures which support
> +tuning address space randomization.  This value will be bounded
> +by the architecture's minimum and maximum supported values.
> +
> +This value can be changed after boot using the
> +/proc/sys/kernel/mmap_rnd_bits tunable
> +
> +==============================================================
> +
>  modules_disabled:
>  
>  A toggle value indicating if modules are allowed to be loaded
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 80001de..15b083a 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -51,6 +51,12 @@ extern int sysctl_legacy_va_layout;
>  #define sysctl_legacy_va_layout 0
>  #endif
>  
> +#ifdef CONFIG_ARCH_MMAP_RND_BITS
> +extern int mmap_rnd_bits_min;
> +extern int mmap_rnd_bits_max;
> +extern int mmap_rnd_bits;
> +#endif
> +
>  #include <asm/page.h>
>  #include <asm/pgtable.h>
>  #include <asm/processor.h>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index e69201d..37e657a 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1139,6 +1139,17 @@ static struct ctl_table kern_table[] = {
>  		.proc_handler	= timer_migration_handler,
>  	},
>  #endif
> +#ifdef CONFIG_ARCH_MMAP_RND_BITS
> +	{
> +		.procname	= "mmap_rnd_bits",
> +		.data		= &mmap_rnd_bits,
> +		.maxlen		= sizeof(mmap_rnd_bits),
> +		.mode		= 0644,
> +		.proc_handler	= proc_dointvec_minmax,
> +		.extra1		= &mmap_rnd_bits_min,
> +		.extra2		= &mmap_rnd_bits_max,
> +	},
> +#endif
>  	{ }
>  };
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1258494

plain text this time...

> This all would be much cleaner if the arm architecture code were just to
> register the sysctl itself.
>
> As it sits this looks like a patchset that does not meaninfully bisect,
> and would result in code that is hard to trace and understand.

I believe the intent is to follow up with more architecture specific
patches to allow each architecture to define the number of bits to use
(min, max, and default) since these values are architecture dependent.
Arm64 patch should be forthcoming, and others after that. With that in
mind, would you still prefer to have the sysctl code in the
arm-specific patch?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1258504

> > This all would be much cleaner if the arm architecture code were just to
> > register the sysctl itself.
> >
> > As it sits this looks like a patchset that does not meaninfully bisect,
> > and would result in code that is hard to trace and understand.
>
> I believe the intent is to follow up with more architecture specific
> patches to allow each architecture to define the number of bits to use

Yes.  I included these patches together because they provide mutual
context, but each has a different outcome and they could be taken
separately.  The arm architecture-specific portion allows the changing
of the number of bits used for mmap ASLR, useful even without the
sysctl.  The sysctl patch (patch 1) provides another way of setting
this value, and the hope is that this will be adopted across multiple
architectures, with the arm changes (patch 2) providing an example.  I
hope to follow this with changes to arm64 and x86, for example.

On Wed, Oct 28, 2015 at 5:01 PM, Jeffrey Vander Stoep <jeffv@google.com> wrote:
> plain text this time...
>
>> This all would be much cleaner if the arm architecture code were just to
>> register the sysctl itself.
>>
>> As it sits this looks like a patchset that does not meaninfully bisect,
>> and would result in code that is hard to trace and understand.
>
> I believe the intent is to follow up with more architecture specific
> patches to allow each architecture to define the number of bits to use
> (min, max, and default) since these values are architecture dependent.
> Arm64 patch should be forthcoming, and others after that. With that in
> mind, would you still prefer to have the sysctl code in the
> arm-specific patch?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1258553

Dan Cashman <dcashman@android.com> writes:

>> > This all would be much cleaner if the arm architecture code were just to
>> > register the sysctl itself.
>> >
>> > As it sits this looks like a patchset that does not meaninfully bisect,
>> > and would result in code that is hard to trace and understand.
>>
>> I believe the intent is to follow up with more architecture specific
>> patches to allow each architecture to define the number of bits to use
>
> Yes.  I included these patches together because they provide mutual
> context, but each has a different outcome and they could be taken
> separately.

They can not.  The first patch is incomplete by itself.

> The arm architecture-specific portion allows the changing
> of the number of bits used for mmap ASLR, useful even without the
> sysctl.  The sysctl patch (patch 1) provides another way of setting
> this value, and the hope is that this will be adopted across multiple
> architectures, with the arm changes (patch 2) providing an example.  I
> hope to follow this with changes to arm64 and x86, for example.

If you want to make the code generic.  Please maximize the sharing.
That is please define the variables in a generic location, as well
as the Kconfig variables (if possible).

As it is you have an architecture specific piece of code that can not be
reused without duplicating code, and that is just begging for problems.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259037

On 10/28/2015 08:41 PM, Eric W. Biederman wrote:
> Dan Cashman <dcashman@android.com> writes:
> 
>>>> This all would be much cleaner if the arm architecture code were just to
>>>> register the sysctl itself.
>>>>
>>>> As it sits this looks like a patchset that does not meaninfully bisect,
>>>> and would result in code that is hard to trace and understand.
>>>
>>> I believe the intent is to follow up with more architecture specific
>>> patches to allow each architecture to define the number of bits to use
>>
>> Yes.  I included these patches together because they provide mutual
>> context, but each has a different outcome and they could be taken
>> separately.
> 
> They can not.  The first patch is incomplete by itself.

Could you be more specific in what makes the first patch incomplete?  Is
it because it is essentially a no-op without additional architecture
changes (e.g. the second patch) or is it specifically because it
introduces and uses the three "mmap_rnd_bits*" variables without
defining them?  If the former, I'd like to avoid combining the general
procfs change with any architecture-specific one(s).  If the latter, I
hope the proposal below addresses that.

>> The arm architecture-specific portion allows the changing
>> of the number of bits used for mmap ASLR, useful even without the
>> sysctl.  The sysctl patch (patch 1) provides another way of setting
>> this value, and the hope is that this will be adopted across multiple
>> architectures, with the arm changes (patch 2) providing an example.  I
>> hope to follow this with changes to arm64 and x86, for example.
> 
> If you want to make the code generic.  Please maximize the sharing.
> That is please define the variables in a generic location, as well
> as the Kconfig variables (if possible).
> 
> As it is you have an architecture specific piece of code that can not be
> reused without duplicating code, and that is just begging for problems.

I think it would make sense to move the variable definitions into
mm/mmap.c, included conditionally based on the presence of
CONFIG_ARCH_MMAP_RND_BITS.

As for the Kconfigs, I am open to suggestions.  I considered declaring
and documenting ARCH_MMAP_RND_BITS in arch/Kconfig, but I would like it
to be bounded in range by the _MIN and _MAX values, which necessarily
must be defined in the arch-specific Kconfigs.  Thus, we'd have
ARCH_MMAP_RND_BITS declared in arch/Kconfig as it currently is in
arch/arm/Kconfig defaulting to _MIN, and would declare both the _MIN and
_MAX in arch/Kconfig, while specifying default values in
arch/${ARCH}/Kconfig.

Would these changes be more acceptable?

Thank You,
Dan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web