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

Groups > linux.kernel > #1673589 > unrolled thread

[tip:x86/mm] x86/mmap, ASLR: Do not treat unlimited-stack tasks as legacy mmap

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [tip:x86/mm] x86/mmap, ASLR: Do not treat unlimited-stack tasks as  legacy mmap tip-bot for Michal Hocko <tipbot@zytor.com> - 2017-06-23 16:10 +0200
    Re: [tip:x86/mm] x86/mmap, ASLR: Do not treat unlimited-stack tasks  as legacy mmap Oleg Nesterov <oleg@redhat.com> - 2017-06-23 17:00 +0200
      Re: [tip:x86/mm] x86/mmap, ASLR: Do not treat unlimited-stack tasks  as legacy mmap Jiri Kosina <jikos@kernel.org> - 2017-06-27 10:10 +0200
        Re: [tip:x86/mm] x86/mmap, ASLR: Do not treat unlimited-stack tasks  as legacy mmap Oleg Nesterov <oleg@redhat.com> - 2017-06-27 16:30 +0200
          Re: [tip:x86/mm] x86/mmap, ASLR: Do not treat unlimited-stack tasks  as legacy mmap Jiri Kosina <jikos@kernel.org> - 2017-06-28 11:50 +0200
    Re: [tip:x86/mm] x86/mmap, ASLR: Do not treat unlimited-stack tasks  as legacy mmap Jiri Kosina <jikos@kernel.org> - 2017-06-23 22:40 +0200
    [tip:x86/mm] x86/mmap, ASLR: Do not treat unlimited-stack tasks as  legacy mmap tip-bot for Michal Hocko <tipbot@zytor.com> - 2017-06-24 08:50 +0200

#1673589 — [tip:x86/mm] x86/mmap, ASLR: Do not treat unlimited-stack tasks as legacy mmap

Commit-ID:  86b110d2ae6365ce91cabd37588bc8611770421a
Gitweb:     http://git.kernel.org/tip/86b110d2ae6365ce91cabd37588bc8611770421a
Author:     Michal Hocko <mhocko@suse.com>
AuthorDate: Wed, 14 Jun 2017 10:22:18 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 23 Jun 2017 11:02:01 +0200

x86/mmap, ASLR: Do not treat unlimited-stack tasks as legacy mmap

Since the following commit in 2008:

  cc503c1b43e0 ("x86: PIE executable randomization")

We added a heuristics to treat applications with RLIMIT_STACK configured
to unlimited as legacy. This means:

 a) set the mmap_base to 1/3 of address space + randomization and
 b) mmap from bottom to top.

This makes some sense as it allows the stack to grow really large. On the
other hand it reduces the address space usable for default mmaps
(without address hint) quite a lot.

We have received a bug report that SAP HANA workload has hit into this
limitation.

We could argue that the user just got what he asked for when setting
up the unlimited stack but to be realistic growing stack up to 1/6
TASK_SIZE (allowed by mmap_base) is pretty much unimited in the real
life. This would give mmap 20TB of additional address space which is
quite nice. Especially when it is much more likely to use that address
space than the reserved stack.

Digging into the history the original implementation of the randomization:

  8817210d4d96 ("[PATCH] x86_64: Flexmap for 32bit and randomized mappings for 64bit")

didn't have this restriction.

So let's try and remove this assumption - hopefully nothing breaks.

Signed-off-by: Michal Hocko <mhocko@suse.com>
Cc: Dave Jones <davej@codemonkey.org.uk>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: akpm@linux-foundation.org
Cc: hughd@google.com
Cc: linux-mm@kvack.org
Cc: will.deacon@arm.com
Link: http://lkml.kernel.org/r/20170614082218.12450-1-mhocko@kernel.org
[ So I've applied this to tip:x86/mm with a wider Cc: list - if anyone objects to this change please holler. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/mm/mmap.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index 19ad095..797295e 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -74,9 +74,6 @@ static int mmap_is_legacy(void)
 	if (current->personality & ADDR_COMPAT_LAYOUT)
 		return 1;
 
-	if (rlimit(RLIMIT_STACK) == RLIM_INFINITY)
-		return 1;
-
 	return sysctl_legacy_va_layout;
 }
 

[toc] | [next] | [standalone]

#1673621 — Re: [tip:x86/mm] x86/mmap, ASLR: Do not treat unlimited-stack tasks as legacy mmap

On 06/23, tip-bot for Michal Hocko wrote:
>
> We added a heuristics to treat applications with RLIMIT_STACK configured
> to unlimited as legacy. This means:

To me this also means a minor security problem. The comment above
PER_CLEAR_ON_SETID says "must be cleared upon setuid or setgid exec",
but if you do "ulimit -s unlimited" before suid exec then
ADDR_COMPAT_LAYOUT set by security checks will be ignored.

> So let's try and remove this assumption - hopefully nothing breaks.

Agreed.

Oleg.

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

#1675403 — Re: [tip:x86/mm] x86/mmap, ASLR: Do not treat unlimited-stack tasks as legacy mmap

On Fri, 23 Jun 2017, Oleg Nesterov wrote:

> > We added a heuristics to treat applications with RLIMIT_STACK configured
> > to unlimited as legacy. This means:
> 
> To me this also means a minor security problem. The comment above
> PER_CLEAR_ON_SETID says "must be cleared upon setuid or setgid exec",
> but if you do "ulimit -s unlimited" before suid exec then
> ADDR_COMPAT_LAYOUT set by security checks will be ignored.

Could you please be a bit more specific here?

mmap_is_legacy() *first* checks for the ADDR_COMPAT_LAYOUT in the 
personality flags, and only then, if it's unset, RLIMIT_STACK comes to 
play.

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

#1675795 — Re: [tip:x86/mm] x86/mmap, ASLR: Do not treat unlimited-stack tasks as legacy mmap

On 06/27, Jiri Kosina wrote:
>
> On Fri, 23 Jun 2017, Oleg Nesterov wrote:
>
> > > We added a heuristics to treat applications with RLIMIT_STACK configured
> > > to unlimited as legacy. This means:
> >
> > To me this also means a minor security problem. The comment above
> > PER_CLEAR_ON_SETID says "must be cleared upon setuid or setgid exec",
> > but if you do "ulimit -s unlimited" before suid exec then
> > ADDR_COMPAT_LAYOUT set by security checks will be ignored.
>
> Could you please be a bit more specific here?
>
> mmap_is_legacy() *first* checks for the ADDR_COMPAT_LAYOUT in the
> personality flags, and only then, if it's unset, RLIMIT_STACK comes to
> play.

Yes, and this means that even if ADDR_COMPAT_LAYOUT was cleared by

	current->personality &= ~bprm->per_clear;

in flush_old_exec() mmap_is_legacy() still returns true if
rlimit(STACK) == INFINITY.

IOW. Say, in case of suid exec bprm_fill_uid() sets
bprm->per_clear = PER_CLEAR_ON_SETID which includes ADDR_COMPAT_LAYOUT.
To me, this means that we do not want the legacy layout after suid exec,
but "ulimit -s unlimited" can be used to break the rule.



And let me quote my "rlimits && suid exec" email I sent some time before...

Imo RLIMITs are almost pointless security-wise, but now it seems to me they
can harm.

Say, the comment above PER_CLEAR_ON_SETID says "must be cleared upon setuid or
setgid exec" and this mask includes ADDR_COMPAT_LAYOUT. OK, this makes sense,
but this doesn't really work because you can just do "$ ulimit -s unlimited"
before suid exec and this will make mmap_is_legacy() return true. Of course,
only if rlim_max=RLIM_INFINITY, but afaik usually this is true.

Or you can lower RLIMIT_STACK to make suid app crash inside some "system
critical" section...

And even if we forget about the potential security impact, isn't it strange
that suid exec inherits RLIMITs from non-root process?

Perhaps it makes sense to reset RLIMITs on suid exec (say, if bprm->per_clear
is not zero) ? Yes, it is not clear how should we define SANE_RLIMITS_FOR_SUID,
and this should probably depend on sysctl, etc.


Oleg.

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

#1676496 — Re: [tip:x86/mm] x86/mmap, ASLR: Do not treat unlimited-stack tasks as legacy mmap

On Tue, 27 Jun 2017, Oleg Nesterov wrote:

> Perhaps it makes sense to reset RLIMITs on suid exec (say, if 
> bprm->per_clear is not zero) ? Yes, it is not clear how should we define 
> SANE_RLIMITS_FOR_SUID, and this should probably depend on sysctl, etc.

Hmm, this should be an userspace-defined policy. On a 'standard' 
(PAM-based) system, I think a sane expectation would be to get the same 
limits as the ones enforced by pam_limits configuration, but syncing those 
with kernel feels awkward.

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

#1673856 — Re: [tip:x86/mm] x86/mmap, ASLR: Do not treat unlimited-stack tasks as legacy mmap

On Fri, 23 Jun 2017, tip-bot for Michal Hocko wrote:

> TASK_SIZE (allowed by mmap_base) is pretty much unimited in the real
> life. This would give mmap 20TB of additional address space which is
> quite nice. Especially when it is much more likely to use that address
> space than the reserved stack.
> 
> Digging into the history the original implementation of the randomization:
> 
>   8817210d4d96 ("[PATCH] x86_64: Flexmap for 32bit and randomized mappings for 64bit")
> 
> didn't have this restriction.
> 
> So let's try and remove this assumption - hopefully nothing breaks.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> Cc: Dave Jones <davej@codemonkey.org.uk>
> Cc: Jiri Kosina <jkosina@suse.cz>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: akpm@linux-foundation.org
> Cc: hughd@google.com
> Cc: linux-mm@kvack.org
> Cc: will.deacon@arm.com
> Link: http://lkml.kernel.org/r/20170614082218.12450-1-mhocko@kernel.org
> [ So I've applied this to tip:x86/mm with a wider Cc: list - if anyone objects to this change please holler. ]
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> ---
>  arch/x86/mm/mmap.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
> index 19ad095..797295e 100644
> --- a/arch/x86/mm/mmap.c
> +++ b/arch/x86/mm/mmap.c
> @@ -74,9 +74,6 @@ static int mmap_is_legacy(void)
>  	if (current->personality & ADDR_COMPAT_LAYOUT)
>  		return 1;
>  
> -	if (rlimit(RLIMIT_STACK) == RLIM_INFINITY)
> -		return 1;
> -

From the person who actually added this particular restriction

	Acked-by: Jiri Kosina <jkosina@suse.cz>

I think it was some historical remnant from the times when 32/64 arch code 
split still existed, but can't really recall any details after all those 
years; it doesn't make sense to me any more.

Thanks,

-- 
Jiri Kosina
SUSE Labs

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

#1674035

Commit-ID:  4a06370bcb674af88679a4f2c5c87c3e40688935
Gitweb:     http://git.kernel.org/tip/4a06370bcb674af88679a4f2c5c87c3e40688935
Author:     Michal Hocko <mhocko@suse.com>
AuthorDate: Wed, 14 Jun 2017 10:22:18 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 24 Jun 2017 08:39:16 +0200

x86/mmap, ASLR: Do not treat unlimited-stack tasks as legacy mmap

Since the following commit in 2008:

  cc503c1b43e0 ("x86: PIE executable randomization")

We added a heuristics to treat applications with RLIMIT_STACK configured
to unlimited as legacy. This means:

 a) set the mmap_base to 1/3 of address space + randomization and
 b) mmap from bottom to top.

This makes some sense as it allows the stack to grow really large. On the
other hand it reduces the address space usable for default mmaps
(without address hint) quite a lot.

We have received a bug report that SAP HANA workload has hit into this
limitation.

We could argue that the user just got what he asked for when setting
up the unlimited stack but to be realistic growing stack up to 1/6
TASK_SIZE (allowed by mmap_base) is pretty much unimited in the real
life. This would give mmap 20TB of additional address space which is
quite nice. Especially when it is much more likely to use that address
space than the reserved stack.

Digging into the history the original implementation of the randomization:

  8817210d4d96 ("[PATCH] x86_64: Flexmap for 32bit and randomized mappings for 64bit")

didn't have this restriction.

So let's try and remove this assumption - hopefully nothing breaks.

Signed-off-by: Michal Hocko <mhocko@suse.com>
Acked-by: Jiri Kosina <jkosina@suse.cz>
Acked-by: Oleg Nesterov <oleg@redhat.com>
Cc: Dave Jones <davej@codemonkey.org.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: akpm@linux-foundation.org
Cc: hughd@google.com
Cc: linux-mm@kvack.org
Cc: will.deacon@arm.com
Link: http://lkml.kernel.org/r/tip-86b110d2ae6365ce91cabd37588bc8611770421a@git.kernel.org
[ So I've applied this to tip:x86/mm with a wider Cc: list - if anyone objects to this change please holler. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/mm/mmap.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index 19ad095..797295e 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -74,9 +74,6 @@ static int mmap_is_legacy(void)
 	if (current->personality & ADDR_COMPAT_LAYOUT)
 		return 1;
 
-	if (rlimit(RLIMIT_STACK) == RLIM_INFINITY)
-		return 1;
-
 	return sysctl_legacy_va_layout;
 }
 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web