RE: [PATCH v7 09/10] x86, mpx: cleanup unused bound tables
On 2014-07-24, Hansen, Dave wrote: > On 07/23/2014 05:49 PM, Ren, Qiaowei wrote: >> I can check a lot of debug information when one VMA and related >> bounds tables are allocated and freed through adding a lot of >> print() like log into kernel/runtime. Do you think this is enough? > > I thought the entire reason we grabbed a VM_ flag was to make it > possible to figure out without resorting to this. All cleanup work certainly depends on this VM_ flag. In addition, as we discussed, this new VM_ flag can mainly have runtime know how much memory is occupied by MPX. Thanks, Qiaowei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 09/10] x86, mpx: cleanup unused bound tables
On 07/23/2014 05:49 PM, Ren, Qiaowei wrote: > I can check a lot of debug information when one VMA and related > bounds tables are allocated and freed through adding a lot of print() > like log into kernel/runtime. Do you think this is enough? I thought the entire reason we grabbed a VM_ flag was to make it possible to figure out without resorting to this. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v7 09/10] x86, mpx: cleanup unused bound tables
On 2014-07-24, Hansen, Dave wrote: > On 07/20/2014 10:38 PM, Qiaowei Ren wrote: >> Since the kernel allocated those tables on-demand without userspace >> knowledge, it is also responsible for freeing them when the >> associated mappings go away. >> >> Here, the solution for this issue is to hook do_munmap() to check >> whether one process is MPX enabled. If yes, those bounds tables >> covered in the virtual address region which is being unmapped will >> be freed > also. > > This is the part of the code that I'm the most concerned about. > > Could you elaborate on how you've tested this to make sure it works OK? I can check a lot of debug information when one VMA and related bounds tables are allocated and freed through adding a lot of print() like log into kernel/runtime. Do you think this is enough? Thanks, Qiaowei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 09/10] x86, mpx: cleanup unused bound tables
On 07/20/2014 10:38 PM, Qiaowei Ren wrote: > Since the kernel allocated those tables on-demand without userspace > knowledge, it is also responsible for freeing them when the associated > mappings go away. > > Here, the solution for this issue is to hook do_munmap() to check > whether one process is MPX enabled. If yes, those bounds tables covered > in the virtual address region which is being unmapped will be freed also. This is the part of the code that I'm the most concerned about. Could you elaborate on how you've tested this to make sure it works OK? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 09/10] x86, mpx: cleanup unused bound tables
On 07/20/2014 10:38 PM, Qiaowei Ren wrote: Since the kernel allocated those tables on-demand without userspace knowledge, it is also responsible for freeing them when the associated mappings go away. Here, the solution for this issue is to hook do_munmap() to check whether one process is MPX enabled. If yes, those bounds tables covered in the virtual address region which is being unmapped will be freed also. This is the part of the code that I'm the most concerned about. Could you elaborate on how you've tested this to make sure it works OK? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v7 09/10] x86, mpx: cleanup unused bound tables
On 2014-07-24, Hansen, Dave wrote: On 07/20/2014 10:38 PM, Qiaowei Ren wrote: Since the kernel allocated those tables on-demand without userspace knowledge, it is also responsible for freeing them when the associated mappings go away. Here, the solution for this issue is to hook do_munmap() to check whether one process is MPX enabled. If yes, those bounds tables covered in the virtual address region which is being unmapped will be freed also. This is the part of the code that I'm the most concerned about. Could you elaborate on how you've tested this to make sure it works OK? I can check a lot of debug information when one VMA and related bounds tables are allocated and freed through adding a lot of print() like log into kernel/runtime. Do you think this is enough? Thanks, Qiaowei -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v7 09/10] x86, mpx: cleanup unused bound tables
On 07/23/2014 05:49 PM, Ren, Qiaowei wrote: I can check a lot of debug information when one VMA and related bounds tables are allocated and freed through adding a lot of print() like log into kernel/runtime. Do you think this is enough? I thought the entire reason we grabbed a VM_ flag was to make it possible to figure out without resorting to this. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v7 09/10] x86, mpx: cleanup unused bound tables
On 2014-07-24, Hansen, Dave wrote: On 07/23/2014 05:49 PM, Ren, Qiaowei wrote: I can check a lot of debug information when one VMA and related bounds tables are allocated and freed through adding a lot of print() like log into kernel/runtime. Do you think this is enough? I thought the entire reason we grabbed a VM_ flag was to make it possible to figure out without resorting to this. All cleanup work certainly depends on this VM_ flag. In addition, as we discussed, this new VM_ flag can mainly have runtime know how much memory is occupied by MPX. Thanks, Qiaowei -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH v7 09/10] x86, mpx: cleanup unused bound tables
> -Original Message-
> From: owner-linux...@kvack.org [mailto:owner-linux...@kvack.org] On
> Behalf Of Qiaowei Ren
> Sent: Monday, July 21, 2014 1:39 PM
> To: H. Peter Anvin; Thomas Gleixner; Ingo Molnar; Hansen, Dave
> Cc: x...@kernel.org; linux-kernel@vger.kernel.org; linux...@kvack.org; Ren,
> Qiaowei
> Subject: [PATCH v7 09/10] x86, mpx: cleanup unused bound tables
>
> Since the kernel allocated those tables on-demand without userspace
> knowledge, it is also responsible for freeing them when the associated
> mappings go away.
>
> Here, the solution for this issue is to hook do_munmap() to check whether one
> process is MPX enabled. If yes, those bounds tables covered in the virtual
> address region which is being unmapped will be freed also.
>
> Signed-off-by: Qiaowei Ren
> ---
> arch/x86/include/asm/mmu_context.h | 16 +++
> arch/x86/include/asm/mpx.h |9 ++
> arch/x86/mm/mpx.c | 181
>
> include/asm-generic/mmu_context.h |6 +
> mm/mmap.c |2 +
> 5 files changed, 214 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/include/asm/mmu_context.h
> b/arch/x86/include/asm/mmu_context.h
> index be12c53..af70d4f 100644
> --- a/arch/x86/include/asm/mmu_context.h
> +++ b/arch/x86/include/asm/mmu_context.h
> @@ -6,6 +6,7 @@
> #include
> #include
> #include
> +#include
> #ifndef CONFIG_PARAVIRT
> #include
>
> @@ -96,4 +97,19 @@ do { \
> } while (0)
> #endif
>
> +static inline void arch_unmap(struct mm_struct *mm,
> + struct vm_area_struct *vma,
> + unsigned long start, unsigned long end) { #ifdef
> CONFIG_X86_INTEL_MPX
"#indef" new line
> + /*
> + * Check whether this vma comes from MPX-enabled application.
> + * If so, release this vma related bound tables.
> + */
> + if (mm->bd_addr && !(vma->vm_flags & VM_MPX))
> + mpx_unmap(mm, start, end);
> +
> +#endif
> +}
> +
> #endif /* _ASM_X86_MMU_CONTEXT_H */
> diff --git a/arch/x86/include/asm/mpx.h b/arch/x86/include/asm/mpx.h index
> 6cb0853..e848a74 100644
> --- a/arch/x86/include/asm/mpx.h
> +++ b/arch/x86/include/asm/mpx.h
> @@ -42,6 +42,13 @@
> #define MPX_BD_SIZE_BYTES
> (1UL<<(MPX_BD_ENTRY_OFFSET+MPX_BD_ENTRY_SHIFT))
> #define MPX_BT_SIZE_BYTES
> (1UL<<(MPX_BT_ENTRY_OFFSET+MPX_BT_ENTRY_SHIFT))
>
> +#define MPX_BD_ENTRY_MASK((1< +#define MPX_BT_ENTRY_MASK((1< +#define MPX_GET_BD_ENTRY_OFFSET(addr)
> addr)>>(MPX_BT_ENTRY_OFFSET+ \
> + MPX_IGN_BITS)) & MPX_BD_ENTRY_MASK) <<
> MPX_BD_ENTRY_SHIFT)
> +#define MPX_GET_BT_ENTRY_OFFSET(addr)addr)>>MPX_IGN_BITS) & \
> + MPX_BT_ENTRY_MASK) << MPX_BT_ENTRY_SHIFT)
> +
> #define MPX_BNDSTA_ERROR_CODE0x3
> #define MPX_BNDCFG_ENABLE_FLAG 0x1
> #define MPX_BD_ENTRY_VALID_FLAG 0x1
> @@ -63,6 +70,8 @@ struct mpx_insn {
> #define MAX_MPX_INSN_SIZE15
>
> unsigned long mpx_mmap(unsigned long len);
> +void mpx_unmap(struct mm_struct *mm,
> + unsigned long start, unsigned long end);
>
> #ifdef CONFIG_X86_INTEL_MPX
> int do_mpx_bt_fault(struct xsave_struct *xsave_buf); diff --git
> a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c index e1b28e6..d29ec9c 100644
> --- a/arch/x86/mm/mpx.c
> +++ b/arch/x86/mm/mpx.c
> @@ -2,6 +2,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> static const char *mpx_mapping_name(struct vm_area_struct *vma) @@
> -77,3 +78,183 @@ out:
> up_write(>mmap_sem);
> return ret;
> }
> +
> +/*
> + * Get the base of bounds tables pointed by specific bounds
> + * directory entry.
> + */
> +static int get_bt_addr(long __user *bd_entry, unsigned long *bt_addr,
> + unsigned int *valid)
> +{
> + if (get_user(*bt_addr, bd_entry))
> + return -EFAULT;
> +
> + *valid = *bt_addr & MPX_BD_ENTRY_VALID_FLAG;
> + *bt_addr &= MPX_BT_ADDR_MASK;
> +
> + /*
> + * If this bounds directory entry is nonzero, and meanwhile
> + * the valid bit is zero, one SIGSEGV will be produced due to
> + * this unexpected situation.
> + */
> + if (!(*valid) && *bt_addr)
> + force_sig(SIGSEGV, current);
> +
> + return 0;
> +}
> +
> +/*
> + * Free the backing physical pages of bounds table 'bt_addr'.
> + * Assume start...end is within that bounds table.
> + */
> +static void zap_bt_entries(struct mm_struct *mm, unsigned long bt_addr,
> + unsigned long start, unsigned long end) {
> + struct vm_area_struct *vma;
> +
> + /* Find the vma which overlaps this bounds table */
> + vma = find_vma(mm, bt_addr);
> + if (!vma || vma->vm_start > bt_addr ||
> + vma->vm_end < bt_addr+MPX_BT_SIZE_BYTES)
> + return;
> +
> + zap_page_range(vma, start, end, NULL); }
> +
> +static void unmap_single_bt(struct mm_struct *mm, long __user *bd_entry,
> +
RE: [PATCH v7 09/10] x86, mpx: cleanup unused bound tables
-Original Message-
From: owner-linux...@kvack.org [mailto:owner-linux...@kvack.org] On
Behalf Of Qiaowei Ren
Sent: Monday, July 21, 2014 1:39 PM
To: H. Peter Anvin; Thomas Gleixner; Ingo Molnar; Hansen, Dave
Cc: x...@kernel.org; linux-kernel@vger.kernel.org; linux...@kvack.org; Ren,
Qiaowei
Subject: [PATCH v7 09/10] x86, mpx: cleanup unused bound tables
Since the kernel allocated those tables on-demand without userspace
knowledge, it is also responsible for freeing them when the associated
mappings go away.
Here, the solution for this issue is to hook do_munmap() to check whether one
process is MPX enabled. If yes, those bounds tables covered in the virtual
address region which is being unmapped will be freed also.
Signed-off-by: Qiaowei Ren qiaowei@intel.com
---
arch/x86/include/asm/mmu_context.h | 16 +++
arch/x86/include/asm/mpx.h |9 ++
arch/x86/mm/mpx.c | 181
include/asm-generic/mmu_context.h |6 +
mm/mmap.c |2 +
5 files changed, 214 insertions(+), 0 deletions(-)
diff --git a/arch/x86/include/asm/mmu_context.h
b/arch/x86/include/asm/mmu_context.h
index be12c53..af70d4f 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -6,6 +6,7 @@
#include asm/pgalloc.h
#include asm/tlbflush.h
#include asm/paravirt.h
+#include asm/mpx.h
#ifndef CONFIG_PARAVIRT
#include asm-generic/mm_hooks.h
@@ -96,4 +97,19 @@ do { \
} while (0)
#endif
+static inline void arch_unmap(struct mm_struct *mm,
+ struct vm_area_struct *vma,
+ unsigned long start, unsigned long end) { #ifdef
CONFIG_X86_INTEL_MPX
#indef new line
+ /*
+ * Check whether this vma comes from MPX-enabled application.
+ * If so, release this vma related bound tables.
+ */
+ if (mm-bd_addr !(vma-vm_flags VM_MPX))
+ mpx_unmap(mm, start, end);
+
+#endif
+}
+
#endif /* _ASM_X86_MMU_CONTEXT_H */
diff --git a/arch/x86/include/asm/mpx.h b/arch/x86/include/asm/mpx.h index
6cb0853..e848a74 100644
--- a/arch/x86/include/asm/mpx.h
+++ b/arch/x86/include/asm/mpx.h
@@ -42,6 +42,13 @@
#define MPX_BD_SIZE_BYTES
(1UL(MPX_BD_ENTRY_OFFSET+MPX_BD_ENTRY_SHIFT))
#define MPX_BT_SIZE_BYTES
(1UL(MPX_BT_ENTRY_OFFSET+MPX_BT_ENTRY_SHIFT))
+#define MPX_BD_ENTRY_MASK((1MPX_BD_ENTRY_OFFSET)-1)
+#define MPX_BT_ENTRY_MASK((1MPX_BT_ENTRY_OFFSET)-1)
+#define MPX_GET_BD_ENTRY_OFFSET(addr)
addr)(MPX_BT_ENTRY_OFFSET+ \
+ MPX_IGN_BITS)) MPX_BD_ENTRY_MASK)
MPX_BD_ENTRY_SHIFT)
+#define MPX_GET_BT_ENTRY_OFFSET(addr)addr)MPX_IGN_BITS) \
+ MPX_BT_ENTRY_MASK) MPX_BT_ENTRY_SHIFT)
+
#define MPX_BNDSTA_ERROR_CODE0x3
#define MPX_BNDCFG_ENABLE_FLAG 0x1
#define MPX_BD_ENTRY_VALID_FLAG 0x1
@@ -63,6 +70,8 @@ struct mpx_insn {
#define MAX_MPX_INSN_SIZE15
unsigned long mpx_mmap(unsigned long len);
+void mpx_unmap(struct mm_struct *mm,
+ unsigned long start, unsigned long end);
#ifdef CONFIG_X86_INTEL_MPX
int do_mpx_bt_fault(struct xsave_struct *xsave_buf); diff --git
a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c index e1b28e6..d29ec9c 100644
--- a/arch/x86/mm/mpx.c
+++ b/arch/x86/mm/mpx.c
@@ -2,6 +2,7 @@
#include linux/syscalls.h
#include asm/mpx.h
#include asm/mman.h
+#include asm/mmu_context.h
#include linux/sched/sysctl.h
static const char *mpx_mapping_name(struct vm_area_struct *vma) @@
-77,3 +78,183 @@ out:
up_write(mm-mmap_sem);
return ret;
}
+
+/*
+ * Get the base of bounds tables pointed by specific bounds
+ * directory entry.
+ */
+static int get_bt_addr(long __user *bd_entry, unsigned long *bt_addr,
+ unsigned int *valid)
+{
+ if (get_user(*bt_addr, bd_entry))
+ return -EFAULT;
+
+ *valid = *bt_addr MPX_BD_ENTRY_VALID_FLAG;
+ *bt_addr = MPX_BT_ADDR_MASK;
+
+ /*
+ * If this bounds directory entry is nonzero, and meanwhile
+ * the valid bit is zero, one SIGSEGV will be produced due to
+ * this unexpected situation.
+ */
+ if (!(*valid) *bt_addr)
+ force_sig(SIGSEGV, current);
+
+ return 0;
+}
+
+/*
+ * Free the backing physical pages of bounds table 'bt_addr'.
+ * Assume start...end is within that bounds table.
+ */
+static void zap_bt_entries(struct mm_struct *mm, unsigned long bt_addr,
+ unsigned long start, unsigned long end) {
+ struct vm_area_struct *vma;
+
+ /* Find the vma which overlaps this bounds table */
+ vma = find_vma(mm, bt_addr);
+ if (!vma || vma-vm_start bt_addr ||
+ vma-vm_end bt_addr+MPX_BT_SIZE_BYTES)
+ return;
+
+ zap_page_range(vma, start, end, NULL); }
+
+static void unmap_single_bt(struct mm_struct
