linux-kernel - Re: [PATCH] [4.2 fix] x86, mpx: do not set ->vm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20150803103429.0A7D91F6@black.fi.intel.com>
Date:	Mon,  3 Aug 2015 13:34:28 +0300 (EEST)
From:	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
To:	Greg KH <greg@...ah.com>
Cc:	Dave Hansen <dave@...1.net>, kirill.shutemov@...ux.intel.com,
	dave.hansen@...ux.intel.com, luto@...capital.net,
	tglx@...utronix.de, x86@...nel.org, stable@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] [4.2 fix] x86, mpx: do not set ->vm_ops on mpx VMAs

Greg KH wrote:
> On Mon, Jul 20, 2015 at 02:29:58PM -0700, Dave Hansen wrote:
> > 
> > (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.
> 
> 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.

>From 62d8093f0bfd79eff2c87e8924cb9276116aaabe Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
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 <kirill.shutemov@...ux.intel.com>
Signed-off-by: Dave Hansen <dave.hansen@...ux.intel.com>
Cc: <stable@...r.kernel.org> # Fixes: 6b7339f4 (mm: avoid setting up anonymous pages into file mapping)
Cc: Andy Lutomirski <luto@...capital.net>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: dave@...1.net
Link: http://lkml.kernel.org/r/20150720212958.305CC3E9@viggo.jf.intel.com
Signed-off-by: Ingo Molnar <mingo@...nel.org>
---
 arch/x86/mm/mmap.c |  7 +++++++
 arch/x86/mm/mpx.c  | 20 +-------------------
 2 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index 9d518d693b4b..844b06d67df4 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -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;
+}
diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c
index c439ec478216..4d1c11c07fe1 100644
--- a/arch/x86/mm/mpx.c
+++ b/arch/x86/mm/mpx.c
@@ -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;
-- 
2.4.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/