[SRU 0/2 A/Z/Y/X/T] Follow up fixes for CVE-2017-1000364

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[SRU 0/2 A/Z/Y/X/T] Follow up fixes for CVE-2017-1000364

Stefan Bader-2
As of today there are two follow-up changes upstream which address
problems which were introduced by the stack guard area patches.
The second patch has a backport version for Trusty (which only
accepts fuzz 2 of the first hunk). Compile tested for Trusty.

There was also a bug report about a i386 build failure (which I
remembered) which now has been duplicated against a bug report
that has a linux task that is already marked as fixed released.
Not sure this is/was correct as there were some crashes fixed
before but the i386 one was unique in the sense that the jvm
code was acting that way only for i386.

BugLink: https://bugs.launchpad.net/bugs/1700692 ->
BugLink: https://bugs.launchpad.net/bugs/1699772

-Stefan

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 1/2 A/Z/Y/X/T] mm/mmap.c: do not blow on PROT_NONE MAP_FIXED holes in the stack

Stefan Bader-2
From: Michal Hocko <[hidden email]>

Commit 1be7107fbe18 ("mm: larger stack guard gap, between vmas") has
introduced a regression in some rust and Java environments which are
trying to implement their own stack guard page.  They are punching a new
MAP_FIXED mapping inside the existing stack Vma.

This will confuse expand_{downwards,upwards} into thinking that the
stack expansion would in fact get us too close to an existing non-stack
vma which is a correct behavior wrt safety.  It is a real regression on
the other hand.

Let's work around the problem by considering PROT_NONE mapping as a part
of the stack.  This is a gros hack but overflowing to such a mapping
would trap anyway an we only can hope that usespace knows what it is
doing and handle it propely.

Fixes: 1be7107fbe18 ("mm: larger stack guard gap, between vmas")
Link: http://lkml.kernel.org/r/20170705182849.GA18027@...
Signed-off-by: Michal Hocko <[hidden email]>
Debugged-by: Vlastimil Babka <[hidden email]>
Cc: Ben Hutchings <[hidden email]>
Cc: Willy Tarreau <[hidden email]>
Cc: Oleg Nesterov <[hidden email]>
Cc: Rik van Riel <[hidden email]>
Cc: Hugh Dickins <[hidden email]>
Signed-off-by: Andrew Morton <[hidden email]>
Signed-off-by: Linus Torvalds <[hidden email]>

CVE-2017-1000364

(cherry picked from commit 561b5e0709e4a248c67d024d4d94b6e31e3edf2f)
Signed-off-by: Stefan Bader <[hidden email]>
---
 mm/mmap.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 7f8cfe9..d190287 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2244,7 +2244,8 @@ int expand_upwards(struct vm_area_struct *vma, unsigned long address)
  gap_addr = TASK_SIZE;
 
  next = vma->vm_next;
- if (next && next->vm_start < gap_addr) {
+ if (next && next->vm_start < gap_addr &&
+ (next->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) {
  if (!(next->vm_flags & VM_GROWSUP))
  return -ENOMEM;
  /* Check that both stack segments have the same anon_vma? */
@@ -2328,7 +2329,8 @@ int expand_downwards(struct vm_area_struct *vma,
  if (gap_addr > address)
  return -ENOMEM;
  prev = vma->vm_prev;
- if (prev && prev->vm_end > gap_addr) {
+ if (prev && prev->vm_end > gap_addr &&
+ (prev->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) {
  if (!(prev->vm_flags & VM_GROWSDOWN))
  return -ENOMEM;
  /* Check that both stack segments have the same anon_vma? */
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 2/2 A/Z/Y/X] mm/mmap.c: expand_downwards: don't require the gap if !vm_prev

Stefan Bader-2
In reply to this post by Stefan Bader-2
From: Oleg Nesterov <[hidden email]>

expand_stack(vma) fails if address < stack_guard_gap even if there is no
vma->vm_prev.  I don't think this makes sense, and we didn't do this
before the recent commit 1be7107fbe18 ("mm: larger stack guard gap,
between vmas").

We do not need a gap in this case, any address is fine as long as
security_mmap_addr() doesn't object.

This also simplifies the code, we know that address >= prev->vm_end and
thus underflow is not possible.

Link: http://lkml.kernel.org/r/20170628175258.GA24881@...
Signed-off-by: Oleg Nesterov <[hidden email]>
Acked-by: Michal Hocko <[hidden email]>
Cc: Hugh Dickins <[hidden email]>
Cc: Larry Woodman <[hidden email]>
Signed-off-by: Andrew Morton <[hidden email]>
Signed-off-by: Linus Torvalds <[hidden email]>

CVE-2017-1000364

(cherry picked from commit 32e4e6d5cbb0c0e427391635991fe65e17797af8)
Signed-off-by: Stefan Bader <[hidden email]>
---
 mm/mmap.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index d190287..49c56b8 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2316,7 +2316,6 @@ int expand_downwards(struct vm_area_struct *vma,
 {
  struct mm_struct *mm = vma->vm_mm;
  struct vm_area_struct *prev;
- unsigned long gap_addr;
  int error;
 
  address &= PAGE_MASK;
@@ -2325,15 +2324,12 @@ int expand_downwards(struct vm_area_struct *vma,
  return error;
 
  /* Enforce stack_guard_gap */
- gap_addr = address - stack_guard_gap;
- if (gap_addr > address)
- return -ENOMEM;
  prev = vma->vm_prev;
- if (prev && prev->vm_end > gap_addr &&
+ /* Check that both stack segments have the same anon_vma? */
+ if (prev && !(prev->vm_flags & VM_GROWSDOWN) &&
  (prev->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) {
- if (!(prev->vm_flags & VM_GROWSDOWN))
+ if (address - prev->vm_end < stack_guard_gap)
  return -ENOMEM;
- /* Check that both stack segments have the same anon_vma? */
  }
 
  /* We must make sure the anon_vma is allocated. */
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 2/2 T] mm/mmap.c: expand_downwards: don't require the gap if !vm_prev

Stefan Bader-2
In reply to this post by Stefan Bader-2
From: Oleg Nesterov <[hidden email]>

expand_stack(vma) fails if address < stack_guard_gap even if there is no
vma->vm_prev.  I don't think this makes sense, and we didn't do this
before the recent commit 1be7107fbe18 ("mm: larger stack guard gap,
between vmas").

We do not need a gap in this case, any address is fine as long as
security_mmap_addr() doesn't object.

This also simplifies the code, we know that address >= prev->vm_end and
thus underflow is not possible.

Link: http://lkml.kernel.org/r/20170628175258.GA24881@...
Signed-off-by: Oleg Nesterov <[hidden email]>
Acked-by: Michal Hocko <[hidden email]>
Cc: Hugh Dickins <[hidden email]>
Cc: Larry Woodman <[hidden email]>
Signed-off-by: Andrew Morton <[hidden email]>
Signed-off-by: Linus Torvalds <[hidden email]>

CVE-2017-1000364

(backported from commit 32e4e6d5cbb0c0e427391635991fe65e17797af8)
Signed-off-by: Stefan Bader <[hidden email]>
---
 mm/mmap.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 624ccb2..72f35bc 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2227,7 +2227,6 @@ int expand_downwards(struct vm_area_struct *vma,
    unsigned long address)
 {
  struct vm_area_struct *prev;
- unsigned long gap_addr;
  int error;
 
  address &= PAGE_MASK;
@@ -2236,15 +2235,12 @@ int expand_downwards(struct vm_area_struct *vma,
  return error;
 
  /* Enforce stack_guard_gap */
- gap_addr = address - stack_guard_gap;
- if (gap_addr > address)
- return -ENOMEM;
  prev = vma->vm_prev;
- if (prev && prev->vm_end > gap_addr &&
+ /* Check that both stack segments have the same anon_vma? */
+ if (prev && !(prev->vm_flags & VM_GROWSDOWN) &&
  (prev->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) {
- if (!(prev->vm_flags & VM_GROWSDOWN))
+ if (address - prev->vm_end < stack_guard_gap)
  return -ENOMEM;
- /* Check that both stack segments have the same anon_vma? */
  }
 
  /* We must make sure the anon_vma is allocated. */
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

ACK: [SRU 0/2 A/Z/Y/X/T] Follow up fixes for CVE-2017-1000364

Kleber Souza
In reply to this post by Stefan Bader-2
On 07/17/17 14:53, Stefan Bader wrote:

> As of today there are two follow-up changes upstream which address
> problems which were introduced by the stack guard area patches.
> The second patch has a backport version for Trusty (which only
> accepts fuzz 2 of the first hunk). Compile tested for Trusty.
>
> There was also a bug report about a i386 build failure (which I
> remembered) which now has been duplicated against a bug report
> that has a linux task that is already marked as fixed released.
> Not sure this is/was correct as there were some crashes fixed
> before but the i386 one was unique in the sense that the jvm
> code was acting that way only for i386.
>
> BugLink: https://bugs.launchpad.net/bugs/1700692 ->
> BugLink: https://bugs.launchpad.net/bugs/1699772
>
> -Stefan
>

For the cherry-picks and the backport:

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

ACK: [SRU 0/2 A/Z/Y/X/T] Follow up fixes for CVE-2017-1000364

Kamal Mostafa-2
In reply to this post by Stefan Bader-2
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

APPLIED [Z/Y/X/T]: [PATCH 1/2 A/Z/Y/X/T] mm/mmap.c: do not blow on PROT_NONE MAP_FIXED holes in the stack

Kleber Souza
In reply to this post by Stefan Bader-2
Applied to master-next branches of Zesty, Yakkety, Xenial and Trusty trees.

Thanks,
Kleber

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

APPLIED [Z/Y/X]: [PATCH 2/2 A/Z/Y/X] mm/mmap.c: expand_downwards: don't require the gap if !vm_prev

Kleber Souza
In reply to this post by Stefan Bader-2
Applied to the master-next branches of Zesty, Yakkety and Xenial trees.

Thanks,
Kleber

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

APPLIED: [PATCH 2/2 T] mm/mmap.c: expand_downwards: don't require the gap if !vm_prev

Kleber Souza
In reply to this post by Stefan Bader-2
Applied to trusty/master-next branch.

Thanks,
Kleber

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

APPLIED[artful]: [SRU 0/2 A/Z/Y/X/T] Follow up fixes for CVE-2017-1000364

Seth Forshee
In reply to this post by Stefan Bader-2
On Mon, Jul 17, 2017 at 02:53:27PM +0200, Stefan Bader wrote:

> As of today there are two follow-up changes upstream which address
> problems which were introduced by the stack guard area patches.
> The second patch has a backport version for Trusty (which only
> accepts fuzz 2 of the first hunk). Compile tested for Trusty.
>
> There was also a bug report about a i386 build failure (which I
> remembered) which now has been duplicated against a bug report
> that has a linux task that is already marked as fixed released.
> Not sure this is/was correct as there were some crashes fixed
> before but the i386 one was unique in the sense that the jvm
> code was acting that way only for i386.
>
> BugLink: https://bugs.launchpad.net/bugs/1700692 ->
> BugLink: https://bugs.launchpad.net/bugs/1699772

Applied to artful/master-next and unstable/master, thanks.

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Loading...