[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170111142904.GD4895@node.shutemov.name>
Date: Wed, 11 Jan 2017 17:29:04 +0300
From: "Kirill A. Shutemov" <kirill@...temov.name>
To: Dave Hansen <dave.hansen@...el.com>
Cc: Andy Lutomirski <luto@...capital.net>,
"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Andrew Morton <akpm@...ux-foundation.org>,
X86 ML <x86@...nel.org>, Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Arnd Bergmann <arnd@...db.de>,
"H. Peter Anvin" <hpa@...or.com>, Andi Kleen <ak@...ux.intel.com>,
linux-arch <linux-arch@...r.kernel.org>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Linux API <linux-api@...r.kernel.org>
Subject: Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR
On Thu, Jan 05, 2017 at 12:49:44PM -0800, Dave Hansen wrote:
> On 01/05/2017 12:14 PM, Andy Lutomirski wrote:
> >> I'm not sure I'm comfortable with this. Do other rlimit changes cause
> >> silent data corruption? I'm pretty sure doing this to MPX would.
> >>
> > What actually goes wrong in this case? That is, what combination of
> > MPX setup of subsequent allocations will cause a problem, and is the
> > problem worse than just a segfault? IMO it would be really nice to
> > keep the messy case confined to MPX.
>
> The MPX bounds tables are indexed by virtual address. They need to grow
> if the virtual address space grows. There's an MSR that controls
> whether we use the 48-bit or 57-bit layout. It basically decides
> whether we need a 2GB (48-bit) or 1TB (57-bit) bounds directory.
>
> The question is what we do with legacy MPX applications. We obviously
> can't let them just allocate a 2GB table and then go let the hardware
> pretend it's 1TB in size. We also can't hand the hardware using a 2GB
> table an address >48-bits.
>
> Ideally, I'd like to make sure that legacy MPX can't be enabled if this
> RLIMIT is set over 48-bits (really 47). I'd also like to make sure that
> legacy MPX is active, that the RLIMIT can't be raised because all hell
> will break loose when the new addresses show up.
I think we can do this. See the patch below.
Basically, we refuse to enable MPX and issue warning in dmesg if there's
anything mapped above 47-bits. Once MPX is enabled, mmap_max_addr() cannot
be higher than 47-bits too.
Function call from mmap_max_addr() is unfortunate, but I don't see a
way around.
As we add support of MAWA it will get somewhat more complex, but general
idea should be the same.
Build-tested only.
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 07cc4f27ca41..f97b149145f8 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1742,7 +1742,6 @@ config X86_SMAP
config X86_INTEL_MPX
prompt "Intel MPX (Memory Protection Extensions)"
def_bool n
- depends on !X86_5LEVEL
depends on CPU_SUP_INTEL
---help---
MPX provides hardware features that can be used in
diff --git a/arch/x86/include/asm/mpx.h b/arch/x86/include/asm/mpx.h
index 0b416d4cf73b..ba9005f9bf87 100644
--- a/arch/x86/include/asm/mpx.h
+++ b/arch/x86/include/asm/mpx.h
@@ -56,11 +56,8 @@
#ifdef CONFIG_X86_INTEL_MPX
siginfo_t *mpx_generate_siginfo(struct pt_regs *regs);
+int kernel_managing_mpx_tables(struct mm_struct *mm);
int mpx_handle_bd_fault(void);
-static inline int kernel_managing_mpx_tables(struct mm_struct *mm)
-{
- return (mm->context.bd_addr != MPX_INVALID_BOUNDS_DIR);
-}
static inline void mpx_mm_init(struct mm_struct *mm)
{
/*
@@ -80,10 +77,6 @@ static inline int mpx_handle_bd_fault(void)
{
return -EINVAL;
}
-static inline int kernel_managing_mpx_tables(struct mm_struct *mm)
-{
- return 0;
-}
static inline void mpx_mm_init(struct mm_struct *mm)
{
}
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index e02917126859..589610a4f099 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -869,6 +869,7 @@ extern int set_tsc_mode(unsigned int val);
#ifdef CONFIG_X86_INTEL_MPX
extern int mpx_enable_management(void);
extern int mpx_disable_management(void);
+extern int kernel_managing_mpx_tables(struct mm_struct *mm);
#else
static inline int mpx_enable_management(void)
{
@@ -878,8 +879,22 @@ static inline int mpx_disable_management(void)
{
return -EINVAL;
}
+static inline int kernel_managing_mpx_tables(struct mm_struct *mm)
+{
+ return 0;
+}
#endif /* CONFIG_X86_INTEL_MPX */
+#define mmap_max_addr() \
+({ \
+ unsigned long max_addr = min(TASK_SIZE, rlimit(RLIMIT_VADDR)); \
+ /* At the moment, MPX cannot handle addresses above 47-bits */ \
+ if (max_addr > USER_VADDR_LIM && \
+ kernel_managing_mpx_tables(current->mm)) \
+ max_addr = USER_VADDR_LIM; \
+ max_addr; \
+})
+
extern u16 amd_get_nb_id(int cpu);
extern u32 amd_get_nodes_per_socket(void);
diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c
index 324e5713d386..04fa386a165a 100644
--- a/arch/x86/mm/mpx.c
+++ b/arch/x86/mm/mpx.c
@@ -354,10 +354,22 @@ int mpx_enable_management(void)
*/
bd_base = mpx_get_bounds_dir();
down_write(&mm->mmap_sem);
+
+ /*
+ * MPX doesn't support addresses above 47-bits yes.
+ * Make sure nothing is mapped there before enabling.
+ */
+ if (find_vma(mm, 1UL << 47)) {
+ pr_warn("%s (%d): MPX cannot handle addresses above 47-bits. "
+ "Disabling.", current->comm, current->pid);
+ ret = -ENXIO;
+ goto out;
+ }
+
mm->context.bd_addr = bd_base;
if (mm->context.bd_addr == MPX_INVALID_BOUNDS_DIR)
ret = -ENXIO;
-
+out:
up_write(&mm->mmap_sem);
return ret;
}
@@ -516,6 +528,11 @@ static int do_mpx_bt_fault(void)
return allocate_bt(mm, (long __user *)bd_entry);
}
+int kernel_managing_mpx_tables(struct mm_struct *mm)
+{
+ return (mm->context.bd_addr != MPX_INVALID_BOUNDS_DIR);
+}
+
int mpx_handle_bd_fault(void)
{
/*
diff --git a/include/linux/sched.h b/include/linux/sched.h
index f0f23afe0838..d463b800d8ce 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -3661,9 +3661,12 @@ void cpufreq_add_update_util_hook(int cpu, struct update_util_data *data,
void cpufreq_remove_update_util_hook(int cpu);
#endif /* CONFIG_CPU_FREQ */
+#ifndef mmap_max_addr
+#define mmap_max_addr mmap_max_addr
static inline unsigned long mmap_max_addr(void)
{
return min(TASK_SIZE, rlimit(RLIMIT_VADDR));
}
+#endif
#endif
--
Kirill A. Shutemov
Powered by blists - more mailing lists