All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [4.2 fix] x86, mpx: do not set ->vm_ops on mpx VMAs
@ 2015-07-20 21:29 Dave Hansen
  2015-07-21  9:37 ` [tip:x86/urgent] x86/mpx: Do not set ->vm_ops on MPX VMAs tip-bot for Kirill A. Shutemov
  2015-08-01  0:02 ` [PATCH] [4.2 fix] x86, mpx: do not set ->vm_ops on mpx VMAs Greg KH
  0 siblings, 2 replies; 11+ messages in thread
From: Dave Hansen @ 2015-07-20 21:29 UTC (permalink / raw)
  To: dave; +Cc: kirill.shutemov, dave.hansen, luto, tglx, x86, stable, linux-kernel


(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 <kirill.shutemov@linux.intel.com>

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 <kirill.shutemov@linux.intel.com>
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: x86@kernel.org
Cc: <stable@vger.kernel.org> # 6b7339f4: mm: avoid setting up anonymous pages into file mapping
Cc: linux-kernel@vger.kernel.org

---

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

diff -puN arch/x86/mm/mmap.c~2_5-x86-mpx-do-not-set-vm-ops-on-mpx-VMAs arch/x86/mm/mmap.c
--- a/arch/x86/mm/mmap.c~2_5-x86-mpx-do-not-set-vm-ops-on-mpx-VMAs	2015-07-20 13:25:31.012370827 -0700
+++ b/arch/x86/mm/mmap.c	2015-07-20 13:25:31.017371052 -0700
@@ -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;
+}
diff -puN arch/x86/mm/mpx.c~2_5-x86-mpx-do-not-set-vm-ops-on-mpx-VMAs arch/x86/mm/mpx.c
--- a/arch/x86/mm/mpx.c~2_5-x86-mpx-do-not-set-vm-ops-on-mpx-VMAs	2015-07-20 13:25:31.014370917 -0700
+++ b/arch/x86/mm/mpx.c	2015-07-20 13:25:31.017371052 -0700
@@ -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
_

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-08-10  9:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-20 21:29 [PATCH] [4.2 fix] x86, mpx: do not set ->vm_ops on mpx VMAs Dave Hansen
2015-07-21  9:37 ` [tip:x86/urgent] x86/mpx: Do not set ->vm_ops on MPX VMAs tip-bot for Kirill A. Shutemov
2015-08-01  0:02 ` [PATCH] [4.2 fix] x86, mpx: do not set ->vm_ops on mpx VMAs Greg KH
2015-08-03 10:34   ` Kirill A. Shutemov
2015-08-03 16:19     ` Greg KH
2015-08-03 19:40       ` Kirill A. Shutemov
2015-08-03 20:36         ` Greg KH
2015-08-03 21:39           ` Kirill A. Shutemov
2015-08-06 23:35             ` Greg KH
2015-08-10  9:05             ` Luis Henriques
2015-08-10  9:05               ` Luis Henriques

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.