x86, mpx: do not set ->vm_ops on mpx VMAs

PATCH In review Dave Hansen 6 participants 10 messages Jul 20, 2015 – Aug 10, 2015

Subsystems (3)

Stable branch x86 x86 MM

lore

Share & tools

lore

Dave Hansen

author

2015-07-20 21:29 UTC #0 lore raw

PATCH 1/1 x86, mpx: do not set ->vm_ops on mpx VMAs

(sorry for the spam, I screwed up the stable@ address).

BTW, thanks to Kirill for doing this patch! He posted it to LKML but we need to ensure it is picked up for 4.2 and any -stable kernels where this commit is applied:

6b7339f4: mm: avoid setting up anonymous pages into file mapping

That broke MPX support because MPX sets a vma->vm_ops on an anonymous VMA. We need this patch to make it work again, basically removing MPX's use of ->vm_ops. Kirill made me aware of this long ago, but I didn't double-check that his fix got submitted and merged.

I (Dave) fixed up a minor merge conflict and added the try_unmap_single_bt() use of is_mpx_vma() (which were added post-4.1).

Note for -stable: The first hunk may not apply cleanly because of other activity in arch/x86/mm/mmap.c, but should be trivial to apply by hand. Hunk #5 on mpx.c is only present on 4.2-rc kernels.

From: Kirill A. Shutemov <[email protected]>

MPX setups private anonymous mapping, but uses vma->vm_ops too. This can confuse core VM, as it relies on vm->vm_ops to distinguish file VMAs from anonymous.

As result we will get SIGBUS, because handle_pte_fault() thinks it's file VMA without vm_ops->fault and it doesn't know how to handle the situation properly.

Let's fix that by not setting ->vm_ops.

We don't really need ->vm_ops here: MPX VMA can be detected with VM_MPX flag. And vma_merge() will not merge MPX VMA with non-MPX VMA, because ->vm_flags won't match.

The only thing left is name of VMA. I'm not sure if it's part of ABI, or we can just drop it. The patch keep it by providing arch_vma_name() on x86.

Build tested only. (by Kirill)
Build, boot and MPX tested (by Dave)

Signed-off-by Kirill A. Shutemov <[email protected]>

Signed-off-by Dave Hansen <[email protected]>

Cc Andy Lutomirski <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc [email protected]

Cc <[email protected]> # 6b7339f4: mm: avoid setting up anonymous pages into file mapping

Cc [email protected]

---

 b/arch/x86/mm/mmap.c |    7 +++++++
 b/arch/x86/mm/mpx.c  |   24 +++---------------------
 2 files changed, 10 insertions(+), 21 deletions(-)

arch/x86/mm/mmap.c renamed from arch/x86/mm/mmap.c~2_5-x86-mpx-do-not-set-vm-ops-on-mpx-VMAs +7 -0

diff -puN arch/x86/mm/mmap.c~2_5-x86-mpx-do-not-set-vm-ops-on-mpx-VMAs arch/x86/mm/mmap.c
@@ -126,3 +126,10 @@ void arch_pick_mmap_layout(struct mm_str
 		mm->get_unmapped_area = arch_get_unmapped_area_topdown;
 	}
 }
+
+const char *arch_vma_name(struct vm_area_struct *vma)
+{
+	if (vma->vm_flags & VM_MPX)
+		return "[mpx]";
+	return NULL;
+}

arch/x86/mm/mpx.c renamed from arch/x86/mm/mpx.c~2_5-x86-mpx-do-not-set-vm-ops-on-mpx-VMAs +3 ~~-21~~

diff -puN arch/x86/mm/mpx.c~2_5-x86-mpx-do-not-set-vm-ops-on-mpx-VMAs arch/x86/mm/mpx.c
@@ -20,20 +20,6 @@
 #define CREATE_TRACE_POINTS
 #include <asm/trace/mpx.h>
 
-static const char *mpx_mapping_name(struct vm_area_struct *vma)
-{
-	return "[mpx]";
-}
-
-static struct vm_operations_struct mpx_vma_ops = {
-	.name = mpx_mapping_name,
-};
-
-static int is_mpx_vma(struct vm_area_struct *vma)
-{
-	return (vma->vm_ops == &mpx_vma_ops);
-}
-
 static inline unsigned long mpx_bd_size_bytes(struct mm_struct *mm)
 {
 	if (is_64bit_mm(mm))

@@ -53,9 +39,6 @@ static inline unsigned long mpx_bt_size_
 /*
  * This is really a simplified "vm_mmap". it only handles MPX
  * bounds tables (the bounds directory is user-allocated).
- *
- * Later on, we use the vma->vm_ops to uniquely identify these
- * VMAs.
  */
 static unsigned long mpx_mmap(unsigned long len)
 {

@@ -101,7 +84,6 @@ static unsigned long mpx_mmap(unsigned l
 		ret = -ENOMEM;
 		goto out;
 	}
-	vma->vm_ops = &mpx_vma_ops;
 
 	if (vm_flags & VM_LOCKED) {
 		up_write(&mm->mmap_sem);

@@ -812,7 +794,7 @@ static noinline int zap_bt_entries_mappi
 		 * so stop immediately and return an error.  This
 		 * probably results in a SIGSEGV.
 		 */
-		if (!is_mpx_vma(vma))
+		if (!(vma->vm_flags & VM_MPX))
 			return -EINVAL;
 
 		len = min(vma->vm_end, end) - addr;

@@ -945,9 +927,9 @@ static int try_unmap_single_bt(struct mm
 	 * lots of tables even though we have no actual table
 	 * entries in use.
 	 */
-	while (next && is_mpx_vma(next))
+	while (next && (next->vm_flags & VM_MPX))
 		next = next->vm_next;
-	while (prev && is_mpx_vma(prev))
+	while (prev && (prev->vm_flags & VM_MPX))
 		prev = prev->vm_prev;
 	/*
 	 * We know 'start' and 'end' lie within an area controlled

tip-bot for Kirill A. Shutemov

merge bot

2015-07-21 09:38 UTC #1 lore raw

[tip:x86/urgent] x86/mpx: Do not set ->vm_ops on MPX VMAs

x86/mpx: Do not set ->vm_ops on MPX VMAs

Commit-ID:  a89652769470d12cd484ee3d3f7bde0742be8d96
Gitweb:     http://git.kernel.org/tip/a89652769470d12cd484ee3d3f7bde0742be8d96
Author:     Kirill A. Shutemov <[email protected]>
AuthorDate: Mon, 20 Jul 2015 14:29:58 -0700
Committer:  Ingo Molnar <[email protected]>
CommitDate: Tue, 21 Jul 2015 07:57:16 +0200

x86/mpx: Do not set ->vm_ops on MPX VMAs

MPX setups private anonymous mapping, but uses vma->vm_ops too. This can confuse core VM, as it relies on vm->vm_ops to distinguish file VMAs from anonymous.

As result we will get SIGBUS, because handle_pte_fault() thinks it's file VMA without vm_ops->fault and it doesn't know how to handle the situation properly.

Let's fix that by not setting ->vm_ops.

We don't really need ->vm_ops here: MPX VMA can be detected with VM_MPX flag. And vma_merge() will not merge MPX VMA with non-MPX VMA, because ->vm_flags won't match.

The only thing left is name of VMA. I'm not sure if it's part of ABI, or we can just drop it. The patch keep it by providing arch_vma_name() on x86.

Signed-off-by Kirill A. Shutemov <[email protected]>

Signed-off-by Dave Hansen <[email protected]>

Cc <[email protected]> # Fixes: 6b7339f4 (mm: avoid setting up anonymous pages into file mapping)

Cc Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc [email protected]

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/mm/mmap.c |  7 +++++++
 arch/x86/mm/mpx.c  | 24 +++---------------------
 2 files changed, 10 insertions(+), 21 deletions(-)

arch/x86/mm/mmap.c +7 -0

index 9d518d6..844b06d 100644
@@ -126,3 +126,10 @@ void arch_pick_mmap_layout(struct mm_struct *mm)
 		mm->get_unmapped_area = arch_get_unmapped_area_topdown;
 	}
 }
+
+const char *arch_vma_name(struct vm_area_struct *vma)
+{
+	if (vma->vm_flags & VM_MPX)
+		return "[mpx]";
+	return NULL;
+}

arch/x86/mm/mpx.c +3 ~~-21~~

index 7a657f5..db1b0bc 100644
@@ -20,20 +20,6 @@
 #define CREATE_TRACE_POINTS
 #include <asm/trace/mpx.h>
 
-static const char *mpx_mapping_name(struct vm_area_struct *vma)
-{
-	return "[mpx]";
-}
-
-static struct vm_operations_struct mpx_vma_ops = {
-	.name = mpx_mapping_name,
-};
-
-static int is_mpx_vma(struct vm_area_struct *vma)
-{
-	return (vma->vm_ops == &mpx_vma_ops);
-}
-
 static inline unsigned long mpx_bd_size_bytes(struct mm_struct *mm)
 {
 	if (is_64bit_mm(mm))

@@ -53,9 +39,6 @@ static inline unsigned long mpx_bt_size_bytes(struct mm_struct *mm)
 /*
  * This is really a simplified "vm_mmap". it only handles MPX
  * bounds tables (the bounds directory is user-allocated).
- *
- * Later on, we use the vma->vm_ops to uniquely identify these
- * VMAs.
  */
 static unsigned long mpx_mmap(unsigned long len)
 {

@@ -101,7 +84,6 @@ static unsigned long mpx_mmap(unsigned long len)
 		ret = -ENOMEM;
 		goto out;
 	}
-	vma->vm_ops = &mpx_vma_ops;
 
 	if (vm_flags & VM_LOCKED) {
 		up_write(&mm->mmap_sem);

@@ -812,7 +794,7 @@ static noinline int zap_bt_entries_mapping(struct mm_struct *mm,
 		 * so stop immediately and return an error.  This
 		 * probably results in a SIGSEGV.
 		 */
-		if (!is_mpx_vma(vma))
+		if (!(vma->vm_flags & VM_MPX))
 			return -EINVAL;
 
 		len = min(vma->vm_end, end) - addr;

@@ -945,9 +927,9 @@ static int try_unmap_single_bt(struct mm_struct *mm,
 	 * lots of tables even though we have no actual table
 	 * entries in use.
 	 */
-	while (next && is_mpx_vma(next))
+	while (next && (next->vm_flags & VM_MPX))
 		next = next->vm_next;
-	while (prev && is_mpx_vma(prev))
+	while (prev && (prev->vm_flags & VM_MPX))
 		prev = prev->vm_prev;
 	/*
 	 * We know 'start' and 'end' lie within an area controlled

Greg KH

2015-08-01 00:02 UTC #2 lore raw

In reply to Dave Hansen

Quoted 15 lines · Dave Hansen

(sorry for the spam, I screwed up the stable@ address).
BTW, thanks to Kirill for doing this patch! He posted it to LKML but we need to ensure it is picked up for 4.2 and any -stable kernels where this commit is applied:
6b7339f4: mm: avoid setting up anonymous pages into file mapping
That broke MPX support because MPX sets a vma->vm_ops on an anonymous VMA. We need this patch to make it work again, basically removing MPX's use of ->vm_ops. Kirill made me aware of this long ago, but I didn't double-check that his fix got submitted and merged.

Quoted 7 lines

I (Dave) fixed up a minor merge conflict and added the try_unmap_single_bt() use of is_mpx_vma() (which were added post-4.1).
Note for -stable: The first hunk may not apply cleanly because of other activity in arch/x86/mm/mmap.c, but should be trivial to apply by hand. Hunk #5 on mpx.c is only present on 4.2-rc kernels.

Can someone send a version that is known to apply, you don't want to rely on me to get it right :)

thanks,

greg k-h

Kirill A. Shutemov

2015-08-03 10:34 UTC #3 lore raw

Quoted 20 lines · Greg KH

Quoted 19 lines · Dave Hansen
(sorry for the spam, I screwed up the stable@ address).
BTW, thanks to Kirill for doing this patch! He posted it to LKML but we need to ensure it is picked up for 4.2 and any -stable kernels where this commit is applied:
6b7339f4: mm: avoid setting up anonymous pages into file mapping
That broke MPX support because MPX sets a vma->vm_ops on an anonymous VMA. We need this patch to make it work again, basically removing MPX's use of ->vm_ops. Kirill made me aware of this long ago, but I didn't double-check that his fix got submitted and merged.
I (Dave) fixed up a minor merge conflict and added the try_unmap_single_bt() use of is_mpx_vma() (which were added post-4.1).

Quoted 6 lines

Quoted 3 lines
Note for -stable: The first hunk may not apply cleanly because of other activity in arch/x86/mm/mmap.c, but should be trivial to apply by hand. Hunk #5 on mpx.c is only present on 4.2-rc kernels.
Can someone send a version that is known to apply, you don't want to rely on me to get it right :)

Here it is.

MPX support was introduced in 3.19, so it only need to be applied to 4.1-stable.

Quoted 1 line

From 62d8093f0bfd79eff2c87e8924cb9276116aaabe Mon Sep 17 00:00:00 2001

From: "Kirill A. Shutemov" <[email protected]>
Date: Mon, 20 Jul 2015 14:29:58 -0700
Subject: [PATCH] x86/mpx: Do not set ->vm_ops on MPX VMAs

MPX setups private anonymous mapping, but uses vma->vm_ops too. This can confuse core VM, as it relies on vm->vm_ops to distinguish file VMAs from anonymous.

As result we will get SIGBUS, because handle_pte_fault() thinks it's file VMA without vm_ops->fault and it doesn't know how to handle the situation properly.

Let's fix that by not setting ->vm_ops.

We don't really need ->vm_ops here: MPX VMA can be detected with VM_MPX flag. And vma_merge() will not merge MPX VMA with non-MPX VMA, because ->vm_flags won't match.

The only thing left is name of VMA. I'm not sure if it's part of ABI, or we can just drop it. The patch keep it by providing arch_vma_name() on x86.

Signed-off-by Kirill A. Shutemov <[email protected]>

Signed-off-by Dave Hansen <[email protected]>

Cc <[email protected]> # Fixes: 6b7339f4 (mm: avoid setting up anonymous pages into file mapping)

Cc Andy Lutomirski <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc [email protected]

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Ingo Molnar <[email protected]>

---
 arch/x86/mm/mmap.c |  7 +++++++
 arch/x86/mm/mpx.c  | 20 +-------------------
 2 files changed, 8 insertions(+), 19 deletions(-)

arch/x86/mm/mmap.c +7 -0

index 9d518d693b4b..844b06d67df4 100644
@@ -126,3 +126,10 @@ void arch_pick_mmap_layout(struct mm_struct *mm)
 		mm->get_unmapped_area = arch_get_unmapped_area_topdown;
 	}
 }
+
+const char *arch_vma_name(struct vm_area_struct *vma)
+{
+	if (vma->vm_flags & VM_MPX)
+		return "[mpx]";
+	return NULL;
+}

arch/x86/mm/mpx.c +1 ~~-19~~

index c439ec478216..4d1c11c07fe1 100644
@@ -18,26 +18,9 @@
 #include <asm/processor.h>
 #include <asm/fpu-internal.h>
 
-static const char *mpx_mapping_name(struct vm_area_struct *vma)
-{
-	return "[mpx]";
-}
-
-static struct vm_operations_struct mpx_vma_ops = {
-	.name = mpx_mapping_name,
-};
-
-static int is_mpx_vma(struct vm_area_struct *vma)
-{
-	return (vma->vm_ops == &mpx_vma_ops);
-}
-
 /*
  * This is really a simplified "vm_mmap". it only handles MPX
  * bounds tables (the bounds directory is user-allocated).
- *
- * Later on, we use the vma->vm_ops to uniquely identify these
- * VMAs.
  */
 static unsigned long mpx_mmap(unsigned long len)
 {

@@ -83,7 +66,6 @@ static unsigned long mpx_mmap(unsigned long len)
 		ret = -ENOMEM;
 		goto out;
 	}
-	vma->vm_ops = &mpx_vma_ops;
 
 	if (vm_flags & VM_LOCKED) {
 		up_write(&mm->mmap_sem);

@@ -661,7 +643,7 @@ static int zap_bt_entries(struct mm_struct *mm,
 		 * so stop immediately and return an error.  This
 		 * probably results in a SIGSEGV.
 		 */
-		if (!is_mpx_vma(vma))
+		if (!(vma->vm_flags & VM_MPX))
 			return -EINVAL;
 
 		len = min(vma->vm_end, end) - addr;

Signature · 1 line

2.4.6

Greg KH

2015-08-03 16:19 UTC #4 lore raw

Quoted 25 lines · Kirill A. Shutemov

Quoted 24 lines · Greg KH
Quoted 22 lines · Dave Hansen
(sorry for the spam, I screwed up the stable@ address).
BTW, thanks to Kirill for doing this patch! He posted it to LKML but we need to ensure it is picked up for 4.2 and any -stable kernels where this commit is applied:
6b7339f4: mm: avoid setting up anonymous pages into file mapping
That broke MPX support because MPX sets a vma->vm_ops on an anonymous VMA. We need this patch to make it work again, basically removing MPX's use of ->vm_ops. Kirill made me aware of this long ago, but I didn't double-check that his fix got submitted and merged.
I (Dave) fixed up a minor merge conflict and added the try_unmap_single_bt() use of is_mpx_vma() (which were added post-4.1).
Note for -stable: The first hunk may not apply cleanly because of other activity in arch/x86/mm/mmap.c, but should be trivial to apply by hand. Hunk #5 on mpx.c is only present on 4.2-rc kernels.

Quoted 7 lines

Quoted 2 lines
Can someone send a version that is known to apply, you don't want to rely on me to get it right :)
Here it is.
MPX support was introduced in 3.19, so it only need to be applied to 4.1-stable.

Thanks, now applied.

greg k-h

Kirill A. Shutemov

2015-08-03 19:40 UTC #5 lore raw

Quoted 29 lines · Greg KH

Quoted 28 lines · Kirill A. Shutemov
Quoted 26 lines · Greg KH
Quoted 15 lines · Dave Hansen
(sorry for the spam, I screwed up the stable@ address).
BTW, thanks to Kirill for doing this patch! He posted it to LKML but we need to ensure it is picked up for 4.2 and any -stable kernels where this commit is applied:
6b7339f4: mm: avoid setting up anonymous pages into file mapping
That broke MPX support because MPX sets a vma->vm_ops on an anonymous VMA. We need this patch to make it work again, basically removing MPX's use of ->vm_ops. Kirill made me aware of this long ago, but I didn't double-check that his fix got submitted and merged.
Quoted 7 lines
I (Dave) fixed up a minor merge conflict and added the try_unmap_single_bt() use of is_mpx_vma() (which were added post-4.1).
Note for -stable: The first hunk may not apply cleanly because of other activity in arch/x86/mm/mmap.c, but should be trivial to apply by hand. Hunk #5 on mpx.c is only present on 4.2-rc kernels.
Can someone send a version that is known to apply, you don't want to rely on me to get it right :)

Quoted 6 lines

Quoted 4 lines
Here it is.
MPX support was introduced in 3.19, so it only need to be applied to 4.1-stable.
Thanks, now applied.

Just to make clear: we don't need MPX patch for older stables, but we *do* need 6b7339f4 (mm: avoid setting up anonymous pages into file mapping) to be ported into all stables.

Signature · 1 line

 Kirill A. Shutemov

Greg KH

2015-08-03 20:36 UTC #6 lore raw

Quoted 32 lines · Kirill A. Shutemov

Quoted 31 lines · Greg KH
Quoted 30 lines · Kirill A. Shutemov
Quoted 20 lines · Greg KH
Quoted 19 lines · Dave Hansen
(sorry for the spam, I screwed up the stable@ address).
BTW, thanks to Kirill for doing this patch! He posted it to LKML but we need to ensure it is picked up for 4.2 and any -stable kernels where this commit is applied:
6b7339f4: mm: avoid setting up anonymous pages into file mapping
That broke MPX support because MPX sets a vma->vm_ops on an anonymous VMA. We need this patch to make it work again, basically removing MPX's use of ->vm_ops. Kirill made me aware of this long ago, but I didn't double-check that his fix got submitted and merged.
I (Dave) fixed up a minor merge conflict and added the try_unmap_single_bt() use of is_mpx_vma() (which were added post-4.1).
Quoted 6 lines
Quoted 3 lines
Note for -stable: The first hunk may not apply cleanly because of other activity in arch/x86/mm/mmap.c, but should be trivial to apply by hand. Hunk #5 on mpx.c is only present on 4.2-rc kernels.
Can someone send a version that is known to apply, you don't want to rely on me to get it right :)
Here it is.

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
MPX support was introduced in 3.19, so it only need to be applied to 4.1-stable.
Thanks, now applied.
Just to make clear: we don't need MPX patch for older stables, but we *do* need 6b7339f4 (mm: avoid setting up anonymous pages into file mapping) to be ported into all stables.

Can you provide that backport, it isn't a direct cherry-pick.

thanks,

greg k-h

Kirill A. Shutemov

Acked-by Oleg Nesterov Acked-by Oleg Nesterov Acked-by Oleg Nesterov

2015-08-03 21:39 UTC #7 lore raw

Quoted 36 lines · Greg KH

Quoted 35 lines · Kirill A. Shutemov
Quoted 34 lines · Greg KH
Quoted 32 lines · Kirill A. Shutemov
Quoted 20 lines · Greg KH
Quoted 19 lines · Dave Hansen
(sorry for the spam, I screwed up the stable@ address).
BTW, thanks to Kirill for doing this patch! He posted it to LKML but we need to ensure it is picked up for 4.2 and any -stable kernels where this commit is applied:
6b7339f4: mm: avoid setting up anonymous pages into file mapping
That broke MPX support because MPX sets a vma->vm_ops on an anonymous VMA. We need this patch to make it work again, basically removing MPX's use of ->vm_ops. Kirill made me aware of this long ago, but I didn't double-check that his fix got submitted and merged.
I (Dave) fixed up a minor merge conflict and added the try_unmap_single_bt() use of is_mpx_vma() (which were added post-4.1).
Quoted 6 lines
Quoted 3 lines
Note for -stable: The first hunk may not apply cleanly because of other activity in arch/x86/mm/mmap.c, but should be trivial to apply by hand. Hunk #5 on mpx.c is only present on 4.2-rc kernels.
Can someone send a version that is known to apply, you don't want to rely on me to get it right :)
Here it is.
MPX support was introduced in 3.19, so it only need to be applied to 4.1-stable.

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
Thanks, now applied.
Just to make clear: we don't need MPX patch for older stables, but we *do* need 6b7339f4 (mm: avoid setting up anonymous pages into file mapping) to be ported into all stables.
Can you provide that backport, it isn't a direct cherry-pick.

Patches are in attach.

0001-mm-avoid-setting-up-anonymous-pages-3.18.patch is rebased onto v3.18.

0001-mm-avoid-setting-up-anonymous-pages-3.14.patch is rebased onto v3.14, also applies cleanly to v3.12, v3.10, v3.4, v3.2.

0001-mm-avoid-setting-up-anonymous-pages-2.6.32.patch is rebased onto v2.6.32.

Build tested for all these versions.

Signature · 1 line

 Kirill A. Shutemov

Quoted 1 line

From 41456c4806bb0b4561602e037183790020a13532 Mon Sep 17 00:00:00 2001

From: "Kirill A. Shutemov" <[email protected]>
Date: Mon, 6 Jul 2015 23:18:37 +0300
Subject: [PATCH] mm: avoid setting up anonymous pages into file mapping

Reading page fault handler code I've noticed that under right circumstances kernel would map anonymous pages into file mappings: if the VMA doesn't have vm_ops->fault() and the VMA wasn't fully populated on ->mmap(), kernel would handle page fault to not populated pte with do_anonymous_page().

Let's change page fault handler to use do_anonymous_page() only on anonymous VMA (->vm_ops == NULL) and make sure that the VMA is not shared.

For file mappings without vm_ops->fault() or shred VMA without vm_ops, page fault on pte_none() entry would lead to SIGBUS.

Signed-off-by Kirill A. Shutemov <[email protected]>

Acked-by Oleg Nesterov <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Willy Tarreau <[email protected]>

Cc [email protected]

Signed-off-by Linus Torvalds <[email protected]>

---
 mm/memory.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

mm/memory.c +10 -4

index d5f2ae9c4a23..04979a2b3eb4 100644
@@ -2636,6 +2636,10 @@ static int do_anonymous_page(struct mm_struct *mm, struct vm_area_struct *vma,
 
 	pte_unmap(page_table);
 
+	/* File mapping without ->vm_ops ? */
+	if (vma->vm_flags & VM_SHARED)
+		return VM_FAULT_SIGBUS;
+
 	/* Check if we need to add a guard page to the stack */
 	if (check_stack_guard_page(vma, address) < 0)
 		return VM_FAULT_SIGBUS;

@@ -3040,6 +3044,9 @@ static int do_linear_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 			- vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff;
 
 	pte_unmap(page_table);
+	/* The VMA was not fully populated on mmap() or missing VM_DONTEXPAND */
+	if (!vma->vm_ops->fault)
+		return VM_FAULT_SIGBUS;
 	if (!(flags & FAULT_FLAG_WRITE))
 		return do_read_fault(mm, vma, address, pmd, pgoff, flags,
 				orig_pte);

@@ -3205,11 +3212,10 @@ static int handle_pte_fault(struct mm_struct *mm,
 	entry = ACCESS_ONCE(*pte);
 	if (!pte_present(entry)) {
 		if (pte_none(entry)) {
-			if (vma->vm_ops) {
-				if (likely(vma->vm_ops->fault))
-					return do_linear_fault(mm, vma, address,
+			if (vma->vm_ops)
+				return do_linear_fault(mm, vma, address,
 						pte, pmd, flags, entry);
-			}
+
 			return do_anonymous_page(mm, vma, address,
 						 pte, pmd, flags);
 		}

Signature · 1 line

2.4.6

Quoted 1 line

From 1d549cf546de91706f6c199f37bf62b6af656c44 Mon Sep 17 00:00:00 2001

From: "Kirill A. Shutemov" <[email protected]>
Date: Mon, 6 Jul 2015 23:18:37 +0300
Subject: [PATCH] mm: avoid setting up anonymous pages into file mapping

Let's change page fault handler to use do_anonymous_page() only on anonymous VMA (->vm_ops == NULL) and make sure that the VMA is not shared.

For file mappings without vm_ops->fault() or shred VMA without vm_ops, page fault on pte_none() entry would lead to SIGBUS.

Signed-off-by Kirill A. Shutemov <[email protected]>

Acked-by Oleg Nesterov <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Willy Tarreau <[email protected]>

Cc [email protected]

Signed-off-by Linus Torvalds <[email protected]>

---
 mm/memory.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

mm/memory.c +9 -4

index 22dfa617bddb..86c09a8284cd 100644
@@ -3228,6 +3228,10 @@ static int do_anonymous_page(struct mm_struct *mm, struct vm_area_struct *vma,
 
 	pte_unmap(page_table);
 
+	/* File mapping without ->vm_ops ? */
+	if (vma->vm_flags & VM_SHARED)
+		return VM_FAULT_SIGBUS;
+
 	/* Check if we need to add a guard page to the stack */
 	if (check_stack_guard_page(vma, address) < 0)
 		return VM_FAULT_SIGBUS;

@@ -3496,6 +3500,9 @@ static int do_linear_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 			- vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff;
 
 	pte_unmap(page_table);
+	/* The VMA was not fully populated on mmap() or missing VM_DONTEXPAND */
+	if (!vma->vm_ops->fault)
+		return VM_FAULT_SIGBUS;
 	return __do_fault(mm, vma, address, pmd, pgoff, flags, orig_pte);
 }

@@ -3644,11 +3651,9 @@ static int handle_pte_fault(struct mm_struct *mm,
 	entry = *pte;
 	if (!pte_present(entry)) {
 		if (pte_none(entry)) {
-			if (vma->vm_ops) {
-				if (likely(vma->vm_ops->fault))
-					return do_linear_fault(mm, vma, address,
+			if (vma->vm_ops)
+				return do_linear_fault(mm, vma, address,
 						pte, pmd, flags, entry);
-			}
 			return do_anonymous_page(mm, vma, address,
 						 pte, pmd, flags);
 		}

Signature · 1 line

2.4.6

Quoted 1 line

From 1d3aa3b9021f908a0411ae5d743434e076f3feab Mon Sep 17 00:00:00 2001

From: "Kirill A. Shutemov" <[email protected]>
Date: Mon, 6 Jul 2015 23:18:37 +0300
Subject: [PATCH] mm: avoid setting up anonymous pages into file mapping

Let's change page fault handler to use do_anonymous_page() only on anonymous VMA (->vm_ops == NULL) and make sure that the VMA is not shared.

For file mappings without vm_ops->fault() or shred VMA without vm_ops, page fault on pte_none() entry would lead to SIGBUS.

Signed-off-by Kirill A. Shutemov <[email protected]>

Acked-by Oleg Nesterov <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Willy Tarreau <[email protected]>

Cc [email protected]

Signed-off-by Linus Torvalds <[email protected]>

---
 mm/memory.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

mm/memory.c +9 -4

index 6ab19dd4a199..06d2a0e74248 100644
@@ -2645,6 +2645,10 @@ static int do_anonymous_page(struct mm_struct *mm, struct vm_area_struct *vma,
 	/* Allocate our own private page. */
 	pte_unmap(page_table);
 
+	/* File mapping without ->vm_ops ? */
+	if (vma->vm_flags & VM_SHARED)
+		return VM_FAULT_SIGBUS;
+
 	if (unlikely(anon_vma_prepare(vma)))
 		goto oom;
 	page = alloc_zeroed_user_highpage_movable(vma, address);

@@ -2882,6 +2886,9 @@ static int do_linear_fault(struct mm_struct *mm, struct vm_area_struct *vma,
 			- vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff;
 
 	pte_unmap(page_table);
+	/* The VMA was not fully populated on mmap() or missing VM_DONTEXPAND */
+	if (!vma->vm_ops->fault)
+		return VM_FAULT_SIGBUS;
 	return __do_fault(mm, vma, address, pmd, pgoff, flags, orig_pte);
 }

@@ -2940,11 +2947,9 @@ static inline int handle_pte_fault(struct mm_struct *mm,
 	entry = *pte;
 	if (!pte_present(entry)) {
 		if (pte_none(entry)) {
-			if (vma->vm_ops) {
-				if (likely(vma->vm_ops->fault))
-					return do_linear_fault(mm, vma, address,
+			if (vma->vm_ops)
+				return do_linear_fault(mm, vma, address,
 						pte, pmd, flags, entry);
-			}
 			return do_anonymous_page(mm, vma, address,
 						 pte, pmd, flags);
 		}

Signature · 1 line

2.4.6

Greg KH

2015-08-06 23:35 UTC #8 lore raw

Quoted 43 lines · Kirill A. Shutemov

Quoted 42 lines · Greg KH
Quoted 40 lines · Kirill A. Shutemov
Quoted 29 lines · Greg KH
Quoted 28 lines · Kirill A. Shutemov
Quoted 26 lines · Greg KH
Quoted 15 lines · Dave Hansen
(sorry for the spam, I screwed up the stable@ address).
BTW, thanks to Kirill for doing this patch! He posted it to LKML but we need to ensure it is picked up for 4.2 and any -stable kernels where this commit is applied:
6b7339f4: mm: avoid setting up anonymous pages into file mapping
That broke MPX support because MPX sets a vma->vm_ops on an anonymous VMA. We need this patch to make it work again, basically removing MPX's use of ->vm_ops. Kirill made me aware of this long ago, but I didn't double-check that his fix got submitted and merged.
Quoted 7 lines
I (Dave) fixed up a minor merge conflict and added the try_unmap_single_bt() use of is_mpx_vma() (which were added post-4.1).
Note for -stable: The first hunk may not apply cleanly because of other activity in arch/x86/mm/mmap.c, but should be trivial to apply by hand. Hunk #5 on mpx.c is only present on 4.2-rc kernels.
Can someone send a version that is known to apply, you don't want to rely on me to get it right :)
Quoted 6 lines
Quoted 4 lines
Here it is.
MPX support was introduced in 3.19, so it only need to be applied to 4.1-stable.
Thanks, now applied.
Just to make clear: we don't need MPX patch for older stables, but we *do* need 6b7339f4 (mm: avoid setting up anonymous pages into file mapping) to be ported into all stables.

Quoted 8 lines

Quoted 1 line
Can you provide that backport, it isn't a direct cherry-pick.
Patches are in attach.
0001-mm-avoid-setting-up-anonymous-pages-3.18.patch is rebased onto v3.18.
0001-mm-avoid-setting-up-anonymous-pages-3.14.patch is rebased onto v3.14, also applies cleanly to v3.12, v3.10, v3.4, v3.2.

Thanks, I've taken this patch now for 3.10 and 3.14.

greg k-h

Luis Henriques

2015-08-10 09:05 UTC #9 lore raw

In reply to Kirill A. Shutemov

Quoted 43 lines · Kirill A. Shutemov

Quoted 42 lines · Greg KH
Quoted 40 lines · Kirill A. Shutemov
Quoted 29 lines · Greg KH
Quoted 28 lines · Kirill A. Shutemov
Quoted 26 lines · Greg KH
Quoted 15 lines · Dave Hansen
(sorry for the spam, I screwed up the stable@ address).
BTW, thanks to Kirill for doing this patch! He posted it to LKML but we need to ensure it is picked up for 4.2 and any -stable kernels where this commit is applied:
6b7339f4: mm: avoid setting up anonymous pages into file mapping
That broke MPX support because MPX sets a vma->vm_ops on an anonymous VMA. We need this patch to make it work again, basically removing MPX's use of ->vm_ops. Kirill made me aware of this long ago, but I didn't double-check that his fix got submitted and merged.
Quoted 7 lines
I (Dave) fixed up a minor merge conflict and added the try_unmap_single_bt() use of is_mpx_vma() (which were added post-4.1).
Note for -stable: The first hunk may not apply cleanly because of other activity in arch/x86/mm/mmap.c, but should be trivial to apply by hand. Hunk #5 on mpx.c is only present on 4.2-rc kernels.
Can someone send a version that is known to apply, you don't want to rely on me to get it right :)
Quoted 6 lines
Quoted 4 lines
Here it is.
MPX support was introduced in 3.19, so it only need to be applied to 4.1-stable.
Thanks, now applied.
Just to make clear: we don't need MPX patch for older stables, but we *do* need 6b7339f4 (mm: avoid setting up anonymous pages into file mapping) to be ported into all stables.

Quoted 6 lines

Quoted 1 line
Can you provide that backport, it isn't a direct cherry-pick.
Patches are in attach.
0001-mm-avoid-setting-up-anonymous-pages-3.18.patch is rebased onto v3.18.

Thanks, I'm queuing this for the 3.16 kernel as well.

Cheers,
--
Luï¿½s

Quoted 10 lines

0001-mm-avoid-setting-up-anonymous-pages-3.14.patch is rebased onto v3.14, also applies cleanly to v3.12, v3.10, v3.4, v3.2.
0001-mm-avoid-setting-up-anonymous-pages-2.6.32.patch is rebased onto v2.6.32.
Build tested for all these versions.
Signature · 1 line
 Kirill A. Shutemov