New issue
Advanced search Search tips

Issue 737932 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Security



Sign in to add a comment

CrOS: CVE-2017-1000364: Vulnerability reported in Linux kernel

Project Member Reported by vomit.go...@appspot.gserviceaccount.com, Jun 29 2017

Issue description

VOMIT (go/vomit) has received an external vulnerability report for the Linux kernel. 

Advisory: CVE-2017-1000364
  Details: http://vomit.googleplex.com/advisory?id=CVE/CVE-2017-1000364
  CVSS severity score: 10/10.0
  Description:

An issue was discovered in the size of the stack guard page on Linux, specifically a 4k stack guard page is not sufficiently large and can be "jumped" over (the stack guard page is bypassed), this affects Linux Kernel versions 4.11.5 and earlier (the stackguard page was introduced in 2010).



This bug was filed by http://go/vomit
Please contact us at vomit-team@google.com if you need any assistance.

 

Comment 1 by vakh@chromium.org, Jun 30 2017

Owner: groeck@chromium.org
Status: Assigned (was: Untriaged)

Comment 2 by vakh@chromium.org, Jun 30 2017

Components: OS>Kernel

Comment 3 by groeck@chromium.org, Jun 30 2017

Summary: CrOS: CVE-2017-1000364: Vulnerability reported in Linux kernel (was: CrOS: Vulnerability reported in Linux kernel)
Also see 724093.

Comment 4 by groeck@chromium.org, Jun 30 2017

Status: Started (was: Assigned)
groeck@, could you add labels to reflect Security_Impact and Security_Severity?  Thanks!
Labels: Security_Severity-Critical Security_Impact-Stable
Labels: -Security_Severity-Critical Security_Severity-High
matching severity with 724093
Project Member

Comment 8 by sheriffbot@chromium.org, Jul 5 2017

Labels: M-59
Project Member

Comment 9 by sheriffbot@chromium.org, Jul 5 2017

Labels: Pri-1
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 6 2017

Labels: merge-merged-chromeos-3.10
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4

commit 8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4
Author: Hugh Dickins <hughd@google.com>
Date: Thu Jul 06 01:50:22 2017

BACKPORT: mm: larger stack guard gap, between vmas

commit 1be7107fbe18eed3e319a6c3e83c78254b693acb upstream.

Stack guard page is a useful feature to reduce a risk of stack smashing
into a different mapping. We have been using a single page gap which
is sufficient to prevent having stack adjacent to a different mapping.
But this seems to be insufficient in the light of the stack usage in
userspace. E.g. glibc uses as large as 64kB alloca() in many commonly
used functions. Others use constructs liks gid_t buffer[NGROUPS_MAX]
which is 256kB or stack strings with MAX_ARG_STRLEN.

This will become especially dangerous for suid binaries and the default
no limit for the stack size limit because those applications can be
tricked to consume a large portion of the stack and a single glibc call
could jump over the guard page. These attacks are not theoretical,
unfortunatelly.

Make those attacks less probable by increasing the stack guard gap
to 1MB (on systems with 4k pages; but make it depend on the page size
because systems with larger base pages might cap stack allocations in
the PAGE_SIZE units) which should cover larger alloca() and VLA stack
allocations. It is obviously not a full fix because the problem is
somehow inherent, but it should reduce attack space a lot.

One could argue that the gap size should be configurable from userspace,
but that can be done later when somebody finds that the new 1MB is wrong
for some special case applications.  For now, add a kernel command line
option (stack_guard_gap) to specify the stack gap size (in page units).

Implementation wise, first delete all the old code for stack guard page:
because although we could get away with accounting one extra page in a
stack vma, accounting a larger gap can break userspace - case in point,
a program run with "ulimit -S -v 20000" failed when the 1MB gap was
counted for RLIMIT_AS; similar problems could come with RLIMIT_MLOCK
and strict non-overcommit mode.

Instead of keeping gap inside the stack vma, maintain the stack guard
gap as a gap between vmas: using vm_start_gap() in place of vm_start
(or vm_end_gap() in place of vm_end if VM_GROWSUP) in just those few
places which need to respect the gap - mainly arch_get_unmapped_area(),
and and the vma tree's subtree_gap support for that.

BUG= chromium:726072 ,  chromium:724093 ,  chromium:737932 
TEST=Build and run

CQ-DEPEND=CL:559932
Change-Id: Icb104d67a71d5bcfc0918d15b89fad13d286c147
Original-patch-by: Oleg Nesterov <oleg@redhat.com>
Original-patch-by: Michal Hocko <mhocko@suse.com>
Signed-off-by: Hugh Dickins <hughd@google.com>
[wt: backport to 4.11: adjust context]
[wt: backport to 4.9: adjust context ; kernel doc was not in admin-guide]
[wt: backport to 4.4: adjust context ; drop ppc hugetlb_radix changes]
[wt: backport to 3.18: adjust context ; no FOLL_POPULATE ;
     s390 uses generic arch_get_unmapped_area()]
[wt: backport to 3.16: adjust context]
[wt: backport to 3.10: adjust context ; code logic in PARISC's
     arch_get_unmapped_area() wasn't found ; code inserted into
     expand_upwards() and expand_downwards() runs under anon_vma lock;
     changes for gup.c:faultin_page go to memory.c:__get_user_pages();
     included Hugh Dickins' fixes]
Signed-off-by: Willy Tarreau <w@1wt.eu>
[groeck: Backport; minor context changes]
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 1ad9a25dd06f,
 git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git)
Reviewed-on: https://chromium-review.googlesource.com/559930
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/include/linux/mm.h
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/arch/mips/mm/mmap.c
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/arch/sparc/kernel/sys_sparc_64.c
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/mm/mmap.c
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/arch/tile/mm/hugetlbpage.c
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/fs/hugetlbfs/inode.c
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/arch/xtensa/kernel/syscall.c
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/arch/x86/mm/hugetlbpage.c
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/arch/powerpc/mm/slice.c
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/arch/sh/mm/mmap.c
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/arch/x86/kernel/sys_x86_64.c
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/arch/arm/mm/mmap.c
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/arch/sparc/mm/hugetlbpage.c
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/fs/proc/task_mmu.c
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/Documentation/kernel-parameters.txt
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/arch/arc/mm/mmap.c
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/mm/memory.c
[modify] https://crrev.com/8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4/arch/frv/mm/elf-fdpic.c

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/71c7596b284997d55e88cf18da07b5a9cfb58f35

commit 71c7596b284997d55e88cf18da07b5a9cfb58f35
Author: Hugh Dickins <hughd@google.com>
Date: Thu Jul 06 01:50:23 2017

UPSTREAM: mm: fix new crash in unmapped_area_topdown()

commit f4cb767d76cf7ee72f97dd76f6cfa6c76a5edc89 upstream.

Trinity gets kernel BUG at mm/mmap.c:1963! in about 3 minutes of
mmap testing.  That's the VM_BUG_ON(gap_end < gap_start) at the
end of unmapped_area_topdown().  Linus points out how MAP_FIXED
(which does not have to respect our stack guard gap intentions)
could result in gap_end below gap_start there.  Fix that, and
the similar case in its alternative, unmapped_area().

BUG= chromium:726072 ,  chromium:724093 ,  chromium:737932 
TEST=Build and run

Change-Id: If5c4687962ef5f626d86c603144931a7403e115d
Cc: stable@vger.kernel.org
Fixes: 1be7107fbe18 ("mm: larger stack guard gap, between vmas")
Reported-by: Dave Jones <davej@codemonkey.org.uk>
Debugged-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Hugh Dickins <hughd@google.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Willy Tarreau <w@1wt.eu>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 28ebf89579a0,
 git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git)
Reviewed-on: https://chromium-review.googlesource.com/559931
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/71c7596b284997d55e88cf18da07b5a9cfb58f35/mm/mmap.c

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/3c2f4706a9304f9723a85c935c540eaa088f5a43

commit 3c2f4706a9304f9723a85c935c540eaa088f5a43
Author: Greg Hackmann <ghackmann@google.com>
Date: Thu Jul 06 01:50:24 2017

HACK: CHROMIUM: x86/mm: don't check %sp when faulting page immediately before stack

Prior to 1be7107fbe18 ("mm: larger stack guard gap, between vmas"), the
stack guard page was included in the stack's VMA.  This implementation
detail created a subtle corner case in __do_page_fault(), where the
branch

	if (likely(vma->vm_start <= address))
		goto good_area;

would skip over the %sp check when `address' was inside the stack guard
page.  This allowed userspace to expand the stack without altering %sp
by repeatedly touching the page just below the "advertised" start of the
stack VMA.

Although this corner case probably isn't intended, it appears to have
been in the kernel for years, and is used by real software in the wild.
For example the Android Runtime's Thread::InstallImplicitProtection()
routine has relied on this behavior to expand the main thread's stack,
and crashes on x86 kernels with 1be7107fbe18 applied.

NOTE: Marked as HACK since it effectively re-introduces a bug into the
kernel which is exploited by zygote in Android. Fixing that bug results
in a zygote crash in art::Thread::InstallImplicitProtection(). If and when
this problem will be fixed is currently unknown. If possible, this patch
should be dropped at a later time or, if applicable, be replaced with an
upstream patch.

BUG= chromium:726072 ,  chromium:724093 , b:62952017,  chromium:737932 
TEST=Run ARC++

Change-Id: I8d2f6d3e598e99b2605a9caf8e5c6c166de88e0f
Fixes: 1be7107fbe18 ("mm: larger stack guard gap, between vmas")
Signed-off-by: Greg Hackmann <ghackmann@google.com>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/559932
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/3c2f4706a9304f9723a85c935c540eaa088f5a43/arch/x86/mm/fault.c

Project Member

Comment 13 by bugdroid1@chromium.org, Jul 6 2017

Labels: merge-merged-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/a936c044f9084f6a5dc0642076b325c396412a6b

commit a936c044f9084f6a5dc0642076b325c396412a6b
Author: Greg Hackmann <ghackmann@google.com>
Date: Thu Jul 06 01:50:21 2017

HACK: CHROMIUM: x86/mm: don't check %sp when faulting page immediately before stack

Prior to 1be7107fbe18 ("mm: larger stack guard gap, between vmas"), the
stack guard page was included in the stack's VMA.  This implementation
detail created a subtle corner case in __do_page_fault(), where the
branch

	if (likely(vma->vm_start <= address))
		goto good_area;

would skip over the %sp check when `address' was inside the stack guard
page.  This allowed userspace to expand the stack without altering %sp
by repeatedly touching the page just below the "advertised" start of the
stack VMA.

Although this corner case probably isn't intended, it appears to have
been in the kernel for years, and is used by real software in the wild.
For example the Android Runtime's Thread::InstallImplicitProtection()
routine has relied on this behavior to expand the main thread's stack,
and crashes on x86 kernels with 1be7107fbe18 applied.

NOTE: Marked as HACK since it effectively re-introduces a bug into the
kernel which is exploited by zygote in Android. Fixing that bug results
in a zygote crash in art::Thread::InstallImplicitProtection(). If and when
this problem will be fixed is currently unknown. If possible, this patch
should be dropped at a later time or, if applicable, be replaced with an
upstream patch.

BUG= chromium:726072 ,  chromium:724093 , b:62952017,  chromium:737932 
TEST=Run ARC++

Change-Id: I8d2f6d3e598e99b2605a9caf8e5c6c166de88e0f
Fixes: 1be7107fbe18 ("mm: larger stack guard gap, between vmas")
Signed-off-by: Greg Hackmann <ghackmann@google.com>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/549182
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/a936c044f9084f6a5dc0642076b325c396412a6b/arch/x86/mm/fault.c

Project Member

Comment 14 by bugdroid1@chromium.org, Jul 6 2017

Labels: merge-merged-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/e16e994e6b0e96187530228ad22a87758dd628eb

commit e16e994e6b0e96187530228ad22a87758dd628eb
Author: Hugh Dickins <hughd@google.com>
Date: Thu Jul 06 04:54:44 2017

UPSTREAM: mm: larger stack guard gap, between vmas

commit 1be7107fbe18eed3e319a6c3e83c78254b693acb upstream.

Stack guard page is a useful feature to reduce a risk of stack smashing
into a different mapping. We have been using a single page gap which
is sufficient to prevent having stack adjacent to a different mapping.
But this seems to be insufficient in the light of the stack usage in
userspace. E.g. glibc uses as large as 64kB alloca() in many commonly
used functions. Others use constructs liks gid_t buffer[NGROUPS_MAX]
which is 256kB or stack strings with MAX_ARG_STRLEN.

This will become especially dangerous for suid binaries and the default
no limit for the stack size limit because those applications can be
tricked to consume a large portion of the stack and a single glibc call
could jump over the guard page. These attacks are not theoretical,
unfortunatelly.

Make those attacks less probable by increasing the stack guard gap
to 1MB (on systems with 4k pages; but make it depend on the page size
because systems with larger base pages might cap stack allocations in
the PAGE_SIZE units) which should cover larger alloca() and VLA stack
allocations. It is obviously not a full fix because the problem is
somehow inherent, but it should reduce attack space a lot.

One could argue that the gap size should be configurable from userspace,
but that can be done later when somebody finds that the new 1MB is wrong
for some special case applications.  For now, add a kernel command line
option (stack_guard_gap) to specify the stack gap size (in page units).

Implementation wise, first delete all the old code for stack guard page:
because although we could get away with accounting one extra page in a
stack vma, accounting a larger gap can break userspace - case in point,
a program run with "ulimit -S -v 20000" failed when the 1MB gap was
counted for RLIMIT_AS; similar problems could come with RLIMIT_MLOCK
and strict non-overcommit mode.

Instead of keeping gap inside the stack vma, maintain the stack guard
gap as a gap between vmas: using vm_start_gap() in place of vm_start
(or vm_end_gap() in place of vm_end if VM_GROWSUP) in just those few
places which need to respect the gap - mainly arch_get_unmapped_area(),
and and the vma tree's subtree_gap support for that.

BUG= chromium:726072 ,  chromium:724093 ,  chromium:737932 
TEST=Build and run

CQ-DEPEND=CL:557932
Change-Id: I09201def44e81686facd7d8333146336b49f84e1
Original-patch-by: Oleg Nesterov <oleg@redhat.com>
Original-patch-by: Michal Hocko <mhocko@suse.com>
Signed-off-by: Hugh Dickins <hughd@google.com>
[wt: backport to 4.11: adjust context]
[wt: backport to 4.9: adjust context ; kernel doc was not in admin-guide]
[wt: backport to 4.4: adjust context ; drop ppc hugetlb_radix changes]
[wt: backport to 3.18: adjust context ; no FOLL_POPULATE ;
     s390 uses generic arch_get_unmapped_area()]
[wt: backport to 3.16: adjust context]
[wt: backport to 3.10: adjust context ; code logic in PARISC's
     arch_get_unmapped_area() wasn't found ; code inserted into
     expand_upwards() and expand_downwards() runs under anon_vma lock;
     changes for gup.c:faultin_page go to memory.c:__get_user_pages();
     included Hugh Dickins' fixes]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 1ad9a25dd06f,
 git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git)
Reviewed-on: https://chromium-review.googlesource.com/557930
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/include/linux/mm.h
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/arch/mips/mm/mmap.c
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/arch/sparc/kernel/sys_sparc_64.c
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/mm/mmap.c
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/arch/tile/mm/hugetlbpage.c
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/fs/hugetlbfs/inode.c
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/arch/xtensa/kernel/syscall.c
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/arch/x86/mm/hugetlbpage.c
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/arch/powerpc/mm/slice.c
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/arch/sh/mm/mmap.c
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/arch/x86/kernel/sys_x86_64.c
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/arch/arm/mm/mmap.c
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/arch/sparc/mm/hugetlbpage.c
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/fs/proc/task_mmu.c
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/Documentation/kernel-parameters.txt
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/arch/arc/mm/mmap.c
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/mm/memory.c
[modify] https://crrev.com/e16e994e6b0e96187530228ad22a87758dd628eb/arch/frv/mm/elf-fdpic.c

Project Member

Comment 15 by bugdroid1@chromium.org, Jul 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/e0aa322a8fac4856cb22083c550ccbafc8b2bb56

commit e0aa322a8fac4856cb22083c550ccbafc8b2bb56
Author: Hugh Dickins <hughd@google.com>
Date: Thu Jul 06 04:54:45 2017

UPSTREAM: mm: fix new crash in unmapped_area_topdown()

commit f4cb767d76cf7ee72f97dd76f6cfa6c76a5edc89 upstream.

Trinity gets kernel BUG at mm/mmap.c:1963! in about 3 minutes of
mmap testing.  That's the VM_BUG_ON(gap_end < gap_start) at the
end of unmapped_area_topdown().  Linus points out how MAP_FIXED
(which does not have to respect our stack guard gap intentions)
could result in gap_end below gap_start there.  Fix that, and
the similar case in its alternative, unmapped_area().

BUG= chromium:726072 ,  chromium:724093 ,  chromium:737932 
TEST=Build and run

Change-Id: I573cce18919ce07a68cd1390d2159fb5447cfcb9
Cc: stable@vger.kernel.org
Fixes: 1be7107fbe18 ("mm: larger stack guard gap, between vmas")
Reported-by: Dave Jones <davej@codemonkey.org.uk>
Debugged-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Hugh Dickins <hughd@google.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Willy Tarreau <w@1wt.eu>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 28ebf89579a0,
 git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git)
Reviewed-on: https://chromium-review.googlesource.com/557931
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/e0aa322a8fac4856cb22083c550ccbafc8b2bb56/mm/mmap.c

Project Member

Comment 16 by bugdroid1@chromium.org, Jul 6 2017

Labels: merge-merged-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/11f0ee346e03a269cb386cbfb3ebc4df658a3446

commit 11f0ee346e03a269cb386cbfb3ebc4df658a3446
Author: Greg Hackmann <ghackmann@google.com>
Date: Thu Jul 06 04:54:46 2017

HACK: CHROMIUM: x86/mm: don't check %sp when faulting page immediately before stack

Prior to 1be7107fbe18 ("mm: larger stack guard gap, between vmas"), the
stack guard page was included in the stack's VMA.  This implementation
detail created a subtle corner case in __do_page_fault(), where the
branch

	if (likely(vma->vm_start <= address))
		goto good_area;

would skip over the %sp check when `address' was inside the stack guard
page.  This allowed userspace to expand the stack without altering %sp
by repeatedly touching the page just below the "advertised" start of the
stack VMA.

Although this corner case probably isn't intended, it appears to have
been in the kernel for years, and is used by real software in the wild.
For example the Android Runtime's Thread::InstallImplicitProtection()
routine has relied on this behavior to expand the main thread's stack,
and crashes on x86 kernels with 1be7107fbe18 applied.

NOTE: Marked as HACK since it effectively re-introduces a bug into the
kernel which is exploited by zygote in Android. Fixing that bug results
in a zygote crash in art::Thread::InstallImplicitProtection(). If and when
this problem will be fixed is currently unknown. If possible, this patch
should be dropped at a later time or, if applicable, be replaced with an
upstream patch.

BUG= chromium:726072 ,  chromium:724093 , b:62952017,  chromium:737932 
TEST=Run ARC++

Change-Id: I8d2f6d3e598e99b2605a9caf8e5c6c166de88e0f
Fixes: 1be7107fbe18 ("mm: larger stack guard gap, between vmas")
Signed-off-by: Greg Hackmann <ghackmann@google.com>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/557932
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>

[modify] https://crrev.com/11f0ee346e03a269cb386cbfb3ebc4df658a3446/arch/x86/mm/fault.c

Project Member

Comment 17 by sheriffbot@chromium.org, Jul 6 2017

Status: Fixed (was: Started)
Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 18 by sheriffbot@chromium.org, Jul 7 2017

Labels: Restrict-View-SecurityNotify
Project Member

Comment 19 by bugdroid1@chromium.org, Jul 7 2017

Labels: merge-merged-release-R60-9592.B-chromeos-3.10
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/a7c97aa9c8df609bc90ee588df5654e2e9570c0d

commit a7c97aa9c8df609bc90ee588df5654e2e9570c0d
Author: Greg Hackmann <ghackmann@google.com>
Date: Fri Jul 07 14:39:32 2017

HACK: CHROMIUM: x86/mm: don't check %sp when faulting page immediately before stack

Prior to 1be7107fbe18 ("mm: larger stack guard gap, between vmas"), the
stack guard page was included in the stack's VMA.  This implementation
detail created a subtle corner case in __do_page_fault(), where the
branch

	if (likely(vma->vm_start <= address))
		goto good_area;

would skip over the %sp check when `address' was inside the stack guard
page.  This allowed userspace to expand the stack without altering %sp
by repeatedly touching the page just below the "advertised" start of the
stack VMA.

Although this corner case probably isn't intended, it appears to have
been in the kernel for years, and is used by real software in the wild.
For example the Android Runtime's Thread::InstallImplicitProtection()
routine has relied on this behavior to expand the main thread's stack,
and crashes on x86 kernels with 1be7107fbe18 applied.

NOTE: Marked as HACK since it effectively re-introduces a bug into the
kernel which is exploited by zygote in Android. Fixing that bug results
in a zygote crash in art::Thread::InstallImplicitProtection(). If and when
this problem will be fixed is currently unknown. If possible, this patch
should be dropped at a later time or, if applicable, be replaced with an
upstream patch.

BUG= chromium:726072 ,  chromium:724093 , b:62952017,  chromium:737932 
TEST=Run ARC++

Change-Id: I8d2f6d3e598e99b2605a9caf8e5c6c166de88e0f
Fixes: 1be7107fbe18 ("mm: larger stack guard gap, between vmas")
Signed-off-by: Greg Hackmann <ghackmann@google.com>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/559932
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 3c2f4706a9304f9723a85c935c540eaa088f5a43)
Reviewed-on: https://chromium-review.googlesource.com/562439

[modify] https://crrev.com/a7c97aa9c8df609bc90ee588df5654e2e9570c0d/arch/x86/mm/fault.c

Project Member

Comment 20 by bugdroid1@chromium.org, Jul 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/aff149fbcca7fb750e4302b2e34f9d8a8eea1173

commit aff149fbcca7fb750e4302b2e34f9d8a8eea1173
Author: Hugh Dickins <hughd@google.com>
Date: Fri Jul 07 14:39:44 2017

UPSTREAM: mm: fix new crash in unmapped_area_topdown()

commit f4cb767d76cf7ee72f97dd76f6cfa6c76a5edc89 upstream.

Trinity gets kernel BUG at mm/mmap.c:1963! in about 3 minutes of
mmap testing.  That's the VM_BUG_ON(gap_end < gap_start) at the
end of unmapped_area_topdown().  Linus points out how MAP_FIXED
(which does not have to respect our stack guard gap intentions)
could result in gap_end below gap_start there.  Fix that, and
the similar case in its alternative, unmapped_area().

BUG= chromium:726072 ,  chromium:724093 ,  chromium:737932 
TEST=Build and run

Change-Id: If5c4687962ef5f626d86c603144931a7403e115d
Cc: stable@vger.kernel.org
Fixes: 1be7107fbe18 ("mm: larger stack guard gap, between vmas")
Reported-by: Dave Jones <davej@codemonkey.org.uk>
Debugged-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Hugh Dickins <hughd@google.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Willy Tarreau <w@1wt.eu>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 28ebf89579a0,
 git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git)
Reviewed-on: https://chromium-review.googlesource.com/559931
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 71c7596b284997d55e88cf18da07b5a9cfb58f35)
Reviewed-on: https://chromium-review.googlesource.com/562438

[modify] https://crrev.com/aff149fbcca7fb750e4302b2e34f9d8a8eea1173/mm/mmap.c

Project Member

Comment 21 by bugdroid1@chromium.org, Jul 7 2017

Labels: merge-merged-release-R60-9592.B-chromeos-3.14
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/e373bfc921965214c410d4f74791b3a3729b0593

commit e373bfc921965214c410d4f74791b3a3729b0593
Author: Hugh Dickins <hughd@google.com>
Date: Fri Jul 07 14:42:09 2017

UPSTREAM: mm: larger stack guard gap, between vmas

commit 1be7107fbe18eed3e319a6c3e83c78254b693acb upstream.

Stack guard page is a useful feature to reduce a risk of stack smashing
into a different mapping. We have been using a single page gap which
is sufficient to prevent having stack adjacent to a different mapping.
But this seems to be insufficient in the light of the stack usage in
userspace. E.g. glibc uses as large as 64kB alloca() in many commonly
used functions. Others use constructs liks gid_t buffer[NGROUPS_MAX]
which is 256kB or stack strings with MAX_ARG_STRLEN.

This will become especially dangerous for suid binaries and the default
no limit for the stack size limit because those applications can be
tricked to consume a large portion of the stack and a single glibc call
could jump over the guard page. These attacks are not theoretical,
unfortunatelly.

Make those attacks less probable by increasing the stack guard gap
to 1MB (on systems with 4k pages; but make it depend on the page size
because systems with larger base pages might cap stack allocations in
the PAGE_SIZE units) which should cover larger alloca() and VLA stack
allocations. It is obviously not a full fix because the problem is
somehow inherent, but it should reduce attack space a lot.

One could argue that the gap size should be configurable from userspace,
but that can be done later when somebody finds that the new 1MB is wrong
for some special case applications.  For now, add a kernel command line
option (stack_guard_gap) to specify the stack gap size (in page units).

Implementation wise, first delete all the old code for stack guard page:
because although we could get away with accounting one extra page in a
stack vma, accounting a larger gap can break userspace - case in point,
a program run with "ulimit -S -v 20000" failed when the 1MB gap was
counted for RLIMIT_AS; similar problems could come with RLIMIT_MLOCK
and strict non-overcommit mode.

Instead of keeping gap inside the stack vma, maintain the stack guard
gap as a gap between vmas: using vm_start_gap() in place of vm_start
(or vm_end_gap() in place of vm_end if VM_GROWSUP) in just those few
places which need to respect the gap - mainly arch_get_unmapped_area(),
and and the vma tree's subtree_gap support for that.

BUG= chromium:726072 ,  chromium:724093 ,  chromium:737932 
TEST=Build and run

CQ-DEPEND=CL:557932
Change-Id: I09201def44e81686facd7d8333146336b49f84e1
Original-patch-by: Oleg Nesterov <oleg@redhat.com>
Original-patch-by: Michal Hocko <mhocko@suse.com>
Signed-off-by: Hugh Dickins <hughd@google.com>
[wt: backport to 4.11: adjust context]
[wt: backport to 4.9: adjust context ; kernel doc was not in admin-guide]
[wt: backport to 4.4: adjust context ; drop ppc hugetlb_radix changes]
[wt: backport to 3.18: adjust context ; no FOLL_POPULATE ;
     s390 uses generic arch_get_unmapped_area()]
[wt: backport to 3.16: adjust context]
[wt: backport to 3.10: adjust context ; code logic in PARISC's
     arch_get_unmapped_area() wasn't found ; code inserted into
     expand_upwards() and expand_downwards() runs under anon_vma lock;
     changes for gup.c:faultin_page go to memory.c:__get_user_pages();
     included Hugh Dickins' fixes]
Signed-off-by: Willy Tarreau <w@1wt.eu>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 1ad9a25dd06f,
 git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git)
Reviewed-on: https://chromium-review.googlesource.com/557930
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit e16e994e6b0e96187530228ad22a87758dd628eb)
Reviewed-on: https://chromium-review.googlesource.com/561962

[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/include/linux/mm.h
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/arch/mips/mm/mmap.c
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/arch/sparc/kernel/sys_sparc_64.c
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/mm/mmap.c
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/arch/tile/mm/hugetlbpage.c
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/fs/hugetlbfs/inode.c
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/arch/xtensa/kernel/syscall.c
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/arch/x86/mm/hugetlbpage.c
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/arch/powerpc/mm/slice.c
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/arch/sh/mm/mmap.c
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/arch/x86/kernel/sys_x86_64.c
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/arch/arm/mm/mmap.c
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/arch/sparc/mm/hugetlbpage.c
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/fs/proc/task_mmu.c
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/Documentation/kernel-parameters.txt
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/arch/arc/mm/mmap.c
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/mm/memory.c
[modify] https://crrev.com/e373bfc921965214c410d4f74791b3a3729b0593/arch/frv/mm/elf-fdpic.c

Project Member

Comment 22 by bugdroid1@chromium.org, Jul 7 2017

Labels: merge-merged-release-R60-9592.B-chromeos-3.10
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80

commit d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80
Author: Hugh Dickins <hughd@google.com>
Date: Fri Jul 07 14:42:21 2017

BACKPORT: mm: larger stack guard gap, between vmas

commit 1be7107fbe18eed3e319a6c3e83c78254b693acb upstream.

Stack guard page is a useful feature to reduce a risk of stack smashing
into a different mapping. We have been using a single page gap which
is sufficient to prevent having stack adjacent to a different mapping.
But this seems to be insufficient in the light of the stack usage in
userspace. E.g. glibc uses as large as 64kB alloca() in many commonly
used functions. Others use constructs liks gid_t buffer[NGROUPS_MAX]
which is 256kB or stack strings with MAX_ARG_STRLEN.

This will become especially dangerous for suid binaries and the default
no limit for the stack size limit because those applications can be
tricked to consume a large portion of the stack and a single glibc call
could jump over the guard page. These attacks are not theoretical,
unfortunatelly.

Make those attacks less probable by increasing the stack guard gap
to 1MB (on systems with 4k pages; but make it depend on the page size
because systems with larger base pages might cap stack allocations in
the PAGE_SIZE units) which should cover larger alloca() and VLA stack
allocations. It is obviously not a full fix because the problem is
somehow inherent, but it should reduce attack space a lot.

One could argue that the gap size should be configurable from userspace,
but that can be done later when somebody finds that the new 1MB is wrong
for some special case applications.  For now, add a kernel command line
option (stack_guard_gap) to specify the stack gap size (in page units).

Implementation wise, first delete all the old code for stack guard page:
because although we could get away with accounting one extra page in a
stack vma, accounting a larger gap can break userspace - case in point,
a program run with "ulimit -S -v 20000" failed when the 1MB gap was
counted for RLIMIT_AS; similar problems could come with RLIMIT_MLOCK
and strict non-overcommit mode.

Instead of keeping gap inside the stack vma, maintain the stack guard
gap as a gap between vmas: using vm_start_gap() in place of vm_start
(or vm_end_gap() in place of vm_end if VM_GROWSUP) in just those few
places which need to respect the gap - mainly arch_get_unmapped_area(),
and and the vma tree's subtree_gap support for that.

BUG= chromium:726072 ,  chromium:724093 ,  chromium:737932 
TEST=Build and run

CQ-DEPEND=CL:559932
Change-Id: Icb104d67a71d5bcfc0918d15b89fad13d286c147
Original-patch-by: Oleg Nesterov <oleg@redhat.com>
Original-patch-by: Michal Hocko <mhocko@suse.com>
Signed-off-by: Hugh Dickins <hughd@google.com>
[wt: backport to 4.11: adjust context]
[wt: backport to 4.9: adjust context ; kernel doc was not in admin-guide]
[wt: backport to 4.4: adjust context ; drop ppc hugetlb_radix changes]
[wt: backport to 3.18: adjust context ; no FOLL_POPULATE ;
     s390 uses generic arch_get_unmapped_area()]
[wt: backport to 3.16: adjust context]
[wt: backport to 3.10: adjust context ; code logic in PARISC's
     arch_get_unmapped_area() wasn't found ; code inserted into
     expand_upwards() and expand_downwards() runs under anon_vma lock;
     changes for gup.c:faultin_page go to memory.c:__get_user_pages();
     included Hugh Dickins' fixes]
Signed-off-by: Willy Tarreau <w@1wt.eu>
[groeck: Backport; minor context changes]
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 1ad9a25dd06f,
 git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git)
Reviewed-on: https://chromium-review.googlesource.com/559930
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 8fe88d7f70aa4170f3c992ceec7738ab1eb2b3e4)
Reviewed-on: https://chromium-review.googlesource.com/562437

[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/include/linux/mm.h
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/arch/mips/mm/mmap.c
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/arch/sparc/kernel/sys_sparc_64.c
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/mm/mmap.c
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/arch/tile/mm/hugetlbpage.c
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/fs/hugetlbfs/inode.c
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/arch/xtensa/kernel/syscall.c
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/arch/x86/mm/hugetlbpage.c
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/arch/powerpc/mm/slice.c
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/arch/sh/mm/mmap.c
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/arch/x86/kernel/sys_x86_64.c
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/arch/arm/mm/mmap.c
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/arch/sparc/mm/hugetlbpage.c
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/fs/proc/task_mmu.c
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/Documentation/kernel-parameters.txt
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/arch/arc/mm/mmap.c
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/mm/memory.c
[modify] https://crrev.com/d1d27c0b2ebd66fe6a8ec78563ca1d7ff4af5c80/arch/frv/mm/elf-fdpic.c

Project Member

Comment 23 by bugdroid1@chromium.org, Jul 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/6b797367317f026a846f35f6c1cd848dfa26f3e1

commit 6b797367317f026a846f35f6c1cd848dfa26f3e1
Author: Greg Hackmann <ghackmann@google.com>
Date: Fri Jul 07 14:42:26 2017

HACK: CHROMIUM: x86/mm: don't check %sp when faulting page immediately before stack

Prior to 1be7107fbe18 ("mm: larger stack guard gap, between vmas"), the
stack guard page was included in the stack's VMA.  This implementation
detail created a subtle corner case in __do_page_fault(), where the
branch

	if (likely(vma->vm_start <= address))
		goto good_area;

would skip over the %sp check when `address' was inside the stack guard
page.  This allowed userspace to expand the stack without altering %sp
by repeatedly touching the page just below the "advertised" start of the
stack VMA.

Although this corner case probably isn't intended, it appears to have
been in the kernel for years, and is used by real software in the wild.
For example the Android Runtime's Thread::InstallImplicitProtection()
routine has relied on this behavior to expand the main thread's stack,
and crashes on x86 kernels with 1be7107fbe18 applied.

NOTE: Marked as HACK since it effectively re-introduces a bug into the
kernel which is exploited by zygote in Android. Fixing that bug results
in a zygote crash in art::Thread::InstallImplicitProtection(). If and when
this problem will be fixed is currently unknown. If possible, this patch
should be dropped at a later time or, if applicable, be replaced with an
upstream patch.

BUG= chromium:726072 ,  chromium:724093 , b:62952017,  chromium:737932 
TEST=Run ARC++

Change-Id: I8d2f6d3e598e99b2605a9caf8e5c6c166de88e0f
Fixes: 1be7107fbe18 ("mm: larger stack guard gap, between vmas")
Signed-off-by: Greg Hackmann <ghackmann@google.com>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/557932
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit 11f0ee346e03a269cb386cbfb3ebc4df658a3446)
Reviewed-on: https://chromium-review.googlesource.com/561967

[modify] https://crrev.com/6b797367317f026a846f35f6c1cd848dfa26f3e1/arch/x86/mm/fault.c

Project Member

Comment 24 by bugdroid1@chromium.org, Jul 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/89fcabe08f6ae6c8ec98455dff01d04f5c9088f6

commit 89fcabe08f6ae6c8ec98455dff01d04f5c9088f6
Author: Hugh Dickins <hughd@google.com>
Date: Fri Jul 07 14:42:29 2017

UPSTREAM: mm: fix new crash in unmapped_area_topdown()

commit f4cb767d76cf7ee72f97dd76f6cfa6c76a5edc89 upstream.

Trinity gets kernel BUG at mm/mmap.c:1963! in about 3 minutes of
mmap testing.  That's the VM_BUG_ON(gap_end < gap_start) at the
end of unmapped_area_topdown().  Linus points out how MAP_FIXED
(which does not have to respect our stack guard gap intentions)
could result in gap_end below gap_start there.  Fix that, and
the similar case in its alternative, unmapped_area().

BUG= chromium:726072 ,  chromium:724093 ,  chromium:737932 
TEST=Build and run

Change-Id: I573cce18919ce07a68cd1390d2159fb5447cfcb9
Cc: stable@vger.kernel.org
Fixes: 1be7107fbe18 ("mm: larger stack guard gap, between vmas")
Reported-by: Dave Jones <davej@codemonkey.org.uk>
Debugged-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Hugh Dickins <hughd@google.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Willy Tarreau <w@1wt.eu>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
(cherry picked from commit 28ebf89579a0,
 git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git)
Reviewed-on: https://chromium-review.googlesource.com/557931
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit e0aa322a8fac4856cb22083c550ccbafc8b2bb56)
Reviewed-on: https://chromium-review.googlesource.com/561966

[modify] https://crrev.com/89fcabe08f6ae6c8ec98455dff01d04f5c9088f6/mm/mmap.c

Project Member

Comment 25 by bugdroid1@chromium.org, Jul 7 2017

Labels: merge-merged-release-R60-9592.B-chromeos-3.18
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/62dca0a6127026debe502b9fff47ce6e00868899

commit 62dca0a6127026debe502b9fff47ce6e00868899
Author: Greg Hackmann <ghackmann@google.com>
Date: Fri Jul 07 14:45:00 2017

HACK: CHROMIUM: x86/mm: don't check %sp when faulting page immediately before stack

Prior to 1be7107fbe18 ("mm: larger stack guard gap, between vmas"), the
stack guard page was included in the stack's VMA.  This implementation
detail created a subtle corner case in __do_page_fault(), where the
branch

	if (likely(vma->vm_start <= address))
		goto good_area;

would skip over the %sp check when `address' was inside the stack guard
page.  This allowed userspace to expand the stack without altering %sp
by repeatedly touching the page just below the "advertised" start of the
stack VMA.

Although this corner case probably isn't intended, it appears to have
been in the kernel for years, and is used by real software in the wild.
For example the Android Runtime's Thread::InstallImplicitProtection()
routine has relied on this behavior to expand the main thread's stack,
and crashes on x86 kernels with 1be7107fbe18 applied.

NOTE: Marked as HACK since it effectively re-introduces a bug into the
kernel which is exploited by zygote in Android. Fixing that bug results
in a zygote crash in art::Thread::InstallImplicitProtection(). If and when
this problem will be fixed is currently unknown. If possible, this patch
should be dropped at a later time or, if applicable, be replaced with an
upstream patch.

BUG= chromium:726072 ,  chromium:724093 , b:62952017,  chromium:737932 
TEST=Run ARC++

Change-Id: I8d2f6d3e598e99b2605a9caf8e5c6c166de88e0f
Fixes: 1be7107fbe18 ("mm: larger stack guard gap, between vmas")
Signed-off-by: Greg Hackmann <ghackmann@google.com>
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/549182
Reviewed-by: Sonny Rao <sonnyrao@chromium.org>
(cherry picked from commit a936c044f9084f6a5dc0642076b325c396412a6b)
Reviewed-on: https://chromium-review.googlesource.com/561965

[modify] https://crrev.com/62dca0a6127026debe502b9fff47ce6e00868899/arch/x86/mm/fault.c

Project Member

Comment 26 by sheriffbot@chromium.org, Oct 13 2017

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 27 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment