Re: Kernel bug with MPX?

[Dave Hansen <dave.hansen@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1175 bytes --]

For those just joining the thread now, here's the background:

> https://lkml.kernel.org/r/alpine.LSU.2.20.1903060944550.7898@zhemvz.fhfr.qr

Turning on a bunch of kernel debugging found the culprit:

>         /*
>          * mpx unmap needs to be called with mmap_sem held for write.
>          * It is safe to call it before unmap_region().
>          */
>         arch_unmap(mm, vma, start, end);
> 
>         if (downgrade)
>                 downgrade_write(&mm->mmap_sem);
> 
>         unmap_region(mm, vma, prev, start, end);

arch_unmap() can, in some cases, free 'prev'.  unmap_region() uses
'prev' to calculate the page table ranges that it frees.  It's probably
working on incorrect or garbage ranges at times.

I have some patches to really fix this by pre-calculating the
page-table-free ranges before arch_unmap().  They're not *too* bad, but
they do involve mucking with mm/mmap.c a bit to pass some new parameters
around.

The other option would be to just use this opportunity to start removing
MPX and apply the attached patch so this is no longer able to be triggered.

I'm inclined to opt for the patch to addle MPX rather than trying to fix
it for real.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mpx-remove-apis.patch --]
[-- Type: text/x-patch; name="mpx-remove-apis.patch", Size: 2234 bytes --]


From: Dave Hansen <dave.hansen@linux.intel.com>

MPX is being removed from the kernel due to a lack of support
in the toolchain going forward (gcc).

The first thing we need to do is remove the userspace-visible
ABIs so that applications will stop using it.  The most visible
one are the enable/disable prctl()s.  Remove them first.

This is the most minimal and least invasive patch needed to
start removing MPX.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
---

 b/include/uapi/linux/prctl.h |    2 +-
 b/kernel/sys.c               |   16 ++--------------
 2 files changed, 3 insertions(+), 15 deletions(-)

diff -puN include/uapi/linux/prctl.h~mpx-remove-apis include/uapi/linux/prctl.h
--- a/include/uapi/linux/prctl.h~mpx-remove-apis	2019-01-04 14:40:06.853514089 -0800
+++ b/include/uapi/linux/prctl.h	2019-01-04 14:40:06.860514089 -0800
@@ -181,7 +181,7 @@ struct prctl_mm_map {
 #define PR_GET_THP_DISABLE	42
 
 /*
- * Tell the kernel to start/stop helping userspace manage bounds tables.
+ * No longer implemented, but left here to ensure the numbers stay reserved:
  */
 #define PR_MPX_ENABLE_MANAGEMENT  43
 #define PR_MPX_DISABLE_MANAGEMENT 44
diff -puN kernel/sys.c~mpx-remove-apis kernel/sys.c
--- a/kernel/sys.c~mpx-remove-apis	2019-01-04 14:40:06.857514089 -0800
+++ b/kernel/sys.c	2019-01-04 14:40:06.860514089 -0800
@@ -103,12 +103,6 @@
 #ifndef SET_TSC_CTL
 # define SET_TSC_CTL(a)		(-EINVAL)
 #endif
-#ifndef MPX_ENABLE_MANAGEMENT
-# define MPX_ENABLE_MANAGEMENT()	(-EINVAL)
-#endif
-#ifndef MPX_DISABLE_MANAGEMENT
-# define MPX_DISABLE_MANAGEMENT()	(-EINVAL)
-#endif
 #ifndef GET_FP_MODE
 # define GET_FP_MODE(a)		(-EINVAL)
 #endif
@@ -2448,15 +2442,9 @@ SYSCALL_DEFINE5(prctl, int, option, unsi
 		up_write(&me->mm->mmap_sem);
 		break;
 	case PR_MPX_ENABLE_MANAGEMENT:
-		if (arg2 || arg3 || arg4 || arg5)
-			return -EINVAL;
-		error = MPX_ENABLE_MANAGEMENT();
-		break;
 	case PR_MPX_DISABLE_MANAGEMENT:
-		if (arg2 || arg3 || arg4 || arg5)
-			return -EINVAL;
-		error = MPX_DISABLE_MANAGEMENT();
-		break;
+		/* No longer implemented: */
+		return -EINVAL;
 	case PR_SET_FP_MODE:
 		error = SET_FP_MODE(me, arg2);
 		break;
_