linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv4 0/4] WX checking for arm64
@ 2016-10-27 16:27 Laura Abbott
  2016-10-27 16:27 ` [PATCHv4 1/4] arm64: dump: Make ptdump debugfs a separate option Laura Abbott
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Laura Abbott @ 2016-10-27 16:27 UTC (permalink / raw)
  To: AKASHI Takahiro, Mark Rutland, Ard Biesheuvel, David Brown,
	Will Deacon, Catalin Marinas
  Cc: Laura Abbott, linux-arm-kernel, linux-kernel, Kees Cook,
	kernel-hardening, Matt Fleming, linux-efi

Hi,

This is v4 of the implementation to check for writable and executable pages on
arm64. This version contains a review from Ard and makes the UXN page count
a separate variable. Overall, minor changes.

Thanks,
Laura

Laura Abbott (4):
  arm64: dump: Make ptdump debugfs a separate option
  arm64: dump: Make the page table dumping seq_file optional
  arm64: dump: Remove max_addr
  arm64: dump: Add checking for writable and exectuable pages

 arch/arm64/Kconfig.debug           |  35 ++++++++++++-
 arch/arm64/include/asm/ptdump.h    |  22 +++++---
 arch/arm64/mm/Makefile             |   3 +-
 arch/arm64/mm/dump.c               | 105 +++++++++++++++++++++++++++----------
 arch/arm64/mm/mmu.c                |   2 +
 arch/arm64/mm/ptdump_debugfs.c     |  31 +++++++++++
 drivers/firmware/efi/arm-runtime.c |   4 +-
 7 files changed, 164 insertions(+), 38 deletions(-)
 create mode 100644 arch/arm64/mm/ptdump_debugfs.c

-- 
2.7.4

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

* [PATCHv4 1/4] arm64: dump: Make ptdump debugfs a separate option
  2016-10-27 16:27 [PATCHv4 0/4] WX checking for arm64 Laura Abbott
@ 2016-10-27 16:27 ` Laura Abbott
  2016-10-27 16:27 ` [PATCHv4 2/4] arm64: dump: Make the page table dumping seq_file optional Laura Abbott
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Laura Abbott @ 2016-10-27 16:27 UTC (permalink / raw)
  To: AKASHI Takahiro, Mark Rutland, Ard Biesheuvel, David Brown,
	Will Deacon, Catalin Marinas
  Cc: Laura Abbott, linux-arm-kernel, linux-kernel, Kees Cook,
	kernel-hardening, Matt Fleming, linux-efi


ptdump_register currently initializes a set of page table information and
registers debugfs. There are uses for the ptdump option without wanting the
debugfs options. Split this out to make it a separate option.

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v4: Reviewed-by from Ard
---
 arch/arm64/Kconfig.debug           |  6 +++++-
 arch/arm64/include/asm/ptdump.h    | 15 +++++++++------
 arch/arm64/mm/Makefile             |  3 ++-
 arch/arm64/mm/dump.c               | 26 +++++---------------------
 arch/arm64/mm/ptdump_debugfs.c     | 31 +++++++++++++++++++++++++++++++
 drivers/firmware/efi/arm-runtime.c |  4 ++--
 6 files changed, 54 insertions(+), 31 deletions(-)
 create mode 100644 arch/arm64/mm/ptdump_debugfs.c

diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
index b661fe7..21a5b74 100644
--- a/arch/arm64/Kconfig.debug
+++ b/arch/arm64/Kconfig.debug
@@ -2,9 +2,13 @@ menu "Kernel hacking"
 
 source "lib/Kconfig.debug"
 
-config ARM64_PTDUMP
+config ARM64_PTDUMP_CORE
+	def_bool n
+
+config ARM64_PTDUMP_DEBUGFS
 	bool "Export kernel pagetable layout to userspace via debugfs"
 	depends on DEBUG_KERNEL
+	select ARM64_PTDUMP_CORE
 	select DEBUG_FS
         help
 	  Say Y here if you want to show the kernel pagetable layout in a
diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h
index 07b8ed0..16335da 100644
--- a/arch/arm64/include/asm/ptdump.h
+++ b/arch/arm64/include/asm/ptdump.h
@@ -16,9 +16,10 @@
 #ifndef __ASM_PTDUMP_H
 #define __ASM_PTDUMP_H
 
-#ifdef CONFIG_ARM64_PTDUMP
+#ifdef CONFIG_ARM64_PTDUMP_CORE
 
 #include <linux/mm_types.h>
+#include <linux/seq_file.h>
 
 struct addr_marker {
 	unsigned long start_address;
@@ -32,13 +33,15 @@ struct ptdump_info {
 	unsigned long			max_addr;
 };
 
-int ptdump_register(struct ptdump_info *info, const char *name);
-
+void ptdump_walk_pgd(struct seq_file *s, struct ptdump_info *info);
+#ifdef CONFIG_ARM64_PTDUMP_DEBUGFS
+int ptdump_debugfs_register(struct ptdump_info *info, const char *name);
 #else
-static inline int ptdump_register(struct ptdump_info *info, const char *name)
+static inline int ptdump_debugfs_register(struct ptdump_info *info,
+					const char *name)
 {
 	return 0;
 }
-#endif /* CONFIG_ARM64_PTDUMP */
-
+#endif
+#endif /* CONFIG_ARM64_PTDUMP_CORE */
 #endif /* __ASM_PTDUMP_H */
diff --git a/arch/arm64/mm/Makefile b/arch/arm64/mm/Makefile
index 54bb209..e703fb9 100644
--- a/arch/arm64/mm/Makefile
+++ b/arch/arm64/mm/Makefile
@@ -3,7 +3,8 @@ obj-y				:= dma-mapping.o extable.o fault.o init.o \
 				   ioremap.o mmap.o pgd.o mmu.o \
 				   context.o proc.o pageattr.o
 obj-$(CONFIG_HUGETLB_PAGE)	+= hugetlbpage.o
-obj-$(CONFIG_ARM64_PTDUMP)	+= dump.o
+obj-$(CONFIG_ARM64_PTDUMP_CORE)	+= dump.o
+obj-$(CONFIG_ARM64_PTDUMP_DEBUGFS)	+= ptdump_debugfs.o
 obj-$(CONFIG_NUMA)		+= numa.o
 
 obj-$(CONFIG_KASAN)		+= kasan_init.o
diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
index 9c3e75d..f0f0be7 100644
--- a/arch/arm64/mm/dump.c
+++ b/arch/arm64/mm/dump.c
@@ -304,9 +304,8 @@ static void walk_pgd(struct pg_state *st, struct mm_struct *mm,
 	}
 }
 
-static int ptdump_show(struct seq_file *m, void *v)
+void ptdump_walk_pgd(struct seq_file *m, struct ptdump_info *info)
 {
-	struct ptdump_info *info = m->private;
 	struct pg_state st = {
 		.seq = m,
 		.marker = info->markers,
@@ -315,33 +314,16 @@ static int ptdump_show(struct seq_file *m, void *v)
 	walk_pgd(&st, info->mm, info->base_addr);
 
 	note_page(&st, 0, 0, 0);
-	return 0;
 }
 
-static int ptdump_open(struct inode *inode, struct file *file)
+static void ptdump_initialize(void)
 {
-	return single_open(file, ptdump_show, inode->i_private);
-}
-
-static const struct file_operations ptdump_fops = {
-	.open		= ptdump_open,
-	.read		= seq_read,
-	.llseek		= seq_lseek,
-	.release	= single_release,
-};
-
-int ptdump_register(struct ptdump_info *info, const char *name)
-{
-	struct dentry *pe;
 	unsigned i, j;
 
 	for (i = 0; i < ARRAY_SIZE(pg_level); i++)
 		if (pg_level[i].bits)
 			for (j = 0; j < pg_level[i].num; j++)
 				pg_level[i].mask |= pg_level[i].bits[j].mask;
-
-	pe = debugfs_create_file(name, 0400, NULL, info, &ptdump_fops);
-	return pe ? 0 : -ENOMEM;
 }
 
 static struct ptdump_info kernel_ptdump_info = {
@@ -352,6 +334,8 @@ static struct ptdump_info kernel_ptdump_info = {
 
 static int ptdump_init(void)
 {
-	return ptdump_register(&kernel_ptdump_info, "kernel_page_tables");
+	ptdump_initialize();
+	return ptdump_debugfs_register(&kernel_ptdump_info,
+					"kernel_page_tables");
 }
 device_initcall(ptdump_init);
diff --git a/arch/arm64/mm/ptdump_debugfs.c b/arch/arm64/mm/ptdump_debugfs.c
new file mode 100644
index 0000000..eee4d86
--- /dev/null
+++ b/arch/arm64/mm/ptdump_debugfs.c
@@ -0,0 +1,31 @@
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+
+#include <asm/ptdump.h>
+
+static int ptdump_show(struct seq_file *m, void *v)
+{
+	struct ptdump_info *info = m->private;
+	ptdump_walk_pgd(m, info);
+	return 0;
+}
+
+static int ptdump_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, ptdump_show, inode->i_private);
+}
+
+static const struct file_operations ptdump_fops = {
+	.open		= ptdump_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+int ptdump_debugfs_register(struct ptdump_info *info, const char *name)
+{
+	struct dentry *pe;
+	pe = debugfs_create_file(name, 0400, NULL, info, &ptdump_fops);
+	return pe ? 0 : -ENOMEM;
+
+}
diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
index 7c75a8d..349dc3e 100644
--- a/drivers/firmware/efi/arm-runtime.c
+++ b/drivers/firmware/efi/arm-runtime.c
@@ -39,7 +39,7 @@ static struct mm_struct efi_mm = {
 	.mmlist			= LIST_HEAD_INIT(efi_mm.mmlist),
 };
 
-#ifdef CONFIG_ARM64_PTDUMP
+#ifdef CONFIG_ARM64_PTDUMP_DEBUGFS
 #include <asm/ptdump.h>
 
 static struct ptdump_info efi_ptdump_info = {
@@ -53,7 +53,7 @@ static struct ptdump_info efi_ptdump_info = {
 
 static int __init ptdump_init(void)
 {
-	return ptdump_register(&efi_ptdump_info, "efi_page_tables");
+	return ptdump_debugfs_register(&efi_ptdump_info, "efi_page_tables");
 }
 device_initcall(ptdump_init);
 
-- 
2.7.4

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

* [PATCHv4 2/4] arm64: dump: Make the page table dumping seq_file optional
  2016-10-27 16:27 [PATCHv4 0/4] WX checking for arm64 Laura Abbott
  2016-10-27 16:27 ` [PATCHv4 1/4] arm64: dump: Make ptdump debugfs a separate option Laura Abbott
@ 2016-10-27 16:27 ` Laura Abbott
  2016-10-27 16:27 ` [PATCHv4 3/4] arm64: dump: Remove max_addr Laura Abbott
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Laura Abbott @ 2016-10-27 16:27 UTC (permalink / raw)
  To: AKASHI Takahiro, Mark Rutland, Ard Biesheuvel, David Brown,
	Will Deacon, Catalin Marinas
  Cc: Laura Abbott, linux-arm-kernel, linux-kernel, Kees Cook,
	kernel-hardening, Matt Fleming, linux-efi


The page table dumping code always assumes it will be dumping to a
seq_file to userspace. Future code will be taking advantage of
the page table dumping code but will not need the seq_file. Make
the seq_file optional for these cases.

Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v4: No changes
---
 arch/arm64/mm/dump.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
index f0f0be7..bb36649 100644
--- a/arch/arm64/mm/dump.c
+++ b/arch/arm64/mm/dump.c
@@ -50,6 +50,18 @@ static const struct addr_marker address_markers[] = {
 	{ -1,				NULL },
 };
 
+#define pt_dump_seq_printf(m, fmt, args...)	\
+({						\
+	if (m)					\
+		seq_printf(m, fmt, ##args);	\
+})
+
+#define pt_dump_seq_puts(m, fmt)	\
+({					\
+	if (m)				\
+		seq_printf(m, fmt);	\
+})
+
 /*
  * The page dumper groups page table entries of the same type into a single
  * description. It uses pg_state to track the range information while
@@ -186,7 +198,7 @@ static void dump_prot(struct pg_state *st, const struct prot_bits *bits,
 			s = bits->clear;
 
 		if (s)
-			seq_printf(st->seq, " %s", s);
+			pt_dump_seq_printf(st->seq, " %s", s);
 	}
 }
 
@@ -200,14 +212,14 @@ static void note_page(struct pg_state *st, unsigned long addr, unsigned level,
 		st->level = level;
 		st->current_prot = prot;
 		st->start_address = addr;
-		seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
+		pt_dump_seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
 	} else if (prot != st->current_prot || level != st->level ||
 		   addr >= st->marker[1].start_address) {
 		const char *unit = units;
 		unsigned long delta;
 
 		if (st->current_prot) {
-			seq_printf(st->seq, "0x%016lx-0x%016lx   ",
+			pt_dump_seq_printf(st->seq, "0x%016lx-0x%016lx   ",
 				   st->start_address, addr);
 
 			delta = (addr - st->start_address) >> 10;
@@ -215,17 +227,17 @@ static void note_page(struct pg_state *st, unsigned long addr, unsigned level,
 				delta >>= 10;
 				unit++;
 			}
-			seq_printf(st->seq, "%9lu%c %s", delta, *unit,
+			pt_dump_seq_printf(st->seq, "%9lu%c %s", delta, *unit,
 				   pg_level[st->level].name);
 			if (pg_level[st->level].bits)
 				dump_prot(st, pg_level[st->level].bits,
 					  pg_level[st->level].num);
-			seq_puts(st->seq, "\n");
+			pt_dump_seq_puts(st->seq, "\n");
 		}
 
 		if (addr >= st->marker[1].start_address) {
 			st->marker++;
-			seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
+			pt_dump_seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
 		}
 
 		st->start_address = addr;
@@ -235,7 +247,7 @@ static void note_page(struct pg_state *st, unsigned long addr, unsigned level,
 
 	if (addr >= st->marker[1].start_address) {
 		st->marker++;
-		seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
+		pt_dump_seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
 	}
 
 }
-- 
2.7.4

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

* [PATCHv4 3/4] arm64: dump: Remove max_addr
  2016-10-27 16:27 [PATCHv4 0/4] WX checking for arm64 Laura Abbott
  2016-10-27 16:27 ` [PATCHv4 1/4] arm64: dump: Make ptdump debugfs a separate option Laura Abbott
  2016-10-27 16:27 ` [PATCHv4 2/4] arm64: dump: Make the page table dumping seq_file optional Laura Abbott
@ 2016-10-27 16:27 ` Laura Abbott
  2016-10-27 16:27 ` [PATCHv4 4/4] arm64: dump: Add checking for writable and exectuable pages Laura Abbott
  2016-10-30 15:03 ` [PATCHv4 0/4] WX checking for arm64 Catalin Marinas
  4 siblings, 0 replies; 12+ messages in thread
From: Laura Abbott @ 2016-10-27 16:27 UTC (permalink / raw)
  To: AKASHI Takahiro, Mark Rutland, Ard Biesheuvel, David Brown,
	Will Deacon, Catalin Marinas
  Cc: Laura Abbott, linux-arm-kernel, linux-kernel, Kees Cook,
	kernel-hardening, Matt Fleming, linux-efi


max_addr was added as part of struct ptdump_info but has never actually
been used. Remove it.

Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v4: No changes
---
 arch/arm64/include/asm/ptdump.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h
index 16335da..f72ee69 100644
--- a/arch/arm64/include/asm/ptdump.h
+++ b/arch/arm64/include/asm/ptdump.h
@@ -30,7 +30,6 @@ struct ptdump_info {
 	struct mm_struct		*mm;
 	const struct addr_marker	*markers;
 	unsigned long			base_addr;
-	unsigned long			max_addr;
 };
 
 void ptdump_walk_pgd(struct seq_file *s, struct ptdump_info *info);
-- 
2.7.4

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

* [PATCHv4 4/4] arm64: dump: Add checking for writable and exectuable pages
  2016-10-27 16:27 [PATCHv4 0/4] WX checking for arm64 Laura Abbott
                   ` (2 preceding siblings ...)
  2016-10-27 16:27 ` [PATCHv4 3/4] arm64: dump: Remove max_addr Laura Abbott
@ 2016-10-27 16:27 ` Laura Abbott
  2016-10-28 11:52   ` Ard Biesheuvel
  2016-10-30 15:03 ` [PATCHv4 0/4] WX checking for arm64 Catalin Marinas
  4 siblings, 1 reply; 12+ messages in thread
From: Laura Abbott @ 2016-10-27 16:27 UTC (permalink / raw)
  To: AKASHI Takahiro, Mark Rutland, Ard Biesheuvel, David Brown,
	Will Deacon, Catalin Marinas
  Cc: Laura Abbott, linux-arm-kernel, linux-kernel, Kees Cook,
	kernel-hardening, Matt Fleming, linux-efi


Page mappings with full RWX permissions are a security risk. x86
has an option to walk the page tables and dump any bad pages.
(See e1a58320a38d ("x86/mm: Warn on W^X mappings")). Add a similar
implementation for arm64.

Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v4: Changed pr_info -> pr_warn. Added a separate count variable for uxn to avoid
double counting.
---
 arch/arm64/Kconfig.debug        | 29 ++++++++++++++++++++++
 arch/arm64/include/asm/ptdump.h |  8 +++++++
 arch/arm64/mm/dump.c            | 53 +++++++++++++++++++++++++++++++++++++++++
 arch/arm64/mm/mmu.c             |  2 ++
 4 files changed, 92 insertions(+)

diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
index 21a5b74..d1ebd46 100644
--- a/arch/arm64/Kconfig.debug
+++ b/arch/arm64/Kconfig.debug
@@ -42,6 +42,35 @@ config ARM64_RANDOMIZE_TEXT_OFFSET
 	  of TEXT_OFFSET and platforms must not require a specific
 	  value.
 
+config DEBUG_WX
+	bool "Warn on W+X mappings at boot"
+	select ARM64_PTDUMP_CORE
+	---help---
+	  Generate a warning if any W+X mappings are found at boot.
+
+	  This is useful for discovering cases where the kernel is leaving
+	  W+X mappings after applying NX, as such mappings are a security risk.
+	  This check also includes UXN, which should be set on all kernel
+	  mappings.
+
+	  Look for a message in dmesg output like this:
+
+	    arm64/mm: Checked W+X mappings: passed, no W+X pages found.
+
+	  or like this, if the check failed:
+
+	    arm64/mm: Checked W+X mappings: FAILED, <N> W+X pages found.
+
+	  Note that even if the check fails, your kernel is possibly
+	  still fine, as W+X mappings are not a security hole in
+	  themselves, what they do is that they make the exploitation
+	  of other unfixed kernel bugs easier.
+
+	  There is no runtime or memory usage effect of this option
+	  once the kernel has booted up - it's a one time check.
+
+	  If in doubt, say "Y".
+
 config DEBUG_SET_MODULE_RONX
 	bool "Set loadable kernel module data as NX and text as RO"
 	depends on MODULES
diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h
index f72ee69..6afd847 100644
--- a/arch/arm64/include/asm/ptdump.h
+++ b/arch/arm64/include/asm/ptdump.h
@@ -42,5 +42,13 @@ static inline int ptdump_debugfs_register(struct ptdump_info *info,
 	return 0;
 }
 #endif
+void ptdump_check_wx(void);
 #endif /* CONFIG_ARM64_PTDUMP_CORE */
+
+#ifdef CONFIG_DEBUG_WX
+#define debug_checkwx()	ptdump_check_wx()
+#else
+#define debug_checkwx()	do { } while (0)
+#endif
+
 #endif /* __ASM_PTDUMP_H */
diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
index bb36649..ef8aca8 100644
--- a/arch/arm64/mm/dump.c
+++ b/arch/arm64/mm/dump.c
@@ -74,6 +74,9 @@ struct pg_state {
 	unsigned long start_address;
 	unsigned level;
 	u64 current_prot;
+	bool check_wx;
+	unsigned long wx_pages;
+	unsigned long uxn_pages;
 };
 
 struct prot_bits {
@@ -202,6 +205,35 @@ static void dump_prot(struct pg_state *st, const struct prot_bits *bits,
 	}
 }
 
+static void note_prot_uxn(struct pg_state *st, unsigned long addr)
+{
+	if (!st->check_wx)
+		return;
+
+	if ((st->current_prot & PTE_UXN) == PTE_UXN)
+		return;
+
+	WARN_ONCE(1, "arm64/mm: Found non-UXN mapping at address %p/%pS\n",
+		  (void *)st->start_address, (void *)st->start_address);
+
+	st->uxn_pages += (addr - st->start_address) / PAGE_SIZE;
+}
+
+static void note_prot_wx(struct pg_state *st, unsigned long addr)
+{
+	if (!st->check_wx)
+		return;
+	if ((st->current_prot & PTE_RDONLY) == PTE_RDONLY)
+		return;
+	if ((st->current_prot & PTE_PXN) == PTE_PXN)
+		return;
+
+	WARN_ONCE(1, "arm64/mm: Found insecure W+X mapping at address %p/%pS\n",
+		  (void *)st->start_address, (void *)st->start_address);
+
+	st->wx_pages += (addr - st->start_address) / PAGE_SIZE;
+}
+
 static void note_page(struct pg_state *st, unsigned long addr, unsigned level,
 				u64 val)
 {
@@ -219,6 +251,8 @@ static void note_page(struct pg_state *st, unsigned long addr, unsigned level,
 		unsigned long delta;
 
 		if (st->current_prot) {
+			note_prot_uxn(st, addr);
+			note_prot_wx(st, addr);
 			pt_dump_seq_printf(st->seq, "0x%016lx-0x%016lx   ",
 				   st->start_address, addr);
 
@@ -344,6 +378,25 @@ static struct ptdump_info kernel_ptdump_info = {
 	.base_addr	= VA_START,
 };
 
+void ptdump_check_wx(void)
+{
+	struct pg_state st = {
+		.seq = NULL,
+		.marker = (struct addr_marker[]) {
+			{ -1, NULL},
+		},
+		.check_wx = true,
+	};
+
+	walk_pgd(&st, &init_mm, 0);
+	note_page(&st, 0, 0, 0);
+	if (st.wx_pages || st.uxn_pages)
+		pr_warn("Checked W+X mappings: FAILED, %lu W+X pages found, %lu non-UXN pages found\n",
+			st.wx_pages, st.uxn_pages);
+	else
+		pr_info("Checked W+X mappings: passed, no W+X pages found\n");
+}
+
 static int ptdump_init(void)
 {
 	ptdump_initialize();
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 05615a3..2cbe2fe 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -42,6 +42,7 @@
 #include <asm/tlb.h>
 #include <asm/memblock.h>
 #include <asm/mmu_context.h>
+#include <asm/ptdump.h>
 
 u64 idmap_t0sz = TCR_T0SZ(VA_BITS);
 
@@ -396,6 +397,7 @@ void mark_rodata_ro(void)
 	section_size = (unsigned long)__init_begin - (unsigned long)__start_rodata;
 	create_mapping_late(__pa(__start_rodata), (unsigned long)__start_rodata,
 			    section_size, PAGE_KERNEL_RO);
+	debug_checkwx();
 }
 
 static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
-- 
2.7.4

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

* Re: [PATCHv4 4/4] arm64: dump: Add checking for writable and exectuable pages
  2016-10-27 16:27 ` [PATCHv4 4/4] arm64: dump: Add checking for writable and exectuable pages Laura Abbott
@ 2016-10-28 11:52   ` Ard Biesheuvel
  0 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2016-10-28 11:52 UTC (permalink / raw)
  To: Laura Abbott
  Cc: AKASHI Takahiro, Mark Rutland, David Brown, Will Deacon,
	Catalin Marinas, linux-arm-kernel, linux-kernel, Kees Cook,
	kernel-hardening, Matt Fleming, linux-efi

On 27 October 2016 at 17:27, Laura Abbott <labbott@redhat.com> wrote:
>
> Page mappings with full RWX permissions are a security risk. x86
> has an option to walk the page tables and dump any bad pages.
> (See e1a58320a38d ("x86/mm: Warn on W^X mappings")). Add a similar
> implementation for arm64.
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> Tested-by: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Laura Abbott <labbott@redhat.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
> v4: Changed pr_info -> pr_warn. Added a separate count variable for uxn to avoid
> double counting.
> ---
>  arch/arm64/Kconfig.debug        | 29 ++++++++++++++++++++++
>  arch/arm64/include/asm/ptdump.h |  8 +++++++
>  arch/arm64/mm/dump.c            | 53 +++++++++++++++++++++++++++++++++++++++++
>  arch/arm64/mm/mmu.c             |  2 ++
>  4 files changed, 92 insertions(+)
>
> diff --git a/arch/arm64/Kconfig.debug b/arch/arm64/Kconfig.debug
> index 21a5b74..d1ebd46 100644
> --- a/arch/arm64/Kconfig.debug
> +++ b/arch/arm64/Kconfig.debug
> @@ -42,6 +42,35 @@ config ARM64_RANDOMIZE_TEXT_OFFSET
>           of TEXT_OFFSET and platforms must not require a specific
>           value.
>
> +config DEBUG_WX
> +       bool "Warn on W+X mappings at boot"
> +       select ARM64_PTDUMP_CORE
> +       ---help---
> +         Generate a warning if any W+X mappings are found at boot.
> +
> +         This is useful for discovering cases where the kernel is leaving
> +         W+X mappings after applying NX, as such mappings are a security risk.
> +         This check also includes UXN, which should be set on all kernel
> +         mappings.
> +
> +         Look for a message in dmesg output like this:
> +
> +           arm64/mm: Checked W+X mappings: passed, no W+X pages found.
> +
> +         or like this, if the check failed:
> +
> +           arm64/mm: Checked W+X mappings: FAILED, <N> W+X pages found.
> +
> +         Note that even if the check fails, your kernel is possibly
> +         still fine, as W+X mappings are not a security hole in
> +         themselves, what they do is that they make the exploitation
> +         of other unfixed kernel bugs easier.
> +
> +         There is no runtime or memory usage effect of this option
> +         once the kernel has booted up - it's a one time check.
> +
> +         If in doubt, say "Y".
> +
>  config DEBUG_SET_MODULE_RONX
>         bool "Set loadable kernel module data as NX and text as RO"
>         depends on MODULES
> diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h
> index f72ee69..6afd847 100644
> --- a/arch/arm64/include/asm/ptdump.h
> +++ b/arch/arm64/include/asm/ptdump.h
> @@ -42,5 +42,13 @@ static inline int ptdump_debugfs_register(struct ptdump_info *info,
>         return 0;
>  }
>  #endif
> +void ptdump_check_wx(void);
>  #endif /* CONFIG_ARM64_PTDUMP_CORE */
> +
> +#ifdef CONFIG_DEBUG_WX
> +#define debug_checkwx()        ptdump_check_wx()
> +#else
> +#define debug_checkwx()        do { } while (0)
> +#endif
> +
>  #endif /* __ASM_PTDUMP_H */
> diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
> index bb36649..ef8aca8 100644
> --- a/arch/arm64/mm/dump.c
> +++ b/arch/arm64/mm/dump.c
> @@ -74,6 +74,9 @@ struct pg_state {
>         unsigned long start_address;
>         unsigned level;
>         u64 current_prot;
> +       bool check_wx;
> +       unsigned long wx_pages;
> +       unsigned long uxn_pages;
>  };
>
>  struct prot_bits {
> @@ -202,6 +205,35 @@ static void dump_prot(struct pg_state *st, const struct prot_bits *bits,
>         }
>  }
>
> +static void note_prot_uxn(struct pg_state *st, unsigned long addr)
> +{
> +       if (!st->check_wx)
> +               return;
> +
> +       if ((st->current_prot & PTE_UXN) == PTE_UXN)
> +               return;
> +
> +       WARN_ONCE(1, "arm64/mm: Found non-UXN mapping at address %p/%pS\n",
> +                 (void *)st->start_address, (void *)st->start_address);
> +
> +       st->uxn_pages += (addr - st->start_address) / PAGE_SIZE;
> +}
> +
> +static void note_prot_wx(struct pg_state *st, unsigned long addr)
> +{
> +       if (!st->check_wx)
> +               return;
> +       if ((st->current_prot & PTE_RDONLY) == PTE_RDONLY)
> +               return;
> +       if ((st->current_prot & PTE_PXN) == PTE_PXN)
> +               return;
> +
> +       WARN_ONCE(1, "arm64/mm: Found insecure W+X mapping at address %p/%pS\n",
> +                 (void *)st->start_address, (void *)st->start_address);
> +
> +       st->wx_pages += (addr - st->start_address) / PAGE_SIZE;
> +}
> +
>  static void note_page(struct pg_state *st, unsigned long addr, unsigned level,
>                                 u64 val)
>  {
> @@ -219,6 +251,8 @@ static void note_page(struct pg_state *st, unsigned long addr, unsigned level,
>                 unsigned long delta;
>
>                 if (st->current_prot) {
> +                       note_prot_uxn(st, addr);
> +                       note_prot_wx(st, addr);
>                         pt_dump_seq_printf(st->seq, "0x%016lx-0x%016lx   ",
>                                    st->start_address, addr);
>
> @@ -344,6 +378,25 @@ static struct ptdump_info kernel_ptdump_info = {
>         .base_addr      = VA_START,
>  };
>
> +void ptdump_check_wx(void)
> +{
> +       struct pg_state st = {
> +               .seq = NULL,
> +               .marker = (struct addr_marker[]) {
> +                       { -1, NULL},
> +               },
> +               .check_wx = true,
> +       };
> +
> +       walk_pgd(&st, &init_mm, 0);
> +       note_page(&st, 0, 0, 0);
> +       if (st.wx_pages || st.uxn_pages)
> +               pr_warn("Checked W+X mappings: FAILED, %lu W+X pages found, %lu non-UXN pages found\n",
> +                       st.wx_pages, st.uxn_pages);
> +       else
> +               pr_info("Checked W+X mappings: passed, no W+X pages found\n");
> +}
> +
>  static int ptdump_init(void)
>  {
>         ptdump_initialize();
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 05615a3..2cbe2fe 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -42,6 +42,7 @@
>  #include <asm/tlb.h>
>  #include <asm/memblock.h>
>  #include <asm/mmu_context.h>
> +#include <asm/ptdump.h>
>
>  u64 idmap_t0sz = TCR_T0SZ(VA_BITS);
>
> @@ -396,6 +397,7 @@ void mark_rodata_ro(void)
>         section_size = (unsigned long)__init_begin - (unsigned long)__start_rodata;
>         create_mapping_late(__pa(__start_rodata), (unsigned long)__start_rodata,
>                             section_size, PAGE_KERNEL_RO);
> +       debug_checkwx();
>  }
>
>  static void __init map_kernel_segment(pgd_t *pgd, void *va_start, void *va_end,
> --
> 2.7.4
>

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

* Re: [PATCHv4 0/4] WX checking for arm64
  2016-10-27 16:27 [PATCHv4 0/4] WX checking for arm64 Laura Abbott
                   ` (3 preceding siblings ...)
  2016-10-27 16:27 ` [PATCHv4 4/4] arm64: dump: Add checking for writable and exectuable pages Laura Abbott
@ 2016-10-30 15:03 ` Catalin Marinas
  2016-11-07 15:38   ` Mark Rutland
  4 siblings, 1 reply; 12+ messages in thread
From: Catalin Marinas @ 2016-10-30 15:03 UTC (permalink / raw)
  To: Laura Abbott
  Cc: AKASHI Takahiro, Mark Rutland, Ard Biesheuvel, David Brown,
	Will Deacon, linux-efi, Kees Cook, kernel-hardening,
	Matt Fleming, linux-kernel, linux-arm-kernel

On Thu, Oct 27, 2016 at 09:27:30AM -0700, Laura Abbott wrote:
> Laura Abbott (4):
>   arm64: dump: Make ptdump debugfs a separate option
>   arm64: dump: Make the page table dumping seq_file optional
>   arm64: dump: Remove max_addr
>   arm64: dump: Add checking for writable and exectuable pages

Queued for 4.10. Thanks.

-- 
Catalin

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

* Re: [PATCHv4 0/4] WX checking for arm64
  2016-10-30 15:03 ` [PATCHv4 0/4] WX checking for arm64 Catalin Marinas
@ 2016-11-07 15:38   ` Mark Rutland
  2016-11-07 16:26     ` Laura Abbott
  2016-11-07 19:49     ` [kernel-hardening] " Mark Rutland
  0 siblings, 2 replies; 12+ messages in thread
From: Mark Rutland @ 2016-11-07 15:38 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Laura Abbott, AKASHI Takahiro, Ard Biesheuvel, David Brown,
	Will Deacon, linux-efi, Kees Cook, kernel-hardening,
	Matt Fleming, linux-kernel, linux-arm-kernel

On Sun, Oct 30, 2016 at 03:03:07PM +0000, Catalin Marinas wrote:
> On Thu, Oct 27, 2016 at 09:27:30AM -0700, Laura Abbott wrote:
> > Laura Abbott (4):
> >   arm64: dump: Make ptdump debugfs a separate option
> >   arm64: dump: Make the page table dumping seq_file optional
> >   arm64: dump: Remove max_addr
> >   arm64: dump: Add checking for writable and exectuable pages
> 
> Queued for 4.10. Thanks.

Catalin mentioned to me that he saw some KASAN splats when testing; it
looks like need a fixup something like the below.

Apologies for not having spotted this when testing!

Thanks,
Mark.

---->8----
>From 06fef1ad1138d0808eec770e64458a350941bd2d Mon Sep 17 00:00:00 2001
From: Mark Rutland <mark.rutland@arm.com>
Date: Mon, 7 Nov 2016 15:24:40 +0000
Subject: [PATCH] Fix KASAN splats with DEBUG_WX

Booting a kernel built with both CONFIG_KASAN and CONFIG_DEBUG_WX
results in a stream of KASAN splats for stack-out-of-bounds accesses,
e.g.

==================================================================
BUG: KASAN: stack-out-of-bounds in note_page+0xd8/0x650 at addr ffff8009364ebdd0
Read of size 8 by task swapper/0/1
page:ffff7e0024d93ac0 count:0 mapcount:0 mapping:          (null) index:0x0
flags: 0x4000000000000000()
page dumped because: kasan: bad access detected
CPU: 2 PID: 1 Comm: swapper/0 Not tainted 4.9.0-rc3-00004-g25f7267 #77
Hardware name: ARM Juno development board (r1) (DT)
Call trace:
[<ffff20000808b2d8>] dump_backtrace+0x0/0x278
[<ffff20000808b564>] show_stack+0x14/0x20
[<ffff2000084e4e4c>] dump_stack+0xa4/0xc8
[<ffff200008256ee0>] kasan_report_error+0x4a8/0x4d0
[<ffff200008257330>] kasan_report+0x40/0x48
[<ffff200008255894>] __asan_load8+0x84/0x98
[<ffff2000080a0928>] note_page+0xd8/0x650
[<ffff2000080a0fb4>] walk_pgd.isra.1+0x114/0x220
[<ffff2000080a1248>] ptdump_check_wx+0xa8/0x118
[<ffff20000809ed40>] mark_rodata_ro+0x90/0xd0
[<ffff200008cafb88>] kernel_init+0x28/0x110
[<ffff200008083680>] ret_from_fork+0x10/0x50
Memory state around the buggy address:
 ffff8009364ebc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 ffff8009364ebd00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff8009364ebd80: 00 00 00 00 f1 f1 f1 f1 00 00 f4 f4 f2 f2 f2 f2
                                                 ^
 ffff8009364ebe00: 00 00 00 00 00 00 00 00 f3 f3 f3 f3 00 00 00 00
 ffff8009364ebe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
==================================================================

... this happens because note_page assumes that the marker array has at
least two elements (the latter of which may be the terminator), but the
marker array for ptdump_check_wx only contains one element. Thus we
dereference some garbage on the stack when looking at
marker[1].start_address.

Given we don't need the markers for the WX checks, we could modify
note_page to allow for a NULL marker array, but for now it's simpler to
add an entry to the ptdump_check_wx marker array, so let's do that. As
it's somewhat confusing to have two identical entries, we add an initial
entry with a start address of zero.

Reported-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm64/mm/dump.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
index ef8aca8..ca74a2a 100644
--- a/arch/arm64/mm/dump.c
+++ b/arch/arm64/mm/dump.c
@@ -383,6 +383,7 @@ void ptdump_check_wx(void)
 	struct pg_state st = {
 		.seq = NULL,
 		.marker = (struct addr_marker[]) {
+			{ 0, NULL},
 			{ -1, NULL},
 		},
 		.check_wx = true,
-- 
1.9.1

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

* Re: [PATCHv4 0/4] WX checking for arm64
  2016-11-07 15:38   ` Mark Rutland
@ 2016-11-07 16:26     ` Laura Abbott
  2016-11-07 16:31       ` Catalin Marinas
  2016-11-07 19:49     ` [kernel-hardening] " Mark Rutland
  1 sibling, 1 reply; 12+ messages in thread
From: Laura Abbott @ 2016-11-07 16:26 UTC (permalink / raw)
  To: Mark Rutland, Catalin Marinas
  Cc: AKASHI Takahiro, Ard Biesheuvel, David Brown, Will Deacon,
	linux-efi, Kees Cook, kernel-hardening, Matt Fleming,
	linux-kernel, linux-arm-kernel

On 11/07/2016 07:38 AM, Mark Rutland wrote:
> On Sun, Oct 30, 2016 at 03:03:07PM +0000, Catalin Marinas wrote:
>> On Thu, Oct 27, 2016 at 09:27:30AM -0700, Laura Abbott wrote:
>>> Laura Abbott (4):
>>>   arm64: dump: Make ptdump debugfs a separate option
>>>   arm64: dump: Make the page table dumping seq_file optional
>>>   arm64: dump: Remove max_addr
>>>   arm64: dump: Add checking for writable and exectuable pages
>>
>> Queued for 4.10. Thanks.
>
> Catalin mentioned to me that he saw some KASAN splats when testing; it
> looks like need a fixup something like the below.
>
> Apologies for not having spotted this when testing!
>
> Thanks,
> Mark.
>
> ---->8----
> From 06fef1ad1138d0808eec770e64458a350941bd2d Mon Sep 17 00:00:00 2001
> From: Mark Rutland <mark.rutland@arm.com>
> Date: Mon, 7 Nov 2016 15:24:40 +0000
> Subject: [PATCH] Fix KASAN splats with DEBUG_WX
>
> Booting a kernel built with both CONFIG_KASAN and CONFIG_DEBUG_WX
> results in a stream of KASAN splats for stack-out-of-bounds accesses,
> e.g.
>
> ==================================================================
> BUG: KASAN: stack-out-of-bounds in note_page+0xd8/0x650 at addr ffff8009364ebdd0
> Read of size 8 by task swapper/0/1
> page:ffff7e0024d93ac0 count:0 mapcount:0 mapping:          (null) index:0x0
> flags: 0x4000000000000000()
> page dumped because: kasan: bad access detected
> CPU: 2 PID: 1 Comm: swapper/0 Not tainted 4.9.0-rc3-00004-g25f7267 #77
> Hardware name: ARM Juno development board (r1) (DT)
> Call trace:
> [<ffff20000808b2d8>] dump_backtrace+0x0/0x278
> [<ffff20000808b564>] show_stack+0x14/0x20
> [<ffff2000084e4e4c>] dump_stack+0xa4/0xc8
> [<ffff200008256ee0>] kasan_report_error+0x4a8/0x4d0
> [<ffff200008257330>] kasan_report+0x40/0x48
> [<ffff200008255894>] __asan_load8+0x84/0x98
> [<ffff2000080a0928>] note_page+0xd8/0x650
> [<ffff2000080a0fb4>] walk_pgd.isra.1+0x114/0x220
> [<ffff2000080a1248>] ptdump_check_wx+0xa8/0x118
> [<ffff20000809ed40>] mark_rodata_ro+0x90/0xd0
> [<ffff200008cafb88>] kernel_init+0x28/0x110
> [<ffff200008083680>] ret_from_fork+0x10/0x50
> Memory state around the buggy address:
>  ffff8009364ebc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>  ffff8009364ebd00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> ffff8009364ebd80: 00 00 00 00 f1 f1 f1 f1 00 00 f4 f4 f2 f2 f2 f2
>                                                  ^
>  ffff8009364ebe00: 00 00 00 00 00 00 00 00 f3 f3 f3 f3 00 00 00 00
>  ffff8009364ebe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ==================================================================
>
> ... this happens because note_page assumes that the marker array has at
> least two elements (the latter of which may be the terminator), but the
> marker array for ptdump_check_wx only contains one element. Thus we
> dereference some garbage on the stack when looking at
> marker[1].start_address.
>
> Given we don't need the markers for the WX checks, we could modify
> note_page to allow for a NULL marker array, but for now it's simpler to
> add an entry to the ptdump_check_wx marker array, so let's do that. As
> it's somewhat confusing to have two identical entries, we add an initial
> entry with a start address of zero.
>
> Reported-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  arch/arm64/mm/dump.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/mm/dump.c b/arch/arm64/mm/dump.c
> index ef8aca8..ca74a2a 100644
> --- a/arch/arm64/mm/dump.c
> +++ b/arch/arm64/mm/dump.c
> @@ -383,6 +383,7 @@ void ptdump_check_wx(void)
>  	struct pg_state st = {
>  		.seq = NULL,
>  		.marker = (struct addr_marker[]) {
> +			{ 0, NULL},
>  			{ -1, NULL},
>  		},
>  		.check_wx = true,
>

Acked-by: Laura Abbott <labbott@redhat.com>

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

* Re: [PATCHv4 0/4] WX checking for arm64
  2016-11-07 16:26     ` Laura Abbott
@ 2016-11-07 16:31       ` Catalin Marinas
  0 siblings, 0 replies; 12+ messages in thread
From: Catalin Marinas @ 2016-11-07 16:31 UTC (permalink / raw)
  To: Laura Abbott
  Cc: Mark Rutland, linux-efi, Kees Cook, AKASHI Takahiro,
	Matt Fleming, Ard Biesheuvel, Will Deacon, linux-kernel,
	David Brown, kernel-hardening, linux-arm-kernel

On Mon, Nov 07, 2016 at 08:26:34AM -0800, Laura Abbott wrote:
> On 11/07/2016 07:38 AM, Mark Rutland wrote:
> >From 06fef1ad1138d0808eec770e64458a350941bd2d Mon Sep 17 00:00:00 2001
> >From: Mark Rutland <mark.rutland@arm.com>
> >Date: Mon, 7 Nov 2016 15:24:40 +0000
> >Subject: [PATCH] Fix KASAN splats with DEBUG_WX
[...]
> Acked-by: Laura Abbott <labbott@redhat.com>

Thanks. I'll queue the patch on top of the others.

-- 
Catalin

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

* Re: [kernel-hardening] Re: [PATCHv4 0/4] WX checking for arm64
  2016-11-07 15:38   ` Mark Rutland
  2016-11-07 16:26     ` Laura Abbott
@ 2016-11-07 19:49     ` Mark Rutland
  2016-11-07 19:58       ` Ard Biesheuvel
  1 sibling, 1 reply; 12+ messages in thread
From: Mark Rutland @ 2016-11-07 19:49 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Laura Abbott, AKASHI Takahiro, Ard Biesheuvel, David Brown,
	Will Deacon, linux-efi, Kees Cook, kernel-hardening,
	Matt Fleming, linux-kernel, linux-arm-kernel

On Mon, Nov 07, 2016 at 03:38:02PM +0000, Mark Rutland wrote:
> On Sun, Oct 30, 2016 at 03:03:07PM +0000, Catalin Marinas wrote:
> > On Thu, Oct 27, 2016 at 09:27:30AM -0700, Laura Abbott wrote:
> > > Laura Abbott (4):
> > >   arm64: dump: Make ptdump debugfs a separate option
> > >   arm64: dump: Make the page table dumping seq_file optional
> > >   arm64: dump: Remove max_addr
> > >   arm64: dump: Add checking for writable and exectuable pages
> > 
> > Queued for 4.10. Thanks.
> 
> Catalin mentioned to me that he saw some KASAN splats when testing; it
> looks like need a fixup something like the below.

As an aside, it looks like any ptdump usage when KASAN is enabled takes
several minutes, which at boot time looks like a hang.

AFAICT, this is because KASAN allocates *huge* VA ranges (4TB+) worth of
zeroed shadow memory at pte granularity (reusing the same pmd, pud,
tables), and the ptdump code dutifully walks this with, with the added
KASAN instrumentation overhead.

I'll try to dig into that tomorrow; I suspect/hope it's not necessary to
keep all of that mapped.

Thanks,
Mark.

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

* Re: [kernel-hardening] Re: [PATCHv4 0/4] WX checking for arm64
  2016-11-07 19:49     ` [kernel-hardening] " Mark Rutland
@ 2016-11-07 19:58       ` Ard Biesheuvel
  0 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2016-11-07 19:58 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Catalin Marinas, Laura Abbott, AKASHI Takahiro, David Brown,
	Will Deacon, linux-efi, Kees Cook, kernel-hardening,
	Matt Fleming, linux-kernel, linux-arm-kernel

On 7 November 2016 at 19:49, Mark Rutland <mark.rutland@arm.com> wrote:
> On Mon, Nov 07, 2016 at 03:38:02PM +0000, Mark Rutland wrote:
>> On Sun, Oct 30, 2016 at 03:03:07PM +0000, Catalin Marinas wrote:
>> > On Thu, Oct 27, 2016 at 09:27:30AM -0700, Laura Abbott wrote:
>> > > Laura Abbott (4):
>> > >   arm64: dump: Make ptdump debugfs a separate option
>> > >   arm64: dump: Make the page table dumping seq_file optional
>> > >   arm64: dump: Remove max_addr
>> > >   arm64: dump: Add checking for writable and exectuable pages
>> >
>> > Queued for 4.10. Thanks.
>>
>> Catalin mentioned to me that he saw some KASAN splats when testing; it
>> looks like need a fixup something like the below.
>
> As an aside, it looks like any ptdump usage when KASAN is enabled takes
> several minutes, which at boot time looks like a hang.
>
> AFAICT, this is because KASAN allocates *huge* VA ranges (4TB+) worth of
> zeroed shadow memory at pte granularity (reusing the same pmd, pud,
> tables), and the ptdump code dutifully walks this with, with the added
> KASAN instrumentation overhead.
>
> I'll try to dig into that tomorrow; I suspect/hope it's not necessary to
> keep all of that mapped.
>

I have noticed that in the past, but I see how this delay at boot time
is an issue. However, I don't think there is a huge cost involved in
terms of memory footprint: AFAIK, the same PMD/PTE/kasan zero page are
mapped over and over across the range.

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

end of thread, other threads:[~2016-11-07 19:58 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-27 16:27 [PATCHv4 0/4] WX checking for arm64 Laura Abbott
2016-10-27 16:27 ` [PATCHv4 1/4] arm64: dump: Make ptdump debugfs a separate option Laura Abbott
2016-10-27 16:27 ` [PATCHv4 2/4] arm64: dump: Make the page table dumping seq_file optional Laura Abbott
2016-10-27 16:27 ` [PATCHv4 3/4] arm64: dump: Remove max_addr Laura Abbott
2016-10-27 16:27 ` [PATCHv4 4/4] arm64: dump: Add checking for writable and exectuable pages Laura Abbott
2016-10-28 11:52   ` Ard Biesheuvel
2016-10-30 15:03 ` [PATCHv4 0/4] WX checking for arm64 Catalin Marinas
2016-11-07 15:38   ` Mark Rutland
2016-11-07 16:26     ` Laura Abbott
2016-11-07 16:31       ` Catalin Marinas
2016-11-07 19:49     ` [kernel-hardening] " Mark Rutland
2016-11-07 19:58       ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).