All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] KASan for arm64
@ 2015-03-24 14:49 ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-03-24 14:49 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Andrew Morton, linux-mm,
	linux-kernel, Andrey Ryabinin

Hi,

This adds KASan for arm64.
First patch is a small prep, all major changes in the second.

It was lightly tested in qemu.
I should get a real hardware quite soon to test this.

TODO:
	Add more interceptors for memory accessing functions (memcmp, strlen, ...),
	though this could be done later.


Andrey Ryabinin (2):
  kasan, x86: move KASAN_SHADOW_OFFSET to the arch Kconfig
  arm64: add KASan support

 arch/arm64/Kconfig                   |   7 ++
 arch/arm64/include/asm/pgtable.h     |   3 +-
 arch/arm64/include/asm/string.h      |  16 +++
 arch/arm64/include/asm/thread_info.h |   8 ++
 arch/arm64/kernel/head.S             |   3 +
 arch/arm64/kernel/module.c           |  16 ++-
 arch/arm64/kernel/setup.c            |   2 +
 arch/arm64/lib/memcpy.S              |   3 +
 arch/arm64/lib/memmove.S             |   7 +-
 arch/arm64/lib/memset.S              |   3 +
 arch/arm64/mm/Makefile               |   3 +
 arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
 arch/x86/Kconfig                     |   4 +
 lib/Kconfig.kasan                    |   4 -
 14 files changed, 280 insertions(+), 10 deletions(-)
 create mode 100644 arch/arm64/mm/kasan_init.c

-- 
2.3.3


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

* [PATCH 0/2] KASan for arm64
@ 2015-03-24 14:49 ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-03-24 14:49 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Andrew Morton, linux-mm,
	linux-kernel, Andrey Ryabinin

Hi,

This adds KASan for arm64.
First patch is a small prep, all major changes in the second.

It was lightly tested in qemu.
I should get a real hardware quite soon to test this.

TODO:
	Add more interceptors for memory accessing functions (memcmp, strlen, ...),
	though this could be done later.


Andrey Ryabinin (2):
  kasan, x86: move KASAN_SHADOW_OFFSET to the arch Kconfig
  arm64: add KASan support

 arch/arm64/Kconfig                   |   7 ++
 arch/arm64/include/asm/pgtable.h     |   3 +-
 arch/arm64/include/asm/string.h      |  16 +++
 arch/arm64/include/asm/thread_info.h |   8 ++
 arch/arm64/kernel/head.S             |   3 +
 arch/arm64/kernel/module.c           |  16 ++-
 arch/arm64/kernel/setup.c            |   2 +
 arch/arm64/lib/memcpy.S              |   3 +
 arch/arm64/lib/memmove.S             |   7 +-
 arch/arm64/lib/memset.S              |   3 +
 arch/arm64/mm/Makefile               |   3 +
 arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
 arch/x86/Kconfig                     |   4 +
 lib/Kconfig.kasan                    |   4 -
 14 files changed, 280 insertions(+), 10 deletions(-)
 create mode 100644 arch/arm64/mm/kasan_init.c

-- 
2.3.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [PATCH 0/2] KASan for arm64
@ 2015-03-24 14:49 ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-03-24 14:49 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

This adds KASan for arm64.
First patch is a small prep, all major changes in the second.

It was lightly tested in qemu.
I should get a real hardware quite soon to test this.

TODO:
	Add more interceptors for memory accessing functions (memcmp, strlen, ...),
	though this could be done later.


Andrey Ryabinin (2):
  kasan, x86: move KASAN_SHADOW_OFFSET to the arch Kconfig
  arm64: add KASan support

 arch/arm64/Kconfig                   |   7 ++
 arch/arm64/include/asm/pgtable.h     |   3 +-
 arch/arm64/include/asm/string.h      |  16 +++
 arch/arm64/include/asm/thread_info.h |   8 ++
 arch/arm64/kernel/head.S             |   3 +
 arch/arm64/kernel/module.c           |  16 ++-
 arch/arm64/kernel/setup.c            |   2 +
 arch/arm64/lib/memcpy.S              |   3 +
 arch/arm64/lib/memmove.S             |   7 +-
 arch/arm64/lib/memset.S              |   3 +
 arch/arm64/mm/Makefile               |   3 +
 arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
 arch/x86/Kconfig                     |   4 +
 lib/Kconfig.kasan                    |   4 -
 14 files changed, 280 insertions(+), 10 deletions(-)
 create mode 100644 arch/arm64/mm/kasan_init.c

-- 
2.3.3

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

* [PATCH 1/2] kasan, x86: move KASAN_SHADOW_OFFSET to the arch Kconfig
  2015-03-24 14:49 ` Andrey Ryabinin
  (?)
@ 2015-03-24 14:49   ` Andrey Ryabinin
  -1 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-03-24 14:49 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Andrew Morton, linux-mm,
	linux-kernel, Andrey Ryabinin

KASAN_SHADOW_OFFSET is purely arch specific setting,
so it should be in arch's Kconfig file. This simplifies
porting KASan to other architectures and maintenance of it.

Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com>
---
 arch/x86/Kconfig  | 4 ++++
 lib/Kconfig.kasan | 4 ----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cb23206..66ee917 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -247,6 +247,10 @@ config ARCH_SUPPORTS_OPTIMIZED_INLINING
 config ARCH_SUPPORTS_DEBUG_PAGEALLOC
 	def_bool y
 
+config KASAN_SHADOW_OFFSET
+	hex
+	default 0xdffffc0000000000
+
 config HAVE_INTEL_TXT
 	def_bool y
 	depends on INTEL_IOMMU && ACPI
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index 4fecaedc..ba31b8c 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -15,10 +15,6 @@ config KASAN
 	  For better error detection enable CONFIG_STACKTRACE,
 	  and add slub_debug=U to boot cmdline.
 
-config KASAN_SHADOW_OFFSET
-	hex
-	default 0xdffffc0000000000 if X86_64
-
 choice
 	prompt "Instrumentation type"
 	depends on KASAN
-- 
2.3.3


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

* [PATCH 1/2] kasan, x86: move KASAN_SHADOW_OFFSET to the arch Kconfig
@ 2015-03-24 14:49   ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-03-24 14:49 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Andrew Morton, linux-mm,
	linux-kernel, Andrey Ryabinin

KASAN_SHADOW_OFFSET is purely arch specific setting,
so it should be in arch's Kconfig file. This simplifies
porting KASan to other architectures and maintenance of it.

Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com>
---
 arch/x86/Kconfig  | 4 ++++
 lib/Kconfig.kasan | 4 ----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cb23206..66ee917 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -247,6 +247,10 @@ config ARCH_SUPPORTS_OPTIMIZED_INLINING
 config ARCH_SUPPORTS_DEBUG_PAGEALLOC
 	def_bool y
 
+config KASAN_SHADOW_OFFSET
+	hex
+	default 0xdffffc0000000000
+
 config HAVE_INTEL_TXT
 	def_bool y
 	depends on INTEL_IOMMU && ACPI
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index 4fecaedc..ba31b8c 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -15,10 +15,6 @@ config KASAN
 	  For better error detection enable CONFIG_STACKTRACE,
 	  and add slub_debug=U to boot cmdline.
 
-config KASAN_SHADOW_OFFSET
-	hex
-	default 0xdffffc0000000000 if X86_64
-
 choice
 	prompt "Instrumentation type"
 	depends on KASAN
-- 
2.3.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [PATCH 1/2] kasan, x86: move KASAN_SHADOW_OFFSET to the arch Kconfig
@ 2015-03-24 14:49   ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-03-24 14:49 UTC (permalink / raw)
  To: linux-arm-kernel

KASAN_SHADOW_OFFSET is purely arch specific setting,
so it should be in arch's Kconfig file. This simplifies
porting KASan to other architectures and maintenance of it.

Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com>
---
 arch/x86/Kconfig  | 4 ++++
 lib/Kconfig.kasan | 4 ----
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cb23206..66ee917 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -247,6 +247,10 @@ config ARCH_SUPPORTS_OPTIMIZED_INLINING
 config ARCH_SUPPORTS_DEBUG_PAGEALLOC
 	def_bool y
 
+config KASAN_SHADOW_OFFSET
+	hex
+	default 0xdffffc0000000000
+
 config HAVE_INTEL_TXT
 	def_bool y
 	depends on INTEL_IOMMU && ACPI
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index 4fecaedc..ba31b8c 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -15,10 +15,6 @@ config KASAN
 	  For better error detection enable CONFIG_STACKTRACE,
 	  and add slub_debug=U to boot cmdline.
 
-config KASAN_SHADOW_OFFSET
-	hex
-	default 0xdffffc0000000000 if X86_64
-
 choice
 	prompt "Instrumentation type"
 	depends on KASAN
-- 
2.3.3

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

* [PATCH 2/2] arm64: add KASan support
  2015-03-24 14:49 ` Andrey Ryabinin
  (?)
@ 2015-03-24 14:49   ` Andrey Ryabinin
  -1 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-03-24 14:49 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Andrew Morton, linux-mm,
	linux-kernel, Andrey Ryabinin

This patch adds arch specific code for kernel address sanitizer
(see Documentation/kasan.txt).

1/8 of kernel addresses reserved for shadow memory. There was no
big enough hole for this, so virtual addresses for shadow were
stolen from vmalloc area.

At early boot stage the whole shadow region populated with just
one physical page (kasan_zero_page). Later, this page reused
as readonly zero shadow for some memory that KASan currently
don't track (vmalloc).
After mapping the physical memory, pages for shadow memory are
allocated and mapped.

KASan's stack instrumentation significantly increases stack's
consumption, so CONFIG_KASAN doubles THREAD_SIZE.

Functions like memset/memmove/memcpy do a lot of memory accesses.
If bad pointer passed to one of these function it is important
to catch this. Compiler's instrumentation cannot do this since
these functions are written in assembly.
KASan replaces memory functions with manually instrumented variants.
Original functions declared as weak symbols so strong definitions
in mm/kasan/kasan.c could replace them. Original functions have aliases
with '__' prefix in name, so we could call non-instrumented variant
if needed.
Some files built without kasan instrumentation (e.g. mm/slub.c).
Original mem* function replaced (via #define) with prefixed variants
to disable memory access checks for such files.

Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com>
---
 arch/arm64/Kconfig                   |   7 ++
 arch/arm64/include/asm/pgtable.h     |   3 +-
 arch/arm64/include/asm/string.h      |  16 +++
 arch/arm64/include/asm/thread_info.h |   8 ++
 arch/arm64/kernel/head.S             |   3 +
 arch/arm64/kernel/module.c           |  16 ++-
 arch/arm64/kernel/setup.c            |   2 +
 arch/arm64/lib/memcpy.S              |   3 +
 arch/arm64/lib/memmove.S             |   7 +-
 arch/arm64/lib/memset.S              |   3 +
 arch/arm64/mm/Makefile               |   3 +
 arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
 12 files changed, 276 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm64/mm/kasan_init.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 4085df1..10bbd71 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -41,6 +41,7 @@ config ARM64
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_ARCH_BITREVERSE
 	select HAVE_ARCH_JUMP_LABEL
+	select HAVE_ARCH_KASAN if SPARSEMEM_VMEMMAP
 	select HAVE_ARCH_KGDB
 	select HAVE_ARCH_SECCOMP_FILTER
 	select HAVE_ARCH_TRACEHOOK
@@ -116,6 +117,12 @@ config GENERIC_CSUM
 config GENERIC_CALIBRATE_DELAY
 	def_bool y
 
+config KASAN_SHADOW_OFFSET
+	hex
+	default 0xdfff200000000000 if ARM64_VA_BITS_48
+	default 0xdffffc8000000000 if ARM64_VA_BITS_42
+	default 0xdfffff9000000000 if ARM64_VA_BITS_39
+
 config ZONE_DMA
 	def_bool y
 
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index bd5db28..f5ce010 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -40,7 +40,7 @@
  *	fixed mappings and modules
  */
 #define VMEMMAP_SIZE		ALIGN((1UL << (VA_BITS - PAGE_SHIFT)) * sizeof(struct page), PUD_SIZE)
-#define VMALLOC_START		(UL(0xffffffffffffffff) << VA_BITS)
+#define VMALLOC_START		((UL(0xffffffffffffffff) << VA_BITS) + (UL(1) << (VA_BITS - 3)))
 #define VMALLOC_END		(PAGE_OFFSET - PUD_SIZE - VMEMMAP_SIZE - SZ_64K)
 
 #define vmemmap			((struct page *)(VMALLOC_END + SZ_64K))
@@ -73,6 +73,7 @@ extern void __pgd_error(const char *file, int line, unsigned long val);
 
 #define PAGE_KERNEL		__pgprot(_PAGE_DEFAULT | PTE_PXN | PTE_UXN | PTE_DIRTY | PTE_WRITE)
 #define PAGE_KERNEL_EXEC	__pgprot(_PAGE_DEFAULT | PTE_UXN | PTE_DIRTY | PTE_WRITE)
+#define PAGE_KERNEL_RO		__pgprot(_PAGE_DEFAULT | PTE_PXN | PTE_UXN | PTE_DIRTY)
 
 #define PAGE_HYP		__pgprot(_PAGE_DEFAULT | PTE_HYP)
 #define PAGE_HYP_DEVICE		__pgprot(PROT_DEVICE_nGnRE | PTE_HYP)
diff --git a/arch/arm64/include/asm/string.h b/arch/arm64/include/asm/string.h
index 64d2d48..bff522c 100644
--- a/arch/arm64/include/asm/string.h
+++ b/arch/arm64/include/asm/string.h
@@ -36,17 +36,33 @@ extern __kernel_size_t strnlen(const char *, __kernel_size_t);
 
 #define __HAVE_ARCH_MEMCPY
 extern void *memcpy(void *, const void *, __kernel_size_t);
+extern void *__memcpy(void *, const void *, __kernel_size_t);
 
 #define __HAVE_ARCH_MEMMOVE
 extern void *memmove(void *, const void *, __kernel_size_t);
+extern void *__memmove(void *, const void *, __kernel_size_t);
 
 #define __HAVE_ARCH_MEMCHR
 extern void *memchr(const void *, int, __kernel_size_t);
 
 #define __HAVE_ARCH_MEMSET
 extern void *memset(void *, int, __kernel_size_t);
+extern void *__memset(void *, int, __kernel_size_t);
 
 #define __HAVE_ARCH_MEMCMP
 extern int memcmp(const void *, const void *, size_t);
 
+
+#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
+
+/*
+ * For files that not instrumented (e.g. mm/slub.c) we
+ * should use not instrumented version of mem* functions.
+ */
+
+#define memcpy(dst, src, len) __memcpy(dst, src, len)
+#define memmove(dst, src, len) __memmove(dst, src, len)
+#define memset(s, c, n) __memset(s, c, n)
+#endif
+
 #endif
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 702e1e6..4a2c11e 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -24,10 +24,18 @@
 #include <linux/compiler.h>
 
 #ifndef CONFIG_ARM64_64K_PAGES
+#ifndef CONFIG_KASAN
 #define THREAD_SIZE_ORDER	2
+#else
+#define THREAD_SIZE_ORDER	3
+#endif
 #endif
 
+#ifndef CONFIG_KASAN
 #define THREAD_SIZE		16384
+#else
+#define THREAD_SIZE		32768
+#endif
 #define THREAD_START_SP		(THREAD_SIZE - 16)
 
 #ifndef __ASSEMBLY__
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 51c9811..1a99e95 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -482,6 +482,9 @@ __mmap_switched:
 	str_l	x21, __fdt_pointer, x5		// Save FDT pointer
 	str_l	x24, memstart_addr, x6		// Save PHYS_OFFSET
 	mov	x29, #0
+#ifdef CONFIG_KASAN
+	b kasan_early_init
+#endif
 	b	start_kernel
 ENDPROC(__mmap_switched)
 
diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
index 67bf410..7d90c0f 100644
--- a/arch/arm64/kernel/module.c
+++ b/arch/arm64/kernel/module.c
@@ -21,6 +21,7 @@
 #include <linux/bitops.h>
 #include <linux/elf.h>
 #include <linux/gfp.h>
+#include <linux/kasan.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/moduleloader.h>
@@ -34,9 +35,18 @@
 
 void *module_alloc(unsigned long size)
 {
-	return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END,
-				    GFP_KERNEL, PAGE_KERNEL_EXEC, 0,
-				    NUMA_NO_NODE, __builtin_return_address(0));
+	void *p;
+
+	p = __vmalloc_node_range(size, MODULE_ALIGN, MODULES_VADDR, MODULES_END,
+				GFP_KERNEL, PAGE_KERNEL_EXEC, 0,
+				NUMA_NO_NODE, __builtin_return_address(0));
+
+	if (p && (kasan_module_alloc(p, size) < 0)) {
+		vfree(p);
+		return NULL;
+	}
+
+	return p;
 }
 
 enum aarch64_reloc_op {
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 51ef972..f197f45 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -31,6 +31,7 @@
 #include <linux/screen_info.h>
 #include <linux/init.h>
 #include <linux/kexec.h>
+#include <linux/kasan.h>
 #include <linux/crash_dump.h>
 #include <linux/root_dev.h>
 #include <linux/clk-provider.h>
@@ -396,6 +397,7 @@ void __init setup_arch(char **cmdline_p)
 	arm64_memblock_init();
 
 	paging_init();
+	kasan_init();
 	request_standard_resources();
 
 	early_ioremap_reset();
diff --git a/arch/arm64/lib/memcpy.S b/arch/arm64/lib/memcpy.S
index 8a9a96d..845e40a 100644
--- a/arch/arm64/lib/memcpy.S
+++ b/arch/arm64/lib/memcpy.S
@@ -56,6 +56,8 @@ C_h	.req	x12
 D_l	.req	x13
 D_h	.req	x14
 
+.weak memcpy
+ENTRY(__memcpy)
 ENTRY(memcpy)
 	mov	dst, dstin
 	cmp	count, #16
@@ -199,3 +201,4 @@ ENTRY(memcpy)
 	b.ne	.Ltail63
 	ret
 ENDPROC(memcpy)
+ENDPROC(__memcpy)
diff --git a/arch/arm64/lib/memmove.S b/arch/arm64/lib/memmove.S
index 57b19ea..48074d2 100644
--- a/arch/arm64/lib/memmove.S
+++ b/arch/arm64/lib/memmove.S
@@ -57,12 +57,14 @@ C_h	.req	x12
 D_l	.req	x13
 D_h	.req	x14
 
+.weak memmove
+ENTRY(__memmove)
 ENTRY(memmove)
 	cmp	dstin, src
-	b.lo	memcpy
+	b.lo	__memcpy
 	add	tmp1, src, count
 	cmp	dstin, tmp1
-	b.hs	memcpy		/* No overlap.  */
+	b.hs	__memcpy		/* No overlap.  */
 
 	add	dst, dstin, count
 	add	src, src, count
@@ -195,3 +197,4 @@ ENTRY(memmove)
 	b.ne	.Ltail63
 	ret
 ENDPROC(memmove)
+ENDPROC(__memmove)
diff --git a/arch/arm64/lib/memset.S b/arch/arm64/lib/memset.S
index 7c72dfd..4ab2594 100644
--- a/arch/arm64/lib/memset.S
+++ b/arch/arm64/lib/memset.S
@@ -54,6 +54,8 @@ dst		.req	x8
 tmp3w		.req	w9
 tmp3		.req	x9
 
+.weak memset
+ENTRY(__memset)
 ENTRY(memset)
 	mov	dst, dstin	/* Preserve return value.  */
 	and	A_lw, val, #255
@@ -214,3 +216,4 @@ ENTRY(memset)
 	b.ne	.Ltail_maybe_long
 	ret
 ENDPROC(memset)
+ENDPROC(__memset)
diff --git a/arch/arm64/mm/Makefile b/arch/arm64/mm/Makefile
index 773d37a..e17703c 100644
--- a/arch/arm64/mm/Makefile
+++ b/arch/arm64/mm/Makefile
@@ -4,3 +4,6 @@ obj-y				:= dma-mapping.o extable.o fault.o init.o \
 				   context.o proc.o pageattr.o
 obj-$(CONFIG_HUGETLB_PAGE)	+= hugetlbpage.o
 obj-$(CONFIG_ARM64_PTDUMP)	+= dump.o
+
+KASAN_SANITIZE_kasan_init.o	:= n
+obj-$(CONFIG_KASAN)		+= kasan_init.o
diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
new file mode 100644
index 0000000..df537da
--- /dev/null
+++ b/arch/arm64/mm/kasan_init.c
@@ -0,0 +1,211 @@
+#include <linux/kasan.h>
+#include <linux/kernel.h>
+#include <linux/memblock.h>
+#include <linux/start_kernel.h>
+
+#include <asm/page.h>
+#include <asm/pgtable.h>
+#include <asm/tlbflush.h>
+
+static char kasan_zero_page[PAGE_SIZE] __page_aligned_bss;
+static pgd_t tmp_page_table[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
+
+#if CONFIG_PGTABLE_LEVELS > 3
+static pud_t kasan_zero_pud[PTRS_PER_PUD] __page_aligned_bss;
+#endif
+#if CONFIG_PGTABLE_LEVELS > 2
+static pmd_t kasan_zero_pmd[PTRS_PER_PMD] __page_aligned_bss;
+#endif
+static pte_t kasan_zero_pte[PTRS_PER_PTE] __page_aligned_bss;
+
+static void __init init_kasan_page_tables(void)
+{
+	int i;
+
+#if CONFIG_PGTABLE_LEVELS > 3
+	for (i = 0; i < PTRS_PER_PUD; i++)
+		set_pud(&kasan_zero_pud[i], __pud(__pa(kasan_zero_pmd)
+							| PAGE_KERNEL));
+#endif
+#if CONFIG_PGTABLE_LEVELS > 2
+	for (i = 0; i < PTRS_PER_PMD; i++)
+		set_pmd(&kasan_zero_pmd[i], __pmd(__pa(kasan_zero_pte)
+							| PAGE_KERNEL));
+#endif
+	for (i = 0; i < PTRS_PER_PTE; i++)
+		set_pte(&kasan_zero_pte[i], __pte(__pa(kasan_zero_page)
+							| PAGE_KERNEL));
+}
+
+void __init kasan_map_early_shadow(pgd_t *pgdp)
+{
+	int i;
+	unsigned long start = KASAN_SHADOW_START;
+	unsigned long end = KASAN_SHADOW_END;
+	pgd_t pgd;
+
+#if CONFIG_PGTABLE_LEVELS > 3
+	pgd = __pgd(__pa(kasan_zero_pud) | PAGE_KERNEL);
+#elif CONFIG_PGTABLE_LEVELS > 2
+	pgd = __pgd(__pa(kasan_zero_pmd) | PAGE_KERNEL);
+#else
+	pgd = __pgd(__pa(kasan_zero_pte) | PAGE_KERNEL);
+#endif
+
+	for (i = pgd_index(start); start < end; i++) {
+		set_pgd(&pgdp[i], pgd);
+		start += PGDIR_SIZE;
+	}
+}
+
+void __init kasan_early_init(void)
+{
+	init_kasan_page_tables();
+	kasan_map_early_shadow(swapper_pg_dir);
+	kasan_map_early_shadow(idmap_pg_dir);
+	flush_tlb_all();
+	start_kernel();
+}
+
+static void __init clear_pgds(unsigned long start,
+			unsigned long end)
+{
+	for (; start && start < end; start += PGDIR_SIZE)
+		set_pgd(pgd_offset_k(start), __pgd(0));
+}
+
+static int __init zero_pte_populate(pmd_t *pmd, unsigned long addr,
+				unsigned long end)
+{
+	pte_t *pte = pte_offset_kernel(pmd, addr);
+
+	while (addr + PAGE_SIZE <= end) {
+		set_pte(pte, __pte(__pa(kasan_zero_page)
+					| PAGE_KERNEL_RO));
+		addr += PAGE_SIZE;
+		pte = pte_offset_kernel(pmd, addr);
+	}
+	return 0;
+}
+
+static int __init zero_pmd_populate(pud_t *pud, unsigned long addr,
+				unsigned long end)
+{
+	int ret = 0;
+	pmd_t *pmd = pmd_offset(pud, addr);
+
+	while (IS_ALIGNED(addr, PMD_SIZE) && addr + PMD_SIZE <= end) {
+		set_pmd(pmd, __pmd(__pa(kasan_zero_pte)
+					| PAGE_KERNEL_RO));
+		addr += PMD_SIZE;
+		pmd++;
+	}
+
+	if (addr < end) {
+		if (pmd_none(*pmd)) {
+			void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
+			if (!p)
+				return -ENOMEM;
+			set_pmd(pmd, __pmd(__pa(p) | PAGE_KERNEL));
+		}
+		ret = zero_pte_populate(pmd, addr, end);
+	}
+	return ret;
+}
+
+static int __init zero_pud_populate(pgd_t *pgd, unsigned long addr,
+				unsigned long end)
+{
+	int ret = 0;
+	pud_t *pud = pud_offset(pgd, addr);
+
+#if CONFIG_PGTABLE_LEVELS > 2
+	while (IS_ALIGNED(addr, PUD_SIZE) && addr + PUD_SIZE <= end) {
+		set_pud(pud, __pud(__pa(kasan_zero_pmd)
+					| PAGE_KERNEL_RO));
+		addr += PUD_SIZE;
+		pud++;
+	}
+#endif
+
+	if (addr < end) {
+		if (pud_none(*pud)) {
+			void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
+			if (!p)
+				return -ENOMEM;
+			set_pud(pud, __pud(__pa(p) | PAGE_KERNEL));
+		}
+		ret = zero_pmd_populate(pud, addr, end);
+	}
+	return ret;
+}
+
+static int __init zero_pgd_populate(unsigned long addr, unsigned long end)
+{
+	int ret = 0;
+	pgd_t *pgd = pgd_offset_k(addr);
+
+#if CONFIG_PGTABLE_LEVELS > 3
+	 while (IS_ALIGNED(addr, PGDIR_SIZE) && addr + PGDIR_SIZE <= end) {
+		set_pgd(pgd, __pgd(__pa(kasan_zero_pud)
+					| PAGE_KERNEL_RO));
+		addr += PGDIR_SIZE;
+		pgd++;
+	}
+#endif
+
+	 if (addr < end) {
+		 if (pgd_none(*pgd)) {
+			 void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
+			 if (!p)
+				 return -ENOMEM;
+			 set_pgd(pgd, __pgd(__pa(p) | PAGE_KERNEL));
+		 }
+		 ret = zero_pud_populate(pgd, addr, end);
+	 }
+	 return ret;
+}
+
+static void __init populate_zero_shadow(unsigned long start, unsigned long end)
+{
+	if (zero_pgd_populate(start, end))
+		panic("kasan: unable to map zero shadow!");
+}
+
+static void cpu_set_ttbr1(unsigned long ttbr1)
+{
+	asm(
+	"	msr	ttbr1_el1, %0\n"
+	"	isb"
+	:
+	: "r" (ttbr1));
+}
+
+void __init kasan_init(void)
+{
+	struct memblock_region *reg;
+
+	memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
+	cpu_set_ttbr1(__pa(tmp_page_table));
+
+	clear_pgds(KASAN_SHADOW_START, KASAN_SHADOW_END);
+
+	populate_zero_shadow(KASAN_SHADOW_START,
+			(unsigned long)kasan_mem_to_shadow((void *)MODULES_VADDR));
+
+	for_each_memblock(memory, reg) {
+		void *start = (void *)__phys_to_virt(reg->base);
+		void *end = (void *)__phys_to_virt(reg->base + reg->size);
+
+		if (start >= end)
+			break;
+
+		vmemmap_populate((unsigned long)kasan_mem_to_shadow(start),
+				(unsigned long)kasan_mem_to_shadow(end),
+				pfn_to_nid(virt_to_pfn(start)));
+	}
+
+	memset(kasan_zero_page, 0, PAGE_SIZE);
+	cpu_set_ttbr1(__pa(swapper_pg_dir));
+	init_task.kasan_depth = 0;
+}
-- 
2.3.3


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

* [PATCH 2/2] arm64: add KASan support
@ 2015-03-24 14:49   ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-03-24 14:49 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, Andrew Morton, linux-mm,
	linux-kernel, Andrey Ryabinin

This patch adds arch specific code for kernel address sanitizer
(see Documentation/kasan.txt).

1/8 of kernel addresses reserved for shadow memory. There was no
big enough hole for this, so virtual addresses for shadow were
stolen from vmalloc area.

At early boot stage the whole shadow region populated with just
one physical page (kasan_zero_page). Later, this page reused
as readonly zero shadow for some memory that KASan currently
don't track (vmalloc).
After mapping the physical memory, pages for shadow memory are
allocated and mapped.

KASan's stack instrumentation significantly increases stack's
consumption, so CONFIG_KASAN doubles THREAD_SIZE.

Functions like memset/memmove/memcpy do a lot of memory accesses.
If bad pointer passed to one of these function it is important
to catch this. Compiler's instrumentation cannot do this since
these functions are written in assembly.
KASan replaces memory functions with manually instrumented variants.
Original functions declared as weak symbols so strong definitions
in mm/kasan/kasan.c could replace them. Original functions have aliases
with '__' prefix in name, so we could call non-instrumented variant
if needed.
Some files built without kasan instrumentation (e.g. mm/slub.c).
Original mem* function replaced (via #define) with prefixed variants
to disable memory access checks for such files.

Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com>
---
 arch/arm64/Kconfig                   |   7 ++
 arch/arm64/include/asm/pgtable.h     |   3 +-
 arch/arm64/include/asm/string.h      |  16 +++
 arch/arm64/include/asm/thread_info.h |   8 ++
 arch/arm64/kernel/head.S             |   3 +
 arch/arm64/kernel/module.c           |  16 ++-
 arch/arm64/kernel/setup.c            |   2 +
 arch/arm64/lib/memcpy.S              |   3 +
 arch/arm64/lib/memmove.S             |   7 +-
 arch/arm64/lib/memset.S              |   3 +
 arch/arm64/mm/Makefile               |   3 +
 arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
 12 files changed, 276 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm64/mm/kasan_init.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 4085df1..10bbd71 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -41,6 +41,7 @@ config ARM64
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_ARCH_BITREVERSE
 	select HAVE_ARCH_JUMP_LABEL
+	select HAVE_ARCH_KASAN if SPARSEMEM_VMEMMAP
 	select HAVE_ARCH_KGDB
 	select HAVE_ARCH_SECCOMP_FILTER
 	select HAVE_ARCH_TRACEHOOK
@@ -116,6 +117,12 @@ config GENERIC_CSUM
 config GENERIC_CALIBRATE_DELAY
 	def_bool y
 
+config KASAN_SHADOW_OFFSET
+	hex
+	default 0xdfff200000000000 if ARM64_VA_BITS_48
+	default 0xdffffc8000000000 if ARM64_VA_BITS_42
+	default 0xdfffff9000000000 if ARM64_VA_BITS_39
+
 config ZONE_DMA
 	def_bool y
 
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index bd5db28..f5ce010 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -40,7 +40,7 @@
  *	fixed mappings and modules
  */
 #define VMEMMAP_SIZE		ALIGN((1UL << (VA_BITS - PAGE_SHIFT)) * sizeof(struct page), PUD_SIZE)
-#define VMALLOC_START		(UL(0xffffffffffffffff) << VA_BITS)
+#define VMALLOC_START		((UL(0xffffffffffffffff) << VA_BITS) + (UL(1) << (VA_BITS - 3)))
 #define VMALLOC_END		(PAGE_OFFSET - PUD_SIZE - VMEMMAP_SIZE - SZ_64K)
 
 #define vmemmap			((struct page *)(VMALLOC_END + SZ_64K))
@@ -73,6 +73,7 @@ extern void __pgd_error(const char *file, int line, unsigned long val);
 
 #define PAGE_KERNEL		__pgprot(_PAGE_DEFAULT | PTE_PXN | PTE_UXN | PTE_DIRTY | PTE_WRITE)
 #define PAGE_KERNEL_EXEC	__pgprot(_PAGE_DEFAULT | PTE_UXN | PTE_DIRTY | PTE_WRITE)
+#define PAGE_KERNEL_RO		__pgprot(_PAGE_DEFAULT | PTE_PXN | PTE_UXN | PTE_DIRTY)
 
 #define PAGE_HYP		__pgprot(_PAGE_DEFAULT | PTE_HYP)
 #define PAGE_HYP_DEVICE		__pgprot(PROT_DEVICE_nGnRE | PTE_HYP)
diff --git a/arch/arm64/include/asm/string.h b/arch/arm64/include/asm/string.h
index 64d2d48..bff522c 100644
--- a/arch/arm64/include/asm/string.h
+++ b/arch/arm64/include/asm/string.h
@@ -36,17 +36,33 @@ extern __kernel_size_t strnlen(const char *, __kernel_size_t);
 
 #define __HAVE_ARCH_MEMCPY
 extern void *memcpy(void *, const void *, __kernel_size_t);
+extern void *__memcpy(void *, const void *, __kernel_size_t);
 
 #define __HAVE_ARCH_MEMMOVE
 extern void *memmove(void *, const void *, __kernel_size_t);
+extern void *__memmove(void *, const void *, __kernel_size_t);
 
 #define __HAVE_ARCH_MEMCHR
 extern void *memchr(const void *, int, __kernel_size_t);
 
 #define __HAVE_ARCH_MEMSET
 extern void *memset(void *, int, __kernel_size_t);
+extern void *__memset(void *, int, __kernel_size_t);
 
 #define __HAVE_ARCH_MEMCMP
 extern int memcmp(const void *, const void *, size_t);
 
+
+#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
+
+/*
+ * For files that not instrumented (e.g. mm/slub.c) we
+ * should use not instrumented version of mem* functions.
+ */
+
+#define memcpy(dst, src, len) __memcpy(dst, src, len)
+#define memmove(dst, src, len) __memmove(dst, src, len)
+#define memset(s, c, n) __memset(s, c, n)
+#endif
+
 #endif
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 702e1e6..4a2c11e 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -24,10 +24,18 @@
 #include <linux/compiler.h>
 
 #ifndef CONFIG_ARM64_64K_PAGES
+#ifndef CONFIG_KASAN
 #define THREAD_SIZE_ORDER	2
+#else
+#define THREAD_SIZE_ORDER	3
+#endif
 #endif
 
+#ifndef CONFIG_KASAN
 #define THREAD_SIZE		16384
+#else
+#define THREAD_SIZE		32768
+#endif
 #define THREAD_START_SP		(THREAD_SIZE - 16)
 
 #ifndef __ASSEMBLY__
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 51c9811..1a99e95 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -482,6 +482,9 @@ __mmap_switched:
 	str_l	x21, __fdt_pointer, x5		// Save FDT pointer
 	str_l	x24, memstart_addr, x6		// Save PHYS_OFFSET
 	mov	x29, #0
+#ifdef CONFIG_KASAN
+	b kasan_early_init
+#endif
 	b	start_kernel
 ENDPROC(__mmap_switched)
 
diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
index 67bf410..7d90c0f 100644
--- a/arch/arm64/kernel/module.c
+++ b/arch/arm64/kernel/module.c
@@ -21,6 +21,7 @@
 #include <linux/bitops.h>
 #include <linux/elf.h>
 #include <linux/gfp.h>
+#include <linux/kasan.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/moduleloader.h>
@@ -34,9 +35,18 @@
 
 void *module_alloc(unsigned long size)
 {
-	return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END,
-				    GFP_KERNEL, PAGE_KERNEL_EXEC, 0,
-				    NUMA_NO_NODE, __builtin_return_address(0));
+	void *p;
+
+	p = __vmalloc_node_range(size, MODULE_ALIGN, MODULES_VADDR, MODULES_END,
+				GFP_KERNEL, PAGE_KERNEL_EXEC, 0,
+				NUMA_NO_NODE, __builtin_return_address(0));
+
+	if (p && (kasan_module_alloc(p, size) < 0)) {
+		vfree(p);
+		return NULL;
+	}
+
+	return p;
 }
 
 enum aarch64_reloc_op {
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 51ef972..f197f45 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -31,6 +31,7 @@
 #include <linux/screen_info.h>
 #include <linux/init.h>
 #include <linux/kexec.h>
+#include <linux/kasan.h>
 #include <linux/crash_dump.h>
 #include <linux/root_dev.h>
 #include <linux/clk-provider.h>
@@ -396,6 +397,7 @@ void __init setup_arch(char **cmdline_p)
 	arm64_memblock_init();
 
 	paging_init();
+	kasan_init();
 	request_standard_resources();
 
 	early_ioremap_reset();
diff --git a/arch/arm64/lib/memcpy.S b/arch/arm64/lib/memcpy.S
index 8a9a96d..845e40a 100644
--- a/arch/arm64/lib/memcpy.S
+++ b/arch/arm64/lib/memcpy.S
@@ -56,6 +56,8 @@ C_h	.req	x12
 D_l	.req	x13
 D_h	.req	x14
 
+.weak memcpy
+ENTRY(__memcpy)
 ENTRY(memcpy)
 	mov	dst, dstin
 	cmp	count, #16
@@ -199,3 +201,4 @@ ENTRY(memcpy)
 	b.ne	.Ltail63
 	ret
 ENDPROC(memcpy)
+ENDPROC(__memcpy)
diff --git a/arch/arm64/lib/memmove.S b/arch/arm64/lib/memmove.S
index 57b19ea..48074d2 100644
--- a/arch/arm64/lib/memmove.S
+++ b/arch/arm64/lib/memmove.S
@@ -57,12 +57,14 @@ C_h	.req	x12
 D_l	.req	x13
 D_h	.req	x14
 
+.weak memmove
+ENTRY(__memmove)
 ENTRY(memmove)
 	cmp	dstin, src
-	b.lo	memcpy
+	b.lo	__memcpy
 	add	tmp1, src, count
 	cmp	dstin, tmp1
-	b.hs	memcpy		/* No overlap.  */
+	b.hs	__memcpy		/* No overlap.  */
 
 	add	dst, dstin, count
 	add	src, src, count
@@ -195,3 +197,4 @@ ENTRY(memmove)
 	b.ne	.Ltail63
 	ret
 ENDPROC(memmove)
+ENDPROC(__memmove)
diff --git a/arch/arm64/lib/memset.S b/arch/arm64/lib/memset.S
index 7c72dfd..4ab2594 100644
--- a/arch/arm64/lib/memset.S
+++ b/arch/arm64/lib/memset.S
@@ -54,6 +54,8 @@ dst		.req	x8
 tmp3w		.req	w9
 tmp3		.req	x9
 
+.weak memset
+ENTRY(__memset)
 ENTRY(memset)
 	mov	dst, dstin	/* Preserve return value.  */
 	and	A_lw, val, #255
@@ -214,3 +216,4 @@ ENTRY(memset)
 	b.ne	.Ltail_maybe_long
 	ret
 ENDPROC(memset)
+ENDPROC(__memset)
diff --git a/arch/arm64/mm/Makefile b/arch/arm64/mm/Makefile
index 773d37a..e17703c 100644
--- a/arch/arm64/mm/Makefile
+++ b/arch/arm64/mm/Makefile
@@ -4,3 +4,6 @@ obj-y				:= dma-mapping.o extable.o fault.o init.o \
 				   context.o proc.o pageattr.o
 obj-$(CONFIG_HUGETLB_PAGE)	+= hugetlbpage.o
 obj-$(CONFIG_ARM64_PTDUMP)	+= dump.o
+
+KASAN_SANITIZE_kasan_init.o	:= n
+obj-$(CONFIG_KASAN)		+= kasan_init.o
diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
new file mode 100644
index 0000000..df537da
--- /dev/null
+++ b/arch/arm64/mm/kasan_init.c
@@ -0,0 +1,211 @@
+#include <linux/kasan.h>
+#include <linux/kernel.h>
+#include <linux/memblock.h>
+#include <linux/start_kernel.h>
+
+#include <asm/page.h>
+#include <asm/pgtable.h>
+#include <asm/tlbflush.h>
+
+static char kasan_zero_page[PAGE_SIZE] __page_aligned_bss;
+static pgd_t tmp_page_table[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
+
+#if CONFIG_PGTABLE_LEVELS > 3
+static pud_t kasan_zero_pud[PTRS_PER_PUD] __page_aligned_bss;
+#endif
+#if CONFIG_PGTABLE_LEVELS > 2
+static pmd_t kasan_zero_pmd[PTRS_PER_PMD] __page_aligned_bss;
+#endif
+static pte_t kasan_zero_pte[PTRS_PER_PTE] __page_aligned_bss;
+
+static void __init init_kasan_page_tables(void)
+{
+	int i;
+
+#if CONFIG_PGTABLE_LEVELS > 3
+	for (i = 0; i < PTRS_PER_PUD; i++)
+		set_pud(&kasan_zero_pud[i], __pud(__pa(kasan_zero_pmd)
+							| PAGE_KERNEL));
+#endif
+#if CONFIG_PGTABLE_LEVELS > 2
+	for (i = 0; i < PTRS_PER_PMD; i++)
+		set_pmd(&kasan_zero_pmd[i], __pmd(__pa(kasan_zero_pte)
+							| PAGE_KERNEL));
+#endif
+	for (i = 0; i < PTRS_PER_PTE; i++)
+		set_pte(&kasan_zero_pte[i], __pte(__pa(kasan_zero_page)
+							| PAGE_KERNEL));
+}
+
+void __init kasan_map_early_shadow(pgd_t *pgdp)
+{
+	int i;
+	unsigned long start = KASAN_SHADOW_START;
+	unsigned long end = KASAN_SHADOW_END;
+	pgd_t pgd;
+
+#if CONFIG_PGTABLE_LEVELS > 3
+	pgd = __pgd(__pa(kasan_zero_pud) | PAGE_KERNEL);
+#elif CONFIG_PGTABLE_LEVELS > 2
+	pgd = __pgd(__pa(kasan_zero_pmd) | PAGE_KERNEL);
+#else
+	pgd = __pgd(__pa(kasan_zero_pte) | PAGE_KERNEL);
+#endif
+
+	for (i = pgd_index(start); start < end; i++) {
+		set_pgd(&pgdp[i], pgd);
+		start += PGDIR_SIZE;
+	}
+}
+
+void __init kasan_early_init(void)
+{
+	init_kasan_page_tables();
+	kasan_map_early_shadow(swapper_pg_dir);
+	kasan_map_early_shadow(idmap_pg_dir);
+	flush_tlb_all();
+	start_kernel();
+}
+
+static void __init clear_pgds(unsigned long start,
+			unsigned long end)
+{
+	for (; start && start < end; start += PGDIR_SIZE)
+		set_pgd(pgd_offset_k(start), __pgd(0));
+}
+
+static int __init zero_pte_populate(pmd_t *pmd, unsigned long addr,
+				unsigned long end)
+{
+	pte_t *pte = pte_offset_kernel(pmd, addr);
+
+	while (addr + PAGE_SIZE <= end) {
+		set_pte(pte, __pte(__pa(kasan_zero_page)
+					| PAGE_KERNEL_RO));
+		addr += PAGE_SIZE;
+		pte = pte_offset_kernel(pmd, addr);
+	}
+	return 0;
+}
+
+static int __init zero_pmd_populate(pud_t *pud, unsigned long addr,
+				unsigned long end)
+{
+	int ret = 0;
+	pmd_t *pmd = pmd_offset(pud, addr);
+
+	while (IS_ALIGNED(addr, PMD_SIZE) && addr + PMD_SIZE <= end) {
+		set_pmd(pmd, __pmd(__pa(kasan_zero_pte)
+					| PAGE_KERNEL_RO));
+		addr += PMD_SIZE;
+		pmd++;
+	}
+
+	if (addr < end) {
+		if (pmd_none(*pmd)) {
+			void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
+			if (!p)
+				return -ENOMEM;
+			set_pmd(pmd, __pmd(__pa(p) | PAGE_KERNEL));
+		}
+		ret = zero_pte_populate(pmd, addr, end);
+	}
+	return ret;
+}
+
+static int __init zero_pud_populate(pgd_t *pgd, unsigned long addr,
+				unsigned long end)
+{
+	int ret = 0;
+	pud_t *pud = pud_offset(pgd, addr);
+
+#if CONFIG_PGTABLE_LEVELS > 2
+	while (IS_ALIGNED(addr, PUD_SIZE) && addr + PUD_SIZE <= end) {
+		set_pud(pud, __pud(__pa(kasan_zero_pmd)
+					| PAGE_KERNEL_RO));
+		addr += PUD_SIZE;
+		pud++;
+	}
+#endif
+
+	if (addr < end) {
+		if (pud_none(*pud)) {
+			void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
+			if (!p)
+				return -ENOMEM;
+			set_pud(pud, __pud(__pa(p) | PAGE_KERNEL));
+		}
+		ret = zero_pmd_populate(pud, addr, end);
+	}
+	return ret;
+}
+
+static int __init zero_pgd_populate(unsigned long addr, unsigned long end)
+{
+	int ret = 0;
+	pgd_t *pgd = pgd_offset_k(addr);
+
+#if CONFIG_PGTABLE_LEVELS > 3
+	 while (IS_ALIGNED(addr, PGDIR_SIZE) && addr + PGDIR_SIZE <= end) {
+		set_pgd(pgd, __pgd(__pa(kasan_zero_pud)
+					| PAGE_KERNEL_RO));
+		addr += PGDIR_SIZE;
+		pgd++;
+	}
+#endif
+
+	 if (addr < end) {
+		 if (pgd_none(*pgd)) {
+			 void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
+			 if (!p)
+				 return -ENOMEM;
+			 set_pgd(pgd, __pgd(__pa(p) | PAGE_KERNEL));
+		 }
+		 ret = zero_pud_populate(pgd, addr, end);
+	 }
+	 return ret;
+}
+
+static void __init populate_zero_shadow(unsigned long start, unsigned long end)
+{
+	if (zero_pgd_populate(start, end))
+		panic("kasan: unable to map zero shadow!");
+}
+
+static void cpu_set_ttbr1(unsigned long ttbr1)
+{
+	asm(
+	"	msr	ttbr1_el1, %0\n"
+	"	isb"
+	:
+	: "r" (ttbr1));
+}
+
+void __init kasan_init(void)
+{
+	struct memblock_region *reg;
+
+	memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
+	cpu_set_ttbr1(__pa(tmp_page_table));
+
+	clear_pgds(KASAN_SHADOW_START, KASAN_SHADOW_END);
+
+	populate_zero_shadow(KASAN_SHADOW_START,
+			(unsigned long)kasan_mem_to_shadow((void *)MODULES_VADDR));
+
+	for_each_memblock(memory, reg) {
+		void *start = (void *)__phys_to_virt(reg->base);
+		void *end = (void *)__phys_to_virt(reg->base + reg->size);
+
+		if (start >= end)
+			break;
+
+		vmemmap_populate((unsigned long)kasan_mem_to_shadow(start),
+				(unsigned long)kasan_mem_to_shadow(end),
+				pfn_to_nid(virt_to_pfn(start)));
+	}
+
+	memset(kasan_zero_page, 0, PAGE_SIZE);
+	cpu_set_ttbr1(__pa(swapper_pg_dir));
+	init_task.kasan_depth = 0;
+}
-- 
2.3.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [PATCH 2/2] arm64: add KASan support
@ 2015-03-24 14:49   ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-03-24 14:49 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds arch specific code for kernel address sanitizer
(see Documentation/kasan.txt).

1/8 of kernel addresses reserved for shadow memory. There was no
big enough hole for this, so virtual addresses for shadow were
stolen from vmalloc area.

At early boot stage the whole shadow region populated with just
one physical page (kasan_zero_page). Later, this page reused
as readonly zero shadow for some memory that KASan currently
don't track (vmalloc).
After mapping the physical memory, pages for shadow memory are
allocated and mapped.

KASan's stack instrumentation significantly increases stack's
consumption, so CONFIG_KASAN doubles THREAD_SIZE.

Functions like memset/memmove/memcpy do a lot of memory accesses.
If bad pointer passed to one of these function it is important
to catch this. Compiler's instrumentation cannot do this since
these functions are written in assembly.
KASan replaces memory functions with manually instrumented variants.
Original functions declared as weak symbols so strong definitions
in mm/kasan/kasan.c could replace them. Original functions have aliases
with '__' prefix in name, so we could call non-instrumented variant
if needed.
Some files built without kasan instrumentation (e.g. mm/slub.c).
Original mem* function replaced (via #define) with prefixed variants
to disable memory access checks for such files.

Signed-off-by: Andrey Ryabinin <a.ryabinin@samsung.com>
---
 arch/arm64/Kconfig                   |   7 ++
 arch/arm64/include/asm/pgtable.h     |   3 +-
 arch/arm64/include/asm/string.h      |  16 +++
 arch/arm64/include/asm/thread_info.h |   8 ++
 arch/arm64/kernel/head.S             |   3 +
 arch/arm64/kernel/module.c           |  16 ++-
 arch/arm64/kernel/setup.c            |   2 +
 arch/arm64/lib/memcpy.S              |   3 +
 arch/arm64/lib/memmove.S             |   7 +-
 arch/arm64/lib/memset.S              |   3 +
 arch/arm64/mm/Makefile               |   3 +
 arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
 12 files changed, 276 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm64/mm/kasan_init.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 4085df1..10bbd71 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -41,6 +41,7 @@ config ARM64
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_ARCH_BITREVERSE
 	select HAVE_ARCH_JUMP_LABEL
+	select HAVE_ARCH_KASAN if SPARSEMEM_VMEMMAP
 	select HAVE_ARCH_KGDB
 	select HAVE_ARCH_SECCOMP_FILTER
 	select HAVE_ARCH_TRACEHOOK
@@ -116,6 +117,12 @@ config GENERIC_CSUM
 config GENERIC_CALIBRATE_DELAY
 	def_bool y
 
+config KASAN_SHADOW_OFFSET
+	hex
+	default 0xdfff200000000000 if ARM64_VA_BITS_48
+	default 0xdffffc8000000000 if ARM64_VA_BITS_42
+	default 0xdfffff9000000000 if ARM64_VA_BITS_39
+
 config ZONE_DMA
 	def_bool y
 
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index bd5db28..f5ce010 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -40,7 +40,7 @@
  *	fixed mappings and modules
  */
 #define VMEMMAP_SIZE		ALIGN((1UL << (VA_BITS - PAGE_SHIFT)) * sizeof(struct page), PUD_SIZE)
-#define VMALLOC_START		(UL(0xffffffffffffffff) << VA_BITS)
+#define VMALLOC_START		((UL(0xffffffffffffffff) << VA_BITS) + (UL(1) << (VA_BITS - 3)))
 #define VMALLOC_END		(PAGE_OFFSET - PUD_SIZE - VMEMMAP_SIZE - SZ_64K)
 
 #define vmemmap			((struct page *)(VMALLOC_END + SZ_64K))
@@ -73,6 +73,7 @@ extern void __pgd_error(const char *file, int line, unsigned long val);
 
 #define PAGE_KERNEL		__pgprot(_PAGE_DEFAULT | PTE_PXN | PTE_UXN | PTE_DIRTY | PTE_WRITE)
 #define PAGE_KERNEL_EXEC	__pgprot(_PAGE_DEFAULT | PTE_UXN | PTE_DIRTY | PTE_WRITE)
+#define PAGE_KERNEL_RO		__pgprot(_PAGE_DEFAULT | PTE_PXN | PTE_UXN | PTE_DIRTY)
 
 #define PAGE_HYP		__pgprot(_PAGE_DEFAULT | PTE_HYP)
 #define PAGE_HYP_DEVICE		__pgprot(PROT_DEVICE_nGnRE | PTE_HYP)
diff --git a/arch/arm64/include/asm/string.h b/arch/arm64/include/asm/string.h
index 64d2d48..bff522c 100644
--- a/arch/arm64/include/asm/string.h
+++ b/arch/arm64/include/asm/string.h
@@ -36,17 +36,33 @@ extern __kernel_size_t strnlen(const char *, __kernel_size_t);
 
 #define __HAVE_ARCH_MEMCPY
 extern void *memcpy(void *, const void *, __kernel_size_t);
+extern void *__memcpy(void *, const void *, __kernel_size_t);
 
 #define __HAVE_ARCH_MEMMOVE
 extern void *memmove(void *, const void *, __kernel_size_t);
+extern void *__memmove(void *, const void *, __kernel_size_t);
 
 #define __HAVE_ARCH_MEMCHR
 extern void *memchr(const void *, int, __kernel_size_t);
 
 #define __HAVE_ARCH_MEMSET
 extern void *memset(void *, int, __kernel_size_t);
+extern void *__memset(void *, int, __kernel_size_t);
 
 #define __HAVE_ARCH_MEMCMP
 extern int memcmp(const void *, const void *, size_t);
 
+
+#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
+
+/*
+ * For files that not instrumented (e.g. mm/slub.c) we
+ * should use not instrumented version of mem* functions.
+ */
+
+#define memcpy(dst, src, len) __memcpy(dst, src, len)
+#define memmove(dst, src, len) __memmove(dst, src, len)
+#define memset(s, c, n) __memset(s, c, n)
+#endif
+
 #endif
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 702e1e6..4a2c11e 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -24,10 +24,18 @@
 #include <linux/compiler.h>
 
 #ifndef CONFIG_ARM64_64K_PAGES
+#ifndef CONFIG_KASAN
 #define THREAD_SIZE_ORDER	2
+#else
+#define THREAD_SIZE_ORDER	3
+#endif
 #endif
 
+#ifndef CONFIG_KASAN
 #define THREAD_SIZE		16384
+#else
+#define THREAD_SIZE		32768
+#endif
 #define THREAD_START_SP		(THREAD_SIZE - 16)
 
 #ifndef __ASSEMBLY__
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 51c9811..1a99e95 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -482,6 +482,9 @@ __mmap_switched:
 	str_l	x21, __fdt_pointer, x5		// Save FDT pointer
 	str_l	x24, memstart_addr, x6		// Save PHYS_OFFSET
 	mov	x29, #0
+#ifdef CONFIG_KASAN
+	b kasan_early_init
+#endif
 	b	start_kernel
 ENDPROC(__mmap_switched)
 
diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
index 67bf410..7d90c0f 100644
--- a/arch/arm64/kernel/module.c
+++ b/arch/arm64/kernel/module.c
@@ -21,6 +21,7 @@
 #include <linux/bitops.h>
 #include <linux/elf.h>
 #include <linux/gfp.h>
+#include <linux/kasan.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/moduleloader.h>
@@ -34,9 +35,18 @@
 
 void *module_alloc(unsigned long size)
 {
-	return __vmalloc_node_range(size, 1, MODULES_VADDR, MODULES_END,
-				    GFP_KERNEL, PAGE_KERNEL_EXEC, 0,
-				    NUMA_NO_NODE, __builtin_return_address(0));
+	void *p;
+
+	p = __vmalloc_node_range(size, MODULE_ALIGN, MODULES_VADDR, MODULES_END,
+				GFP_KERNEL, PAGE_KERNEL_EXEC, 0,
+				NUMA_NO_NODE, __builtin_return_address(0));
+
+	if (p && (kasan_module_alloc(p, size) < 0)) {
+		vfree(p);
+		return NULL;
+	}
+
+	return p;
 }
 
 enum aarch64_reloc_op {
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 51ef972..f197f45 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -31,6 +31,7 @@
 #include <linux/screen_info.h>
 #include <linux/init.h>
 #include <linux/kexec.h>
+#include <linux/kasan.h>
 #include <linux/crash_dump.h>
 #include <linux/root_dev.h>
 #include <linux/clk-provider.h>
@@ -396,6 +397,7 @@ void __init setup_arch(char **cmdline_p)
 	arm64_memblock_init();
 
 	paging_init();
+	kasan_init();
 	request_standard_resources();
 
 	early_ioremap_reset();
diff --git a/arch/arm64/lib/memcpy.S b/arch/arm64/lib/memcpy.S
index 8a9a96d..845e40a 100644
--- a/arch/arm64/lib/memcpy.S
+++ b/arch/arm64/lib/memcpy.S
@@ -56,6 +56,8 @@ C_h	.req	x12
 D_l	.req	x13
 D_h	.req	x14
 
+.weak memcpy
+ENTRY(__memcpy)
 ENTRY(memcpy)
 	mov	dst, dstin
 	cmp	count, #16
@@ -199,3 +201,4 @@ ENTRY(memcpy)
 	b.ne	.Ltail63
 	ret
 ENDPROC(memcpy)
+ENDPROC(__memcpy)
diff --git a/arch/arm64/lib/memmove.S b/arch/arm64/lib/memmove.S
index 57b19ea..48074d2 100644
--- a/arch/arm64/lib/memmove.S
+++ b/arch/arm64/lib/memmove.S
@@ -57,12 +57,14 @@ C_h	.req	x12
 D_l	.req	x13
 D_h	.req	x14
 
+.weak memmove
+ENTRY(__memmove)
 ENTRY(memmove)
 	cmp	dstin, src
-	b.lo	memcpy
+	b.lo	__memcpy
 	add	tmp1, src, count
 	cmp	dstin, tmp1
-	b.hs	memcpy		/* No overlap.  */
+	b.hs	__memcpy		/* No overlap.  */
 
 	add	dst, dstin, count
 	add	src, src, count
@@ -195,3 +197,4 @@ ENTRY(memmove)
 	b.ne	.Ltail63
 	ret
 ENDPROC(memmove)
+ENDPROC(__memmove)
diff --git a/arch/arm64/lib/memset.S b/arch/arm64/lib/memset.S
index 7c72dfd..4ab2594 100644
--- a/arch/arm64/lib/memset.S
+++ b/arch/arm64/lib/memset.S
@@ -54,6 +54,8 @@ dst		.req	x8
 tmp3w		.req	w9
 tmp3		.req	x9
 
+.weak memset
+ENTRY(__memset)
 ENTRY(memset)
 	mov	dst, dstin	/* Preserve return value.  */
 	and	A_lw, val, #255
@@ -214,3 +216,4 @@ ENTRY(memset)
 	b.ne	.Ltail_maybe_long
 	ret
 ENDPROC(memset)
+ENDPROC(__memset)
diff --git a/arch/arm64/mm/Makefile b/arch/arm64/mm/Makefile
index 773d37a..e17703c 100644
--- a/arch/arm64/mm/Makefile
+++ b/arch/arm64/mm/Makefile
@@ -4,3 +4,6 @@ obj-y				:= dma-mapping.o extable.o fault.o init.o \
 				   context.o proc.o pageattr.o
 obj-$(CONFIG_HUGETLB_PAGE)	+= hugetlbpage.o
 obj-$(CONFIG_ARM64_PTDUMP)	+= dump.o
+
+KASAN_SANITIZE_kasan_init.o	:= n
+obj-$(CONFIG_KASAN)		+= kasan_init.o
diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
new file mode 100644
index 0000000..df537da
--- /dev/null
+++ b/arch/arm64/mm/kasan_init.c
@@ -0,0 +1,211 @@
+#include <linux/kasan.h>
+#include <linux/kernel.h>
+#include <linux/memblock.h>
+#include <linux/start_kernel.h>
+
+#include <asm/page.h>
+#include <asm/pgtable.h>
+#include <asm/tlbflush.h>
+
+static char kasan_zero_page[PAGE_SIZE] __page_aligned_bss;
+static pgd_t tmp_page_table[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
+
+#if CONFIG_PGTABLE_LEVELS > 3
+static pud_t kasan_zero_pud[PTRS_PER_PUD] __page_aligned_bss;
+#endif
+#if CONFIG_PGTABLE_LEVELS > 2
+static pmd_t kasan_zero_pmd[PTRS_PER_PMD] __page_aligned_bss;
+#endif
+static pte_t kasan_zero_pte[PTRS_PER_PTE] __page_aligned_bss;
+
+static void __init init_kasan_page_tables(void)
+{
+	int i;
+
+#if CONFIG_PGTABLE_LEVELS > 3
+	for (i = 0; i < PTRS_PER_PUD; i++)
+		set_pud(&kasan_zero_pud[i], __pud(__pa(kasan_zero_pmd)
+							| PAGE_KERNEL));
+#endif
+#if CONFIG_PGTABLE_LEVELS > 2
+	for (i = 0; i < PTRS_PER_PMD; i++)
+		set_pmd(&kasan_zero_pmd[i], __pmd(__pa(kasan_zero_pte)
+							| PAGE_KERNEL));
+#endif
+	for (i = 0; i < PTRS_PER_PTE; i++)
+		set_pte(&kasan_zero_pte[i], __pte(__pa(kasan_zero_page)
+							| PAGE_KERNEL));
+}
+
+void __init kasan_map_early_shadow(pgd_t *pgdp)
+{
+	int i;
+	unsigned long start = KASAN_SHADOW_START;
+	unsigned long end = KASAN_SHADOW_END;
+	pgd_t pgd;
+
+#if CONFIG_PGTABLE_LEVELS > 3
+	pgd = __pgd(__pa(kasan_zero_pud) | PAGE_KERNEL);
+#elif CONFIG_PGTABLE_LEVELS > 2
+	pgd = __pgd(__pa(kasan_zero_pmd) | PAGE_KERNEL);
+#else
+	pgd = __pgd(__pa(kasan_zero_pte) | PAGE_KERNEL);
+#endif
+
+	for (i = pgd_index(start); start < end; i++) {
+		set_pgd(&pgdp[i], pgd);
+		start += PGDIR_SIZE;
+	}
+}
+
+void __init kasan_early_init(void)
+{
+	init_kasan_page_tables();
+	kasan_map_early_shadow(swapper_pg_dir);
+	kasan_map_early_shadow(idmap_pg_dir);
+	flush_tlb_all();
+	start_kernel();
+}
+
+static void __init clear_pgds(unsigned long start,
+			unsigned long end)
+{
+	for (; start && start < end; start += PGDIR_SIZE)
+		set_pgd(pgd_offset_k(start), __pgd(0));
+}
+
+static int __init zero_pte_populate(pmd_t *pmd, unsigned long addr,
+				unsigned long end)
+{
+	pte_t *pte = pte_offset_kernel(pmd, addr);
+
+	while (addr + PAGE_SIZE <= end) {
+		set_pte(pte, __pte(__pa(kasan_zero_page)
+					| PAGE_KERNEL_RO));
+		addr += PAGE_SIZE;
+		pte = pte_offset_kernel(pmd, addr);
+	}
+	return 0;
+}
+
+static int __init zero_pmd_populate(pud_t *pud, unsigned long addr,
+				unsigned long end)
+{
+	int ret = 0;
+	pmd_t *pmd = pmd_offset(pud, addr);
+
+	while (IS_ALIGNED(addr, PMD_SIZE) && addr + PMD_SIZE <= end) {
+		set_pmd(pmd, __pmd(__pa(kasan_zero_pte)
+					| PAGE_KERNEL_RO));
+		addr += PMD_SIZE;
+		pmd++;
+	}
+
+	if (addr < end) {
+		if (pmd_none(*pmd)) {
+			void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
+			if (!p)
+				return -ENOMEM;
+			set_pmd(pmd, __pmd(__pa(p) | PAGE_KERNEL));
+		}
+		ret = zero_pte_populate(pmd, addr, end);
+	}
+	return ret;
+}
+
+static int __init zero_pud_populate(pgd_t *pgd, unsigned long addr,
+				unsigned long end)
+{
+	int ret = 0;
+	pud_t *pud = pud_offset(pgd, addr);
+
+#if CONFIG_PGTABLE_LEVELS > 2
+	while (IS_ALIGNED(addr, PUD_SIZE) && addr + PUD_SIZE <= end) {
+		set_pud(pud, __pud(__pa(kasan_zero_pmd)
+					| PAGE_KERNEL_RO));
+		addr += PUD_SIZE;
+		pud++;
+	}
+#endif
+
+	if (addr < end) {
+		if (pud_none(*pud)) {
+			void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
+			if (!p)
+				return -ENOMEM;
+			set_pud(pud, __pud(__pa(p) | PAGE_KERNEL));
+		}
+		ret = zero_pmd_populate(pud, addr, end);
+	}
+	return ret;
+}
+
+static int __init zero_pgd_populate(unsigned long addr, unsigned long end)
+{
+	int ret = 0;
+	pgd_t *pgd = pgd_offset_k(addr);
+
+#if CONFIG_PGTABLE_LEVELS > 3
+	 while (IS_ALIGNED(addr, PGDIR_SIZE) && addr + PGDIR_SIZE <= end) {
+		set_pgd(pgd, __pgd(__pa(kasan_zero_pud)
+					| PAGE_KERNEL_RO));
+		addr += PGDIR_SIZE;
+		pgd++;
+	}
+#endif
+
+	 if (addr < end) {
+		 if (pgd_none(*pgd)) {
+			 void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
+			 if (!p)
+				 return -ENOMEM;
+			 set_pgd(pgd, __pgd(__pa(p) | PAGE_KERNEL));
+		 }
+		 ret = zero_pud_populate(pgd, addr, end);
+	 }
+	 return ret;
+}
+
+static void __init populate_zero_shadow(unsigned long start, unsigned long end)
+{
+	if (zero_pgd_populate(start, end))
+		panic("kasan: unable to map zero shadow!");
+}
+
+static void cpu_set_ttbr1(unsigned long ttbr1)
+{
+	asm(
+	"	msr	ttbr1_el1, %0\n"
+	"	isb"
+	:
+	: "r" (ttbr1));
+}
+
+void __init kasan_init(void)
+{
+	struct memblock_region *reg;
+
+	memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
+	cpu_set_ttbr1(__pa(tmp_page_table));
+
+	clear_pgds(KASAN_SHADOW_START, KASAN_SHADOW_END);
+
+	populate_zero_shadow(KASAN_SHADOW_START,
+			(unsigned long)kasan_mem_to_shadow((void *)MODULES_VADDR));
+
+	for_each_memblock(memory, reg) {
+		void *start = (void *)__phys_to_virt(reg->base);
+		void *end = (void *)__phys_to_virt(reg->base + reg->size);
+
+		if (start >= end)
+			break;
+
+		vmemmap_populate((unsigned long)kasan_mem_to_shadow(start),
+				(unsigned long)kasan_mem_to_shadow(end),
+				pfn_to_nid(virt_to_pfn(start)));
+	}
+
+	memset(kasan_zero_page, 0, PAGE_SIZE);
+	cpu_set_ttbr1(__pa(swapper_pg_dir));
+	init_task.kasan_depth = 0;
+}
-- 
2.3.3

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

* Re: [PATCH 2/2] arm64: add KASan support
  2015-03-24 14:49   ` Andrey Ryabinin
  (?)
@ 2015-04-01 12:28     ` Catalin Marinas
  -1 siblings, 0 replies; 39+ messages in thread
From: Catalin Marinas @ 2015-04-01 12:28 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: linux-arm-kernel, Will Deacon, linux-kernel, linux-mm, Andrew Morton

Hi Andrey,

On Tue, Mar 24, 2015 at 05:49:04PM +0300, Andrey Ryabinin wrote:
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 4085df1..10bbd71 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -41,6 +41,7 @@ config ARM64
>  	select HAVE_ARCH_AUDITSYSCALL
>  	select HAVE_ARCH_BITREVERSE
>  	select HAVE_ARCH_JUMP_LABEL
> +	select HAVE_ARCH_KASAN if SPARSEMEM_VMEMMAP
>  	select HAVE_ARCH_KGDB
>  	select HAVE_ARCH_SECCOMP_FILTER
>  	select HAVE_ARCH_TRACEHOOK
> @@ -116,6 +117,12 @@ config GENERIC_CSUM
>  config GENERIC_CALIBRATE_DELAY
>  	def_bool y
>  
> +config KASAN_SHADOW_OFFSET
> +	hex
> +	default 0xdfff200000000000 if ARM64_VA_BITS_48
> +	default 0xdffffc8000000000 if ARM64_VA_BITS_42
> +	default 0xdfffff9000000000 if ARM64_VA_BITS_39

Can we compute these at build time in some C header? Or they need to be
passed to gcc when compiling the kernel so that it generates the right
instrumentation?

I'm not familiar with KASan but is the offset address supposed to be
accessible? The addresses encoded above would always generate a fault
(level 0 / address size fault).

> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index bd5db28..f5ce010 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -40,7 +40,7 @@
>   *	fixed mappings and modules
>   */
>  #define VMEMMAP_SIZE		ALIGN((1UL << (VA_BITS - PAGE_SHIFT)) * sizeof(struct page), PUD_SIZE)
> -#define VMALLOC_START		(UL(0xffffffffffffffff) << VA_BITS)
> +#define VMALLOC_START		((UL(0xffffffffffffffff) << VA_BITS) + (UL(1) << (VA_BITS - 3)))

I assume that's where you want to make room for KASan? Some comments and
macros would be useful for why this is needed and how it is calculated.
It also needs to be disabled when KASan is not enabled.

> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 51c9811..1a99e95 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -482,6 +482,9 @@ __mmap_switched:
>  	str_l	x21, __fdt_pointer, x5		// Save FDT pointer
>  	str_l	x24, memstart_addr, x6		// Save PHYS_OFFSET
>  	mov	x29, #0
> +#ifdef CONFIG_KASAN
> +	b kasan_early_init
> +#endif

Nitpick: tab between b and kasan_early_init.

> diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
> new file mode 100644
> index 0000000..df537da
> --- /dev/null
> +++ b/arch/arm64/mm/kasan_init.c
> @@ -0,0 +1,211 @@
> +#include <linux/kasan.h>
> +#include <linux/kernel.h>
> +#include <linux/memblock.h>
> +#include <linux/start_kernel.h>
> +
> +#include <asm/page.h>
> +#include <asm/pgtable.h>
> +#include <asm/tlbflush.h>
> +
> +static char kasan_zero_page[PAGE_SIZE] __page_aligned_bss;

Can we not use the system's zero_page or it's not initialised yet?

> +static pgd_t tmp_page_table[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
> +
> +#if CONFIG_PGTABLE_LEVELS > 3
> +static pud_t kasan_zero_pud[PTRS_PER_PUD] __page_aligned_bss;
> +#endif
> +#if CONFIG_PGTABLE_LEVELS > 2
> +static pmd_t kasan_zero_pmd[PTRS_PER_PMD] __page_aligned_bss;
> +#endif
> +static pte_t kasan_zero_pte[PTRS_PER_PTE] __page_aligned_bss;
> +
> +static void __init init_kasan_page_tables(void)
> +{
> +	int i;
> +
> +#if CONFIG_PGTABLE_LEVELS > 3
> +	for (i = 0; i < PTRS_PER_PUD; i++)
> +		set_pud(&kasan_zero_pud[i], __pud(__pa(kasan_zero_pmd)
> +							| PAGE_KERNEL));
> +#endif
> +#if CONFIG_PGTABLE_LEVELS > 2
> +	for (i = 0; i < PTRS_PER_PMD; i++)
> +		set_pmd(&kasan_zero_pmd[i], __pmd(__pa(kasan_zero_pte)
> +							| PAGE_KERNEL));
> +#endif

These don't look right. You are setting page attributes on table
entries. You should use the standard pmd_populate etc. macros here, see
early_fixmap_init() as an example.

> +	for (i = 0; i < PTRS_PER_PTE; i++)
> +		set_pte(&kasan_zero_pte[i], __pte(__pa(kasan_zero_page)
> +							| PAGE_KERNEL));

PAGE_KERNEL is pgprot_t, so you mix the types here. Just do something
like:

	set_pte(..., pfn_pte(zero_pfn, PAGE_KERNEL_RO));

(shouldn't it be read-only?)

> +void __init kasan_map_early_shadow(pgd_t *pgdp)
> +{
> +	int i;
> +	unsigned long start = KASAN_SHADOW_START;
> +	unsigned long end = KASAN_SHADOW_END;
> +	pgd_t pgd;
> +
> +#if CONFIG_PGTABLE_LEVELS > 3
> +	pgd = __pgd(__pa(kasan_zero_pud) | PAGE_KERNEL);
> +#elif CONFIG_PGTABLE_LEVELS > 2
> +	pgd = __pgd(__pa(kasan_zero_pmd) | PAGE_KERNEL);
> +#else
> +	pgd = __pgd(__pa(kasan_zero_pte) | PAGE_KERNEL);
> +#endif
> +
> +	for (i = pgd_index(start); start < end; i++) {
> +		set_pgd(&pgdp[i], pgd);
> +		start += PGDIR_SIZE;
> +	}
> +}

Same problem as above with PAGE_KERNEL. You should just use
pgd_populate().

> +
> +void __init kasan_early_init(void)
> +{
> +	init_kasan_page_tables();
> +	kasan_map_early_shadow(swapper_pg_dir);
> +	kasan_map_early_shadow(idmap_pg_dir);
> +	flush_tlb_all();
> +	start_kernel();
> +}

Why do you need to map the kasan page tables into the idmap?

> +
> +static void __init clear_pgds(unsigned long start,
> +			unsigned long end)
> +{
> +	for (; start && start < end; start += PGDIR_SIZE)
> +		set_pgd(pgd_offset_k(start), __pgd(0));
> +}

We have dedicated pgd_clear() macro.

> +
> +static int __init zero_pte_populate(pmd_t *pmd, unsigned long addr,
> +				unsigned long end)
> +{
> +	pte_t *pte = pte_offset_kernel(pmd, addr);
> +
> +	while (addr + PAGE_SIZE <= end) {
> +		set_pte(pte, __pte(__pa(kasan_zero_page)
> +					| PAGE_KERNEL_RO));

See above for a pfn_pte() usage.

> +		addr += PAGE_SIZE;
> +		pte = pte_offset_kernel(pmd, addr);
> +	}
> +	return 0;
> +}
> +
> +static int __init zero_pmd_populate(pud_t *pud, unsigned long addr,
> +				unsigned long end)
> +{
> +	int ret = 0;
> +	pmd_t *pmd = pmd_offset(pud, addr);
> +
> +	while (IS_ALIGNED(addr, PMD_SIZE) && addr + PMD_SIZE <= end) {
> +		set_pmd(pmd, __pmd(__pa(kasan_zero_pte)
> +					| PAGE_KERNEL_RO));
> +		addr += PMD_SIZE;
> +		pmd++;
> +	}
> +
> +	if (addr < end) {
> +		if (pmd_none(*pmd)) {
> +			void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
> +			if (!p)
> +				return -ENOMEM;
> +			set_pmd(pmd, __pmd(__pa(p) | PAGE_KERNEL));
> +		}
> +		ret = zero_pte_populate(pmd, addr, end);
> +	}
> +	return ret;
> +}
> +
> +static int __init zero_pud_populate(pgd_t *pgd, unsigned long addr,
> +				unsigned long end)
> +{
> +	int ret = 0;
> +	pud_t *pud = pud_offset(pgd, addr);
> +
> +#if CONFIG_PGTABLE_LEVELS > 2
> +	while (IS_ALIGNED(addr, PUD_SIZE) && addr + PUD_SIZE <= end) {
> +		set_pud(pud, __pud(__pa(kasan_zero_pmd)
> +					| PAGE_KERNEL_RO));
> +		addr += PUD_SIZE;
> +		pud++;
> +	}
> +#endif
> +
> +	if (addr < end) {
> +		if (pud_none(*pud)) {
> +			void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
> +			if (!p)
> +				return -ENOMEM;
> +			set_pud(pud, __pud(__pa(p) | PAGE_KERNEL));
> +		}
> +		ret = zero_pmd_populate(pud, addr, end);
> +	}
> +	return ret;
> +}
> +
> +static int __init zero_pgd_populate(unsigned long addr, unsigned long end)
> +{
> +	int ret = 0;
> +	pgd_t *pgd = pgd_offset_k(addr);
> +
> +#if CONFIG_PGTABLE_LEVELS > 3
> +	 while (IS_ALIGNED(addr, PGDIR_SIZE) && addr + PGDIR_SIZE <= end) {
> +		set_pgd(pgd, __pgd(__pa(kasan_zero_pud)
> +					| PAGE_KERNEL_RO));
> +		addr += PGDIR_SIZE;
> +		pgd++;
> +	}
> +#endif

All these PAGE_KERNEL_RO on table entries are wrong. Please use the
standard pgd/pud/pmd_populate macros.

As for the while loops above, we have a standard way to avoid the
#ifdef's by using pgd_addr_end() etc. See __create_mapping() as an
example, there are a few others throughout the kernel.

> +
> +	 if (addr < end) {
> +		 if (pgd_none(*pgd)) {
> +			 void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
> +			 if (!p)
> +				 return -ENOMEM;
> +			 set_pgd(pgd, __pgd(__pa(p) | PAGE_KERNEL));

I'm just commenting here but it applies to the previous functions. You
may be able to use functions like vmmemap_pgd_populate() which look very
similar (and they also use pgd_populate instead of the set_pgd).

> +static void cpu_set_ttbr1(unsigned long ttbr1)
> +{
> +	asm(
> +	"	msr	ttbr1_el1, %0\n"
> +	"	isb"
> +	:
> +	: "r" (ttbr1));
> +}
> +
> +void __init kasan_init(void)
> +{
> +	struct memblock_region *reg;
> +
> +	memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
> +	cpu_set_ttbr1(__pa(tmp_page_table));

Why is this needed? The code lacks comments in several places but here I
couldn't figure out what the point is.

> +
> +	clear_pgds(KASAN_SHADOW_START, KASAN_SHADOW_END);
> +
> +	populate_zero_shadow(KASAN_SHADOW_START,
> +			(unsigned long)kasan_mem_to_shadow((void *)MODULES_VADDR));
> +
> +	for_each_memblock(memory, reg) {
> +		void *start = (void *)__phys_to_virt(reg->base);
> +		void *end = (void *)__phys_to_virt(reg->base + reg->size);
> +
> +		if (start >= end)
> +			break;
> +
> +		vmemmap_populate((unsigned long)kasan_mem_to_shadow(start),
> +				(unsigned long)kasan_mem_to_shadow(end),
> +				pfn_to_nid(virt_to_pfn(start)));
> +	}
> +
> +	memset(kasan_zero_page, 0, PAGE_SIZE);
> +	cpu_set_ttbr1(__pa(swapper_pg_dir));
> +	init_task.kasan_depth = 0;
> +}

-- 
Catalin

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

* Re: [PATCH 2/2] arm64: add KASan support
@ 2015-04-01 12:28     ` Catalin Marinas
  0 siblings, 0 replies; 39+ messages in thread
From: Catalin Marinas @ 2015-04-01 12:28 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: linux-arm-kernel, Will Deacon, linux-kernel, linux-mm, Andrew Morton

Hi Andrey,

On Tue, Mar 24, 2015 at 05:49:04PM +0300, Andrey Ryabinin wrote:
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 4085df1..10bbd71 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -41,6 +41,7 @@ config ARM64
>  	select HAVE_ARCH_AUDITSYSCALL
>  	select HAVE_ARCH_BITREVERSE
>  	select HAVE_ARCH_JUMP_LABEL
> +	select HAVE_ARCH_KASAN if SPARSEMEM_VMEMMAP
>  	select HAVE_ARCH_KGDB
>  	select HAVE_ARCH_SECCOMP_FILTER
>  	select HAVE_ARCH_TRACEHOOK
> @@ -116,6 +117,12 @@ config GENERIC_CSUM
>  config GENERIC_CALIBRATE_DELAY
>  	def_bool y
>  
> +config KASAN_SHADOW_OFFSET
> +	hex
> +	default 0xdfff200000000000 if ARM64_VA_BITS_48
> +	default 0xdffffc8000000000 if ARM64_VA_BITS_42
> +	default 0xdfffff9000000000 if ARM64_VA_BITS_39

Can we compute these at build time in some C header? Or they need to be
passed to gcc when compiling the kernel so that it generates the right
instrumentation?

I'm not familiar with KASan but is the offset address supposed to be
accessible? The addresses encoded above would always generate a fault
(level 0 / address size fault).

> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index bd5db28..f5ce010 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -40,7 +40,7 @@
>   *	fixed mappings and modules
>   */
>  #define VMEMMAP_SIZE		ALIGN((1UL << (VA_BITS - PAGE_SHIFT)) * sizeof(struct page), PUD_SIZE)
> -#define VMALLOC_START		(UL(0xffffffffffffffff) << VA_BITS)
> +#define VMALLOC_START		((UL(0xffffffffffffffff) << VA_BITS) + (UL(1) << (VA_BITS - 3)))

I assume that's where you want to make room for KASan? Some comments and
macros would be useful for why this is needed and how it is calculated.
It also needs to be disabled when KASan is not enabled.

> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 51c9811..1a99e95 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -482,6 +482,9 @@ __mmap_switched:
>  	str_l	x21, __fdt_pointer, x5		// Save FDT pointer
>  	str_l	x24, memstart_addr, x6		// Save PHYS_OFFSET
>  	mov	x29, #0
> +#ifdef CONFIG_KASAN
> +	b kasan_early_init
> +#endif

Nitpick: tab between b and kasan_early_init.

> diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
> new file mode 100644
> index 0000000..df537da
> --- /dev/null
> +++ b/arch/arm64/mm/kasan_init.c
> @@ -0,0 +1,211 @@
> +#include <linux/kasan.h>
> +#include <linux/kernel.h>
> +#include <linux/memblock.h>
> +#include <linux/start_kernel.h>
> +
> +#include <asm/page.h>
> +#include <asm/pgtable.h>
> +#include <asm/tlbflush.h>
> +
> +static char kasan_zero_page[PAGE_SIZE] __page_aligned_bss;

Can we not use the system's zero_page or it's not initialised yet?

> +static pgd_t tmp_page_table[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
> +
> +#if CONFIG_PGTABLE_LEVELS > 3
> +static pud_t kasan_zero_pud[PTRS_PER_PUD] __page_aligned_bss;
> +#endif
> +#if CONFIG_PGTABLE_LEVELS > 2
> +static pmd_t kasan_zero_pmd[PTRS_PER_PMD] __page_aligned_bss;
> +#endif
> +static pte_t kasan_zero_pte[PTRS_PER_PTE] __page_aligned_bss;
> +
> +static void __init init_kasan_page_tables(void)
> +{
> +	int i;
> +
> +#if CONFIG_PGTABLE_LEVELS > 3
> +	for (i = 0; i < PTRS_PER_PUD; i++)
> +		set_pud(&kasan_zero_pud[i], __pud(__pa(kasan_zero_pmd)
> +							| PAGE_KERNEL));
> +#endif
> +#if CONFIG_PGTABLE_LEVELS > 2
> +	for (i = 0; i < PTRS_PER_PMD; i++)
> +		set_pmd(&kasan_zero_pmd[i], __pmd(__pa(kasan_zero_pte)
> +							| PAGE_KERNEL));
> +#endif

These don't look right. You are setting page attributes on table
entries. You should use the standard pmd_populate etc. macros here, see
early_fixmap_init() as an example.

> +	for (i = 0; i < PTRS_PER_PTE; i++)
> +		set_pte(&kasan_zero_pte[i], __pte(__pa(kasan_zero_page)
> +							| PAGE_KERNEL));

PAGE_KERNEL is pgprot_t, so you mix the types here. Just do something
like:

	set_pte(..., pfn_pte(zero_pfn, PAGE_KERNEL_RO));

(shouldn't it be read-only?)

> +void __init kasan_map_early_shadow(pgd_t *pgdp)
> +{
> +	int i;
> +	unsigned long start = KASAN_SHADOW_START;
> +	unsigned long end = KASAN_SHADOW_END;
> +	pgd_t pgd;
> +
> +#if CONFIG_PGTABLE_LEVELS > 3
> +	pgd = __pgd(__pa(kasan_zero_pud) | PAGE_KERNEL);
> +#elif CONFIG_PGTABLE_LEVELS > 2
> +	pgd = __pgd(__pa(kasan_zero_pmd) | PAGE_KERNEL);
> +#else
> +	pgd = __pgd(__pa(kasan_zero_pte) | PAGE_KERNEL);
> +#endif
> +
> +	for (i = pgd_index(start); start < end; i++) {
> +		set_pgd(&pgdp[i], pgd);
> +		start += PGDIR_SIZE;
> +	}
> +}

Same problem as above with PAGE_KERNEL. You should just use
pgd_populate().

> +
> +void __init kasan_early_init(void)
> +{
> +	init_kasan_page_tables();
> +	kasan_map_early_shadow(swapper_pg_dir);
> +	kasan_map_early_shadow(idmap_pg_dir);
> +	flush_tlb_all();
> +	start_kernel();
> +}

Why do you need to map the kasan page tables into the idmap?

> +
> +static void __init clear_pgds(unsigned long start,
> +			unsigned long end)
> +{
> +	for (; start && start < end; start += PGDIR_SIZE)
> +		set_pgd(pgd_offset_k(start), __pgd(0));
> +}

We have dedicated pgd_clear() macro.

> +
> +static int __init zero_pte_populate(pmd_t *pmd, unsigned long addr,
> +				unsigned long end)
> +{
> +	pte_t *pte = pte_offset_kernel(pmd, addr);
> +
> +	while (addr + PAGE_SIZE <= end) {
> +		set_pte(pte, __pte(__pa(kasan_zero_page)
> +					| PAGE_KERNEL_RO));

See above for a pfn_pte() usage.

> +		addr += PAGE_SIZE;
> +		pte = pte_offset_kernel(pmd, addr);
> +	}
> +	return 0;
> +}
> +
> +static int __init zero_pmd_populate(pud_t *pud, unsigned long addr,
> +				unsigned long end)
> +{
> +	int ret = 0;
> +	pmd_t *pmd = pmd_offset(pud, addr);
> +
> +	while (IS_ALIGNED(addr, PMD_SIZE) && addr + PMD_SIZE <= end) {
> +		set_pmd(pmd, __pmd(__pa(kasan_zero_pte)
> +					| PAGE_KERNEL_RO));
> +		addr += PMD_SIZE;
> +		pmd++;
> +	}
> +
> +	if (addr < end) {
> +		if (pmd_none(*pmd)) {
> +			void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
> +			if (!p)
> +				return -ENOMEM;
> +			set_pmd(pmd, __pmd(__pa(p) | PAGE_KERNEL));
> +		}
> +		ret = zero_pte_populate(pmd, addr, end);
> +	}
> +	return ret;
> +}
> +
> +static int __init zero_pud_populate(pgd_t *pgd, unsigned long addr,
> +				unsigned long end)
> +{
> +	int ret = 0;
> +	pud_t *pud = pud_offset(pgd, addr);
> +
> +#if CONFIG_PGTABLE_LEVELS > 2
> +	while (IS_ALIGNED(addr, PUD_SIZE) && addr + PUD_SIZE <= end) {
> +		set_pud(pud, __pud(__pa(kasan_zero_pmd)
> +					| PAGE_KERNEL_RO));
> +		addr += PUD_SIZE;
> +		pud++;
> +	}
> +#endif
> +
> +	if (addr < end) {
> +		if (pud_none(*pud)) {
> +			void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
> +			if (!p)
> +				return -ENOMEM;
> +			set_pud(pud, __pud(__pa(p) | PAGE_KERNEL));
> +		}
> +		ret = zero_pmd_populate(pud, addr, end);
> +	}
> +	return ret;
> +}
> +
> +static int __init zero_pgd_populate(unsigned long addr, unsigned long end)
> +{
> +	int ret = 0;
> +	pgd_t *pgd = pgd_offset_k(addr);
> +
> +#if CONFIG_PGTABLE_LEVELS > 3
> +	 while (IS_ALIGNED(addr, PGDIR_SIZE) && addr + PGDIR_SIZE <= end) {
> +		set_pgd(pgd, __pgd(__pa(kasan_zero_pud)
> +					| PAGE_KERNEL_RO));
> +		addr += PGDIR_SIZE;
> +		pgd++;
> +	}
> +#endif

All these PAGE_KERNEL_RO on table entries are wrong. Please use the
standard pgd/pud/pmd_populate macros.

As for the while loops above, we have a standard way to avoid the
#ifdef's by using pgd_addr_end() etc. See __create_mapping() as an
example, there are a few others throughout the kernel.

> +
> +	 if (addr < end) {
> +		 if (pgd_none(*pgd)) {
> +			 void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
> +			 if (!p)
> +				 return -ENOMEM;
> +			 set_pgd(pgd, __pgd(__pa(p) | PAGE_KERNEL));

I'm just commenting here but it applies to the previous functions. You
may be able to use functions like vmmemap_pgd_populate() which look very
similar (and they also use pgd_populate instead of the set_pgd).

> +static void cpu_set_ttbr1(unsigned long ttbr1)
> +{
> +	asm(
> +	"	msr	ttbr1_el1, %0\n"
> +	"	isb"
> +	:
> +	: "r" (ttbr1));
> +}
> +
> +void __init kasan_init(void)
> +{
> +	struct memblock_region *reg;
> +
> +	memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
> +	cpu_set_ttbr1(__pa(tmp_page_table));

Why is this needed? The code lacks comments in several places but here I
couldn't figure out what the point is.

> +
> +	clear_pgds(KASAN_SHADOW_START, KASAN_SHADOW_END);
> +
> +	populate_zero_shadow(KASAN_SHADOW_START,
> +			(unsigned long)kasan_mem_to_shadow((void *)MODULES_VADDR));
> +
> +	for_each_memblock(memory, reg) {
> +		void *start = (void *)__phys_to_virt(reg->base);
> +		void *end = (void *)__phys_to_virt(reg->base + reg->size);
> +
> +		if (start >= end)
> +			break;
> +
> +		vmemmap_populate((unsigned long)kasan_mem_to_shadow(start),
> +				(unsigned long)kasan_mem_to_shadow(end),
> +				pfn_to_nid(virt_to_pfn(start)));
> +	}
> +
> +	memset(kasan_zero_page, 0, PAGE_SIZE);
> +	cpu_set_ttbr1(__pa(swapper_pg_dir));
> +	init_task.kasan_depth = 0;
> +}

-- 
Catalin

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [PATCH 2/2] arm64: add KASan support
@ 2015-04-01 12:28     ` Catalin Marinas
  0 siblings, 0 replies; 39+ messages in thread
From: Catalin Marinas @ 2015-04-01 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Andrey,

On Tue, Mar 24, 2015 at 05:49:04PM +0300, Andrey Ryabinin wrote:
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 4085df1..10bbd71 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -41,6 +41,7 @@ config ARM64
>  	select HAVE_ARCH_AUDITSYSCALL
>  	select HAVE_ARCH_BITREVERSE
>  	select HAVE_ARCH_JUMP_LABEL
> +	select HAVE_ARCH_KASAN if SPARSEMEM_VMEMMAP
>  	select HAVE_ARCH_KGDB
>  	select HAVE_ARCH_SECCOMP_FILTER
>  	select HAVE_ARCH_TRACEHOOK
> @@ -116,6 +117,12 @@ config GENERIC_CSUM
>  config GENERIC_CALIBRATE_DELAY
>  	def_bool y
>  
> +config KASAN_SHADOW_OFFSET
> +	hex
> +	default 0xdfff200000000000 if ARM64_VA_BITS_48
> +	default 0xdffffc8000000000 if ARM64_VA_BITS_42
> +	default 0xdfffff9000000000 if ARM64_VA_BITS_39

Can we compute these at build time in some C header? Or they need to be
passed to gcc when compiling the kernel so that it generates the right
instrumentation?

I'm not familiar with KASan but is the offset address supposed to be
accessible? The addresses encoded above would always generate a fault
(level 0 / address size fault).

> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index bd5db28..f5ce010 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -40,7 +40,7 @@
>   *	fixed mappings and modules
>   */
>  #define VMEMMAP_SIZE		ALIGN((1UL << (VA_BITS - PAGE_SHIFT)) * sizeof(struct page), PUD_SIZE)
> -#define VMALLOC_START		(UL(0xffffffffffffffff) << VA_BITS)
> +#define VMALLOC_START		((UL(0xffffffffffffffff) << VA_BITS) + (UL(1) << (VA_BITS - 3)))

I assume that's where you want to make room for KASan? Some comments and
macros would be useful for why this is needed and how it is calculated.
It also needs to be disabled when KASan is not enabled.

> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 51c9811..1a99e95 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -482,6 +482,9 @@ __mmap_switched:
>  	str_l	x21, __fdt_pointer, x5		// Save FDT pointer
>  	str_l	x24, memstart_addr, x6		// Save PHYS_OFFSET
>  	mov	x29, #0
> +#ifdef CONFIG_KASAN
> +	b kasan_early_init
> +#endif

Nitpick: tab between b and kasan_early_init.

> diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
> new file mode 100644
> index 0000000..df537da
> --- /dev/null
> +++ b/arch/arm64/mm/kasan_init.c
> @@ -0,0 +1,211 @@
> +#include <linux/kasan.h>
> +#include <linux/kernel.h>
> +#include <linux/memblock.h>
> +#include <linux/start_kernel.h>
> +
> +#include <asm/page.h>
> +#include <asm/pgtable.h>
> +#include <asm/tlbflush.h>
> +
> +static char kasan_zero_page[PAGE_SIZE] __page_aligned_bss;

Can we not use the system's zero_page or it's not initialised yet?

> +static pgd_t tmp_page_table[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
> +
> +#if CONFIG_PGTABLE_LEVELS > 3
> +static pud_t kasan_zero_pud[PTRS_PER_PUD] __page_aligned_bss;
> +#endif
> +#if CONFIG_PGTABLE_LEVELS > 2
> +static pmd_t kasan_zero_pmd[PTRS_PER_PMD] __page_aligned_bss;
> +#endif
> +static pte_t kasan_zero_pte[PTRS_PER_PTE] __page_aligned_bss;
> +
> +static void __init init_kasan_page_tables(void)
> +{
> +	int i;
> +
> +#if CONFIG_PGTABLE_LEVELS > 3
> +	for (i = 0; i < PTRS_PER_PUD; i++)
> +		set_pud(&kasan_zero_pud[i], __pud(__pa(kasan_zero_pmd)
> +							| PAGE_KERNEL));
> +#endif
> +#if CONFIG_PGTABLE_LEVELS > 2
> +	for (i = 0; i < PTRS_PER_PMD; i++)
> +		set_pmd(&kasan_zero_pmd[i], __pmd(__pa(kasan_zero_pte)
> +							| PAGE_KERNEL));
> +#endif

These don't look right. You are setting page attributes on table
entries. You should use the standard pmd_populate etc. macros here, see
early_fixmap_init() as an example.

> +	for (i = 0; i < PTRS_PER_PTE; i++)
> +		set_pte(&kasan_zero_pte[i], __pte(__pa(kasan_zero_page)
> +							| PAGE_KERNEL));

PAGE_KERNEL is pgprot_t, so you mix the types here. Just do something
like:

	set_pte(..., pfn_pte(zero_pfn, PAGE_KERNEL_RO));

(shouldn't it be read-only?)

> +void __init kasan_map_early_shadow(pgd_t *pgdp)
> +{
> +	int i;
> +	unsigned long start = KASAN_SHADOW_START;
> +	unsigned long end = KASAN_SHADOW_END;
> +	pgd_t pgd;
> +
> +#if CONFIG_PGTABLE_LEVELS > 3
> +	pgd = __pgd(__pa(kasan_zero_pud) | PAGE_KERNEL);
> +#elif CONFIG_PGTABLE_LEVELS > 2
> +	pgd = __pgd(__pa(kasan_zero_pmd) | PAGE_KERNEL);
> +#else
> +	pgd = __pgd(__pa(kasan_zero_pte) | PAGE_KERNEL);
> +#endif
> +
> +	for (i = pgd_index(start); start < end; i++) {
> +		set_pgd(&pgdp[i], pgd);
> +		start += PGDIR_SIZE;
> +	}
> +}

Same problem as above with PAGE_KERNEL. You should just use
pgd_populate().

> +
> +void __init kasan_early_init(void)
> +{
> +	init_kasan_page_tables();
> +	kasan_map_early_shadow(swapper_pg_dir);
> +	kasan_map_early_shadow(idmap_pg_dir);
> +	flush_tlb_all();
> +	start_kernel();
> +}

Why do you need to map the kasan page tables into the idmap?

> +
> +static void __init clear_pgds(unsigned long start,
> +			unsigned long end)
> +{
> +	for (; start && start < end; start += PGDIR_SIZE)
> +		set_pgd(pgd_offset_k(start), __pgd(0));
> +}

We have dedicated pgd_clear() macro.

> +
> +static int __init zero_pte_populate(pmd_t *pmd, unsigned long addr,
> +				unsigned long end)
> +{
> +	pte_t *pte = pte_offset_kernel(pmd, addr);
> +
> +	while (addr + PAGE_SIZE <= end) {
> +		set_pte(pte, __pte(__pa(kasan_zero_page)
> +					| PAGE_KERNEL_RO));

See above for a pfn_pte() usage.

> +		addr += PAGE_SIZE;
> +		pte = pte_offset_kernel(pmd, addr);
> +	}
> +	return 0;
> +}
> +
> +static int __init zero_pmd_populate(pud_t *pud, unsigned long addr,
> +				unsigned long end)
> +{
> +	int ret = 0;
> +	pmd_t *pmd = pmd_offset(pud, addr);
> +
> +	while (IS_ALIGNED(addr, PMD_SIZE) && addr + PMD_SIZE <= end) {
> +		set_pmd(pmd, __pmd(__pa(kasan_zero_pte)
> +					| PAGE_KERNEL_RO));
> +		addr += PMD_SIZE;
> +		pmd++;
> +	}
> +
> +	if (addr < end) {
> +		if (pmd_none(*pmd)) {
> +			void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
> +			if (!p)
> +				return -ENOMEM;
> +			set_pmd(pmd, __pmd(__pa(p) | PAGE_KERNEL));
> +		}
> +		ret = zero_pte_populate(pmd, addr, end);
> +	}
> +	return ret;
> +}
> +
> +static int __init zero_pud_populate(pgd_t *pgd, unsigned long addr,
> +				unsigned long end)
> +{
> +	int ret = 0;
> +	pud_t *pud = pud_offset(pgd, addr);
> +
> +#if CONFIG_PGTABLE_LEVELS > 2
> +	while (IS_ALIGNED(addr, PUD_SIZE) && addr + PUD_SIZE <= end) {
> +		set_pud(pud, __pud(__pa(kasan_zero_pmd)
> +					| PAGE_KERNEL_RO));
> +		addr += PUD_SIZE;
> +		pud++;
> +	}
> +#endif
> +
> +	if (addr < end) {
> +		if (pud_none(*pud)) {
> +			void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
> +			if (!p)
> +				return -ENOMEM;
> +			set_pud(pud, __pud(__pa(p) | PAGE_KERNEL));
> +		}
> +		ret = zero_pmd_populate(pud, addr, end);
> +	}
> +	return ret;
> +}
> +
> +static int __init zero_pgd_populate(unsigned long addr, unsigned long end)
> +{
> +	int ret = 0;
> +	pgd_t *pgd = pgd_offset_k(addr);
> +
> +#if CONFIG_PGTABLE_LEVELS > 3
> +	 while (IS_ALIGNED(addr, PGDIR_SIZE) && addr + PGDIR_SIZE <= end) {
> +		set_pgd(pgd, __pgd(__pa(kasan_zero_pud)
> +					| PAGE_KERNEL_RO));
> +		addr += PGDIR_SIZE;
> +		pgd++;
> +	}
> +#endif

All these PAGE_KERNEL_RO on table entries are wrong. Please use the
standard pgd/pud/pmd_populate macros.

As for the while loops above, we have a standard way to avoid the
#ifdef's by using pgd_addr_end() etc. See __create_mapping() as an
example, there are a few others throughout the kernel.

> +
> +	 if (addr < end) {
> +		 if (pgd_none(*pgd)) {
> +			 void *p = vmemmap_alloc_block(PAGE_SIZE, NUMA_NO_NODE);
> +			 if (!p)
> +				 return -ENOMEM;
> +			 set_pgd(pgd, __pgd(__pa(p) | PAGE_KERNEL));

I'm just commenting here but it applies to the previous functions. You
may be able to use functions like vmmemap_pgd_populate() which look very
similar (and they also use pgd_populate instead of the set_pgd).

> +static void cpu_set_ttbr1(unsigned long ttbr1)
> +{
> +	asm(
> +	"	msr	ttbr1_el1, %0\n"
> +	"	isb"
> +	:
> +	: "r" (ttbr1));
> +}
> +
> +void __init kasan_init(void)
> +{
> +	struct memblock_region *reg;
> +
> +	memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
> +	cpu_set_ttbr1(__pa(tmp_page_table));

Why is this needed? The code lacks comments in several places but here I
couldn't figure out what the point is.

> +
> +	clear_pgds(KASAN_SHADOW_START, KASAN_SHADOW_END);
> +
> +	populate_zero_shadow(KASAN_SHADOW_START,
> +			(unsigned long)kasan_mem_to_shadow((void *)MODULES_VADDR));
> +
> +	for_each_memblock(memory, reg) {
> +		void *start = (void *)__phys_to_virt(reg->base);
> +		void *end = (void *)__phys_to_virt(reg->base + reg->size);
> +
> +		if (start >= end)
> +			break;
> +
> +		vmemmap_populate((unsigned long)kasan_mem_to_shadow(start),
> +				(unsigned long)kasan_mem_to_shadow(end),
> +				pfn_to_nid(virt_to_pfn(start)));
> +	}
> +
> +	memset(kasan_zero_page, 0, PAGE_SIZE);
> +	cpu_set_ttbr1(__pa(swapper_pg_dir));
> +	init_task.kasan_depth = 0;
> +}

-- 
Catalin

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

* Re: [PATCH 2/2] arm64: add KASan support
  2015-04-01 12:28     ` Catalin Marinas
  (?)
@ 2015-04-03 13:44       ` Andrey Ryabinin
  -1 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-04-03 13:44 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-arm-kernel, Will Deacon, linux-kernel, linux-mm, Andrew Morton

On 04/01/2015 03:28 PM, Catalin Marinas wrote:
> Hi Andrey,
> 

Hi Catalin,

> On Tue, Mar 24, 2015 at 05:49:04PM +0300, Andrey Ryabinin wrote:
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 4085df1..10bbd71 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -41,6 +41,7 @@ config ARM64
>>  	select HAVE_ARCH_AUDITSYSCALL
>>  	select HAVE_ARCH_BITREVERSE
>>  	select HAVE_ARCH_JUMP_LABEL
>> +	select HAVE_ARCH_KASAN if SPARSEMEM_VMEMMAP
>>  	select HAVE_ARCH_KGDB
>>  	select HAVE_ARCH_SECCOMP_FILTER
>>  	select HAVE_ARCH_TRACEHOOK
>> @@ -116,6 +117,12 @@ config GENERIC_CSUM
>>  config GENERIC_CALIBRATE_DELAY
>>  	def_bool y
>>  
>> +config KASAN_SHADOW_OFFSET
>> +	hex
>> +	default 0xdfff200000000000 if ARM64_VA_BITS_48
>> +	default 0xdffffc8000000000 if ARM64_VA_BITS_42
>> +	default 0xdfffff9000000000 if ARM64_VA_BITS_39
> 
> Can we compute these at build time in some C header? Or they need to be
> passed to gcc when compiling the kernel so that it generates the right
> instrumentation?
> 

Correct, this value passed to GCC.

> I'm not familiar with KASan but is the offset address supposed to be
> accessible? The addresses encoded above would always generate a fault
> (level 0 / address size fault).
> 

It's fine. KASAN_SHADOW_OFFSET address is shadow address that corresponds to 0 address.
So KASAN_SHADOW_OFFSET could be dereferenced only if we have NULL-ptr derefernce in kernel.

Shadow for kernel addresses starts from KASAN_SHADOW_START constant,
which is defined in arch/arm64/include/asm/kasan.h. But since I forgot to 'git add' that file
it's not present in this patch.

arch/arm64/include/asm/kasan.h:

/*
 * Compiler uses shadow offset assuming that addresses start
 * from 0. Kernel addresses don't start from 0, so shadow
 * for kernel really starts from 'compiler's shadow offset' +
 * ('kernel address space start' >> KASAN_SHADOW_SCALE_SHIFT)
 */
#define KASAN_SHADOW_START      (KASAN_SHADOW_OFFSET + \
					((UL(0xffffffffffffffff) << (VA_BITS)) >> 3))

#define KASAN_SHADOW_END        (KASAN_SHADOW_START + (1ULL << (VA_BITS - 3)))


>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> index bd5db28..f5ce010 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -40,7 +40,7 @@
>>   *	fixed mappings and modules
>>   */
>>  #define VMEMMAP_SIZE		ALIGN((1UL << (VA_BITS - PAGE_SHIFT)) * sizeof(struct page), PUD_SIZE)
>> -#define VMALLOC_START		(UL(0xffffffffffffffff) << VA_BITS)
>> +#define VMALLOC_START		((UL(0xffffffffffffffff) << VA_BITS) + (UL(1) << (VA_BITS - 3)))
> 
> I assume that's where you want to make room for KASan? Some comments and
> macros would be useful for why this is needed and how it is calculated.
> It also needs to be disabled when KASan is not enabled.
> 

Ok.

>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>> index 51c9811..1a99e95 100644
>> --- a/arch/arm64/kernel/head.S
>> +++ b/arch/arm64/kernel/head.S
>> @@ -482,6 +482,9 @@ __mmap_switched:
>>  	str_l	x21, __fdt_pointer, x5		// Save FDT pointer
>>  	str_l	x24, memstart_addr, x6		// Save PHYS_OFFSET
>>  	mov	x29, #0
>> +#ifdef CONFIG_KASAN
>> +	b kasan_early_init
>> +#endif
> 
> Nitpick: tab between b and kasan_early_init.
> 
>> diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
>> new file mode 100644
>> index 0000000..df537da
>> --- /dev/null
>> +++ b/arch/arm64/mm/kasan_init.c
>> @@ -0,0 +1,211 @@
>> +#include <linux/kasan.h>
>> +#include <linux/kernel.h>
>> +#include <linux/memblock.h>
>> +#include <linux/start_kernel.h>
>> +
>> +#include <asm/page.h>
>> +#include <asm/pgtable.h>
>> +#include <asm/tlbflush.h>
>> +
>> +static char kasan_zero_page[PAGE_SIZE] __page_aligned_bss;
> 
> Can we not use the system's zero_page or it's not initialised yet?
> 

System's zero page allocated in paging_init() and that is too late.
But I could put system's zero page into bss and use it here if you ok with this.


>> +static pgd_t tmp_page_table[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
>> +
>> +#if CONFIG_PGTABLE_LEVELS > 3
>> +static pud_t kasan_zero_pud[PTRS_PER_PUD] __page_aligned_bss;
>> +#endif
>> +#if CONFIG_PGTABLE_LEVELS > 2
>> +static pmd_t kasan_zero_pmd[PTRS_PER_PMD] __page_aligned_bss;
>> +#endif
>> +static pte_t kasan_zero_pte[PTRS_PER_PTE] __page_aligned_bss;
>> +
>> +static void __init init_kasan_page_tables(void)
>> +{
>> +	int i;
>> +
>> +#if CONFIG_PGTABLE_LEVELS > 3
>> +	for (i = 0; i < PTRS_PER_PUD; i++)
>> +		set_pud(&kasan_zero_pud[i], __pud(__pa(kasan_zero_pmd)
>> +							| PAGE_KERNEL));
>> +#endif
>> +#if CONFIG_PGTABLE_LEVELS > 2
>> +	for (i = 0; i < PTRS_PER_PMD; i++)
>> +		set_pmd(&kasan_zero_pmd[i], __pmd(__pa(kasan_zero_pte)
>> +							| PAGE_KERNEL));
>> +#endif
> 
> These don't look right. You are setting page attributes on table
> entries. You should use the standard pmd_populate etc. macros here, see
> early_fixmap_init() as an example.
> 

Right. Will fix.

>> +	for (i = 0; i < PTRS_PER_PTE; i++)
>> +		set_pte(&kasan_zero_pte[i], __pte(__pa(kasan_zero_page)
>> +							| PAGE_KERNEL));
> 
> PAGE_KERNEL is pgprot_t, so you mix the types here. Just do something
> like:
> 
> 	set_pte(..., pfn_pte(zero_pfn, PAGE_KERNEL_RO));
> 
> (shouldn't it be read-only?)
> 

It should be read-only, but only after kasan_init().
It should be writable earlier because stack instrumentation writes to shadow memory.
In function's prologue compiler writes to shadow to poison redzones around stack variables.


>> +void __init kasan_map_early_shadow(pgd_t *pgdp)
>> +{
>> +	int i;
>> +	unsigned long start = KASAN_SHADOW_START;
>> +	unsigned long end = KASAN_SHADOW_END;
>> +	pgd_t pgd;
>> +
>> +#if CONFIG_PGTABLE_LEVELS > 3
>> +	pgd = __pgd(__pa(kasan_zero_pud) | PAGE_KERNEL);
>> +#elif CONFIG_PGTABLE_LEVELS > 2
>> +	pgd = __pgd(__pa(kasan_zero_pmd) | PAGE_KERNEL);
>> +#else
>> +	pgd = __pgd(__pa(kasan_zero_pte) | PAGE_KERNEL);
>> +#endif
>> +
>> +	for (i = pgd_index(start); start < end; i++) {
>> +		set_pgd(&pgdp[i], pgd);
>> +		start += PGDIR_SIZE;
>> +	}
>> +}
> 
> Same problem as above with PAGE_KERNEL. You should just use
> pgd_populate().
> 

Ok

>> +
>> +void __init kasan_early_init(void)
>> +{
>> +	init_kasan_page_tables();
>> +	kasan_map_early_shadow(swapper_pg_dir);
>> +	kasan_map_early_shadow(idmap_pg_dir);
>> +	flush_tlb_all();
>> +	start_kernel();
>> +}
> 
> Why do you need to map the kasan page tables into the idmap?
> 

I don't need it. This is some left-over that should be removed.

>> +
>> +static void __init clear_pgds(unsigned long start,
>> +			unsigned long end)
>> +{
>> +	for (; start && start < end; start += PGDIR_SIZE)
>> +		set_pgd(pgd_offset_k(start), __pgd(0));
>> +}
> 
> We have dedicated pgd_clear() macro.
> 

I need to remove references to kasan_zero_p* tables from swapper_pg_dir so
pgd_clear() will not work here because it's noop on CONFIG_PGTABLE_LEVEL <= 3.


[...]

>> +static int __init zero_pgd_populate(unsigned long addr, unsigned long end)
>> +{
>> +	int ret = 0;
>> +	pgd_t *pgd = pgd_offset_k(addr);
>> +
>> +#if CONFIG_PGTABLE_LEVELS > 3
>> +	 while (IS_ALIGNED(addr, PGDIR_SIZE) && addr + PGDIR_SIZE <= end) {
>> +		set_pgd(pgd, __pgd(__pa(kasan_zero_pud)
>> +					| PAGE_KERNEL_RO));
>> +		addr += PGDIR_SIZE;
>> +		pgd++;
>> +	}
>> +#endif
> 
> All these PAGE_KERNEL_RO on table entries are wrong. Please use the
> standard pgd/pud/pmd_populate macros.
> 
> As for the while loops above, we have a standard way to avoid the
> #ifdef's by using pgd_addr_end() etc. See __create_mapping() as an
> example, there are a few others throughout the kernel.
> 

Ok.

[...]

>> +static void cpu_set_ttbr1(unsigned long ttbr1)
>> +{
>> +	asm(
>> +	"	msr	ttbr1_el1, %0\n"
>> +	"	isb"
>> +	:
>> +	: "r" (ttbr1));
>> +}
>> +
>> +void __init kasan_init(void)
>> +{
>> +	struct memblock_region *reg;
>> +
>> +	memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
>> +	cpu_set_ttbr1(__pa(tmp_page_table));
> 
> Why is this needed? The code lacks comments in several places but here I
> couldn't figure out what the point is.
> 

To setup shadow memory properly we need to unmap early shadow first (clear_pgds() in next line).

But instrumented kernel cannot run with unmaped shadow so this temporary
page table with early shadow until setting up shadow in swapper_pg_dir
will be finished.
I'll add comment about this here.


>> +
>> +	clear_pgds(KASAN_SHADOW_START, KASAN_SHADOW_END);
>> +
>> +	populate_zero_shadow(KASAN_SHADOW_START,
>> +			(unsigned long)kasan_mem_to_shadow((void *)MODULES_VADDR));
>> +
>> +	for_each_memblock(memory, reg) {
>> +		void *start = (void *)__phys_to_virt(reg->base);
>> +		void *end = (void *)__phys_to_virt(reg->base + reg->size);
>> +
>> +		if (start >= end)
>> +			break;
>> +
>> +		vmemmap_populate((unsigned long)kasan_mem_to_shadow(start),
>> +				(unsigned long)kasan_mem_to_shadow(end),
>> +				pfn_to_nid(virt_to_pfn(start)));
>> +	}
>> +
>> +	memset(kasan_zero_page, 0, PAGE_SIZE);
>> +	cpu_set_ttbr1(__pa(swapper_pg_dir));
>> +	init_task.kasan_depth = 0;
>> +}
> 

Thank you for detailed review.

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

* Re: [PATCH 2/2] arm64: add KASan support
@ 2015-04-03 13:44       ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-04-03 13:44 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: linux-arm-kernel, Will Deacon, linux-kernel, linux-mm, Andrew Morton

On 04/01/2015 03:28 PM, Catalin Marinas wrote:
> Hi Andrey,
> 

Hi Catalin,

> On Tue, Mar 24, 2015 at 05:49:04PM +0300, Andrey Ryabinin wrote:
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 4085df1..10bbd71 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -41,6 +41,7 @@ config ARM64
>>  	select HAVE_ARCH_AUDITSYSCALL
>>  	select HAVE_ARCH_BITREVERSE
>>  	select HAVE_ARCH_JUMP_LABEL
>> +	select HAVE_ARCH_KASAN if SPARSEMEM_VMEMMAP
>>  	select HAVE_ARCH_KGDB
>>  	select HAVE_ARCH_SECCOMP_FILTER
>>  	select HAVE_ARCH_TRACEHOOK
>> @@ -116,6 +117,12 @@ config GENERIC_CSUM
>>  config GENERIC_CALIBRATE_DELAY
>>  	def_bool y
>>  
>> +config KASAN_SHADOW_OFFSET
>> +	hex
>> +	default 0xdfff200000000000 if ARM64_VA_BITS_48
>> +	default 0xdffffc8000000000 if ARM64_VA_BITS_42
>> +	default 0xdfffff9000000000 if ARM64_VA_BITS_39
> 
> Can we compute these at build time in some C header? Or they need to be
> passed to gcc when compiling the kernel so that it generates the right
> instrumentation?
> 

Correct, this value passed to GCC.

> I'm not familiar with KASan but is the offset address supposed to be
> accessible? The addresses encoded above would always generate a fault
> (level 0 / address size fault).
> 

It's fine. KASAN_SHADOW_OFFSET address is shadow address that corresponds to 0 address.
So KASAN_SHADOW_OFFSET could be dereferenced only if we have NULL-ptr derefernce in kernel.

Shadow for kernel addresses starts from KASAN_SHADOW_START constant,
which is defined in arch/arm64/include/asm/kasan.h. But since I forgot to 'git add' that file
it's not present in this patch.

arch/arm64/include/asm/kasan.h:

/*
 * Compiler uses shadow offset assuming that addresses start
 * from 0. Kernel addresses don't start from 0, so shadow
 * for kernel really starts from 'compiler's shadow offset' +
 * ('kernel address space start' >> KASAN_SHADOW_SCALE_SHIFT)
 */
#define KASAN_SHADOW_START      (KASAN_SHADOW_OFFSET + \
					((UL(0xffffffffffffffff) << (VA_BITS)) >> 3))

#define KASAN_SHADOW_END        (KASAN_SHADOW_START + (1ULL << (VA_BITS - 3)))


>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> index bd5db28..f5ce010 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -40,7 +40,7 @@
>>   *	fixed mappings and modules
>>   */
>>  #define VMEMMAP_SIZE		ALIGN((1UL << (VA_BITS - PAGE_SHIFT)) * sizeof(struct page), PUD_SIZE)
>> -#define VMALLOC_START		(UL(0xffffffffffffffff) << VA_BITS)
>> +#define VMALLOC_START		((UL(0xffffffffffffffff) << VA_BITS) + (UL(1) << (VA_BITS - 3)))
> 
> I assume that's where you want to make room for KASan? Some comments and
> macros would be useful for why this is needed and how it is calculated.
> It also needs to be disabled when KASan is not enabled.
> 

Ok.

>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>> index 51c9811..1a99e95 100644
>> --- a/arch/arm64/kernel/head.S
>> +++ b/arch/arm64/kernel/head.S
>> @@ -482,6 +482,9 @@ __mmap_switched:
>>  	str_l	x21, __fdt_pointer, x5		// Save FDT pointer
>>  	str_l	x24, memstart_addr, x6		// Save PHYS_OFFSET
>>  	mov	x29, #0
>> +#ifdef CONFIG_KASAN
>> +	b kasan_early_init
>> +#endif
> 
> Nitpick: tab between b and kasan_early_init.
> 
>> diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
>> new file mode 100644
>> index 0000000..df537da
>> --- /dev/null
>> +++ b/arch/arm64/mm/kasan_init.c
>> @@ -0,0 +1,211 @@
>> +#include <linux/kasan.h>
>> +#include <linux/kernel.h>
>> +#include <linux/memblock.h>
>> +#include <linux/start_kernel.h>
>> +
>> +#include <asm/page.h>
>> +#include <asm/pgtable.h>
>> +#include <asm/tlbflush.h>
>> +
>> +static char kasan_zero_page[PAGE_SIZE] __page_aligned_bss;
> 
> Can we not use the system's zero_page or it's not initialised yet?
> 

System's zero page allocated in paging_init() and that is too late.
But I could put system's zero page into bss and use it here if you ok with this.


>> +static pgd_t tmp_page_table[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
>> +
>> +#if CONFIG_PGTABLE_LEVELS > 3
>> +static pud_t kasan_zero_pud[PTRS_PER_PUD] __page_aligned_bss;
>> +#endif
>> +#if CONFIG_PGTABLE_LEVELS > 2
>> +static pmd_t kasan_zero_pmd[PTRS_PER_PMD] __page_aligned_bss;
>> +#endif
>> +static pte_t kasan_zero_pte[PTRS_PER_PTE] __page_aligned_bss;
>> +
>> +static void __init init_kasan_page_tables(void)
>> +{
>> +	int i;
>> +
>> +#if CONFIG_PGTABLE_LEVELS > 3
>> +	for (i = 0; i < PTRS_PER_PUD; i++)
>> +		set_pud(&kasan_zero_pud[i], __pud(__pa(kasan_zero_pmd)
>> +							| PAGE_KERNEL));
>> +#endif
>> +#if CONFIG_PGTABLE_LEVELS > 2
>> +	for (i = 0; i < PTRS_PER_PMD; i++)
>> +		set_pmd(&kasan_zero_pmd[i], __pmd(__pa(kasan_zero_pte)
>> +							| PAGE_KERNEL));
>> +#endif
> 
> These don't look right. You are setting page attributes on table
> entries. You should use the standard pmd_populate etc. macros here, see
> early_fixmap_init() as an example.
> 

Right. Will fix.

>> +	for (i = 0; i < PTRS_PER_PTE; i++)
>> +		set_pte(&kasan_zero_pte[i], __pte(__pa(kasan_zero_page)
>> +							| PAGE_KERNEL));
> 
> PAGE_KERNEL is pgprot_t, so you mix the types here. Just do something
> like:
> 
> 	set_pte(..., pfn_pte(zero_pfn, PAGE_KERNEL_RO));
> 
> (shouldn't it be read-only?)
> 

It should be read-only, but only after kasan_init().
It should be writable earlier because stack instrumentation writes to shadow memory.
In function's prologue compiler writes to shadow to poison redzones around stack variables.


>> +void __init kasan_map_early_shadow(pgd_t *pgdp)
>> +{
>> +	int i;
>> +	unsigned long start = KASAN_SHADOW_START;
>> +	unsigned long end = KASAN_SHADOW_END;
>> +	pgd_t pgd;
>> +
>> +#if CONFIG_PGTABLE_LEVELS > 3
>> +	pgd = __pgd(__pa(kasan_zero_pud) | PAGE_KERNEL);
>> +#elif CONFIG_PGTABLE_LEVELS > 2
>> +	pgd = __pgd(__pa(kasan_zero_pmd) | PAGE_KERNEL);
>> +#else
>> +	pgd = __pgd(__pa(kasan_zero_pte) | PAGE_KERNEL);
>> +#endif
>> +
>> +	for (i = pgd_index(start); start < end; i++) {
>> +		set_pgd(&pgdp[i], pgd);
>> +		start += PGDIR_SIZE;
>> +	}
>> +}
> 
> Same problem as above with PAGE_KERNEL. You should just use
> pgd_populate().
> 

Ok

>> +
>> +void __init kasan_early_init(void)
>> +{
>> +	init_kasan_page_tables();
>> +	kasan_map_early_shadow(swapper_pg_dir);
>> +	kasan_map_early_shadow(idmap_pg_dir);
>> +	flush_tlb_all();
>> +	start_kernel();
>> +}
> 
> Why do you need to map the kasan page tables into the idmap?
> 

I don't need it. This is some left-over that should be removed.

>> +
>> +static void __init clear_pgds(unsigned long start,
>> +			unsigned long end)
>> +{
>> +	for (; start && start < end; start += PGDIR_SIZE)
>> +		set_pgd(pgd_offset_k(start), __pgd(0));
>> +}
> 
> We have dedicated pgd_clear() macro.
> 

I need to remove references to kasan_zero_p* tables from swapper_pg_dir so
pgd_clear() will not work here because it's noop on CONFIG_PGTABLE_LEVEL <= 3.


[...]

>> +static int __init zero_pgd_populate(unsigned long addr, unsigned long end)
>> +{
>> +	int ret = 0;
>> +	pgd_t *pgd = pgd_offset_k(addr);
>> +
>> +#if CONFIG_PGTABLE_LEVELS > 3
>> +	 while (IS_ALIGNED(addr, PGDIR_SIZE) && addr + PGDIR_SIZE <= end) {
>> +		set_pgd(pgd, __pgd(__pa(kasan_zero_pud)
>> +					| PAGE_KERNEL_RO));
>> +		addr += PGDIR_SIZE;
>> +		pgd++;
>> +	}
>> +#endif
> 
> All these PAGE_KERNEL_RO on table entries are wrong. Please use the
> standard pgd/pud/pmd_populate macros.
> 
> As for the while loops above, we have a standard way to avoid the
> #ifdef's by using pgd_addr_end() etc. See __create_mapping() as an
> example, there are a few others throughout the kernel.
> 

Ok.

[...]

>> +static void cpu_set_ttbr1(unsigned long ttbr1)
>> +{
>> +	asm(
>> +	"	msr	ttbr1_el1, %0\n"
>> +	"	isb"
>> +	:
>> +	: "r" (ttbr1));
>> +}
>> +
>> +void __init kasan_init(void)
>> +{
>> +	struct memblock_region *reg;
>> +
>> +	memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
>> +	cpu_set_ttbr1(__pa(tmp_page_table));
> 
> Why is this needed? The code lacks comments in several places but here I
> couldn't figure out what the point is.
> 

To setup shadow memory properly we need to unmap early shadow first (clear_pgds() in next line).

But instrumented kernel cannot run with unmaped shadow so this temporary
page table with early shadow until setting up shadow in swapper_pg_dir
will be finished.
I'll add comment about this here.


>> +
>> +	clear_pgds(KASAN_SHADOW_START, KASAN_SHADOW_END);
>> +
>> +	populate_zero_shadow(KASAN_SHADOW_START,
>> +			(unsigned long)kasan_mem_to_shadow((void *)MODULES_VADDR));
>> +
>> +	for_each_memblock(memory, reg) {
>> +		void *start = (void *)__phys_to_virt(reg->base);
>> +		void *end = (void *)__phys_to_virt(reg->base + reg->size);
>> +
>> +		if (start >= end)
>> +			break;
>> +
>> +		vmemmap_populate((unsigned long)kasan_mem_to_shadow(start),
>> +				(unsigned long)kasan_mem_to_shadow(end),
>> +				pfn_to_nid(virt_to_pfn(start)));
>> +	}
>> +
>> +	memset(kasan_zero_page, 0, PAGE_SIZE);
>> +	cpu_set_ttbr1(__pa(swapper_pg_dir));
>> +	init_task.kasan_depth = 0;
>> +}
> 

Thank you for detailed review.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [PATCH 2/2] arm64: add KASan support
@ 2015-04-03 13:44       ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-04-03 13:44 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/01/2015 03:28 PM, Catalin Marinas wrote:
> Hi Andrey,
> 

Hi Catalin,

> On Tue, Mar 24, 2015 at 05:49:04PM +0300, Andrey Ryabinin wrote:
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 4085df1..10bbd71 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -41,6 +41,7 @@ config ARM64
>>  	select HAVE_ARCH_AUDITSYSCALL
>>  	select HAVE_ARCH_BITREVERSE
>>  	select HAVE_ARCH_JUMP_LABEL
>> +	select HAVE_ARCH_KASAN if SPARSEMEM_VMEMMAP
>>  	select HAVE_ARCH_KGDB
>>  	select HAVE_ARCH_SECCOMP_FILTER
>>  	select HAVE_ARCH_TRACEHOOK
>> @@ -116,6 +117,12 @@ config GENERIC_CSUM
>>  config GENERIC_CALIBRATE_DELAY
>>  	def_bool y
>>  
>> +config KASAN_SHADOW_OFFSET
>> +	hex
>> +	default 0xdfff200000000000 if ARM64_VA_BITS_48
>> +	default 0xdffffc8000000000 if ARM64_VA_BITS_42
>> +	default 0xdfffff9000000000 if ARM64_VA_BITS_39
> 
> Can we compute these at build time in some C header? Or they need to be
> passed to gcc when compiling the kernel so that it generates the right
> instrumentation?
> 

Correct, this value passed to GCC.

> I'm not familiar with KASan but is the offset address supposed to be
> accessible? The addresses encoded above would always generate a fault
> (level 0 / address size fault).
> 

It's fine. KASAN_SHADOW_OFFSET address is shadow address that corresponds to 0 address.
So KASAN_SHADOW_OFFSET could be dereferenced only if we have NULL-ptr derefernce in kernel.

Shadow for kernel addresses starts from KASAN_SHADOW_START constant,
which is defined in arch/arm64/include/asm/kasan.h. But since I forgot to 'git add' that file
it's not present in this patch.

arch/arm64/include/asm/kasan.h:

/*
 * Compiler uses shadow offset assuming that addresses start
 * from 0. Kernel addresses don't start from 0, so shadow
 * for kernel really starts from 'compiler's shadow offset' +
 * ('kernel address space start' >> KASAN_SHADOW_SCALE_SHIFT)
 */
#define KASAN_SHADOW_START      (KASAN_SHADOW_OFFSET + \
					((UL(0xffffffffffffffff) << (VA_BITS)) >> 3))

#define KASAN_SHADOW_END        (KASAN_SHADOW_START + (1ULL << (VA_BITS - 3)))


>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> index bd5db28..f5ce010 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -40,7 +40,7 @@
>>   *	fixed mappings and modules
>>   */
>>  #define VMEMMAP_SIZE		ALIGN((1UL << (VA_BITS - PAGE_SHIFT)) * sizeof(struct page), PUD_SIZE)
>> -#define VMALLOC_START		(UL(0xffffffffffffffff) << VA_BITS)
>> +#define VMALLOC_START		((UL(0xffffffffffffffff) << VA_BITS) + (UL(1) << (VA_BITS - 3)))
> 
> I assume that's where you want to make room for KASan? Some comments and
> macros would be useful for why this is needed and how it is calculated.
> It also needs to be disabled when KASan is not enabled.
> 

Ok.

>> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
>> index 51c9811..1a99e95 100644
>> --- a/arch/arm64/kernel/head.S
>> +++ b/arch/arm64/kernel/head.S
>> @@ -482,6 +482,9 @@ __mmap_switched:
>>  	str_l	x21, __fdt_pointer, x5		// Save FDT pointer
>>  	str_l	x24, memstart_addr, x6		// Save PHYS_OFFSET
>>  	mov	x29, #0
>> +#ifdef CONFIG_KASAN
>> +	b kasan_early_init
>> +#endif
> 
> Nitpick: tab between b and kasan_early_init.
> 
>> diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c
>> new file mode 100644
>> index 0000000..df537da
>> --- /dev/null
>> +++ b/arch/arm64/mm/kasan_init.c
>> @@ -0,0 +1,211 @@
>> +#include <linux/kasan.h>
>> +#include <linux/kernel.h>
>> +#include <linux/memblock.h>
>> +#include <linux/start_kernel.h>
>> +
>> +#include <asm/page.h>
>> +#include <asm/pgtable.h>
>> +#include <asm/tlbflush.h>
>> +
>> +static char kasan_zero_page[PAGE_SIZE] __page_aligned_bss;
> 
> Can we not use the system's zero_page or it's not initialised yet?
> 

System's zero page allocated in paging_init() and that is too late.
But I could put system's zero page into bss and use it here if you ok with this.


>> +static pgd_t tmp_page_table[PTRS_PER_PGD] __initdata __aligned(PAGE_SIZE);
>> +
>> +#if CONFIG_PGTABLE_LEVELS > 3
>> +static pud_t kasan_zero_pud[PTRS_PER_PUD] __page_aligned_bss;
>> +#endif
>> +#if CONFIG_PGTABLE_LEVELS > 2
>> +static pmd_t kasan_zero_pmd[PTRS_PER_PMD] __page_aligned_bss;
>> +#endif
>> +static pte_t kasan_zero_pte[PTRS_PER_PTE] __page_aligned_bss;
>> +
>> +static void __init init_kasan_page_tables(void)
>> +{
>> +	int i;
>> +
>> +#if CONFIG_PGTABLE_LEVELS > 3
>> +	for (i = 0; i < PTRS_PER_PUD; i++)
>> +		set_pud(&kasan_zero_pud[i], __pud(__pa(kasan_zero_pmd)
>> +							| PAGE_KERNEL));
>> +#endif
>> +#if CONFIG_PGTABLE_LEVELS > 2
>> +	for (i = 0; i < PTRS_PER_PMD; i++)
>> +		set_pmd(&kasan_zero_pmd[i], __pmd(__pa(kasan_zero_pte)
>> +							| PAGE_KERNEL));
>> +#endif
> 
> These don't look right. You are setting page attributes on table
> entries. You should use the standard pmd_populate etc. macros here, see
> early_fixmap_init() as an example.
> 

Right. Will fix.

>> +	for (i = 0; i < PTRS_PER_PTE; i++)
>> +		set_pte(&kasan_zero_pte[i], __pte(__pa(kasan_zero_page)
>> +							| PAGE_KERNEL));
> 
> PAGE_KERNEL is pgprot_t, so you mix the types here. Just do something
> like:
> 
> 	set_pte(..., pfn_pte(zero_pfn, PAGE_KERNEL_RO));
> 
> (shouldn't it be read-only?)
> 

It should be read-only, but only after kasan_init().
It should be writable earlier because stack instrumentation writes to shadow memory.
In function's prologue compiler writes to shadow to poison redzones around stack variables.


>> +void __init kasan_map_early_shadow(pgd_t *pgdp)
>> +{
>> +	int i;
>> +	unsigned long start = KASAN_SHADOW_START;
>> +	unsigned long end = KASAN_SHADOW_END;
>> +	pgd_t pgd;
>> +
>> +#if CONFIG_PGTABLE_LEVELS > 3
>> +	pgd = __pgd(__pa(kasan_zero_pud) | PAGE_KERNEL);
>> +#elif CONFIG_PGTABLE_LEVELS > 2
>> +	pgd = __pgd(__pa(kasan_zero_pmd) | PAGE_KERNEL);
>> +#else
>> +	pgd = __pgd(__pa(kasan_zero_pte) | PAGE_KERNEL);
>> +#endif
>> +
>> +	for (i = pgd_index(start); start < end; i++) {
>> +		set_pgd(&pgdp[i], pgd);
>> +		start += PGDIR_SIZE;
>> +	}
>> +}
> 
> Same problem as above with PAGE_KERNEL. You should just use
> pgd_populate().
> 

Ok

>> +
>> +void __init kasan_early_init(void)
>> +{
>> +	init_kasan_page_tables();
>> +	kasan_map_early_shadow(swapper_pg_dir);
>> +	kasan_map_early_shadow(idmap_pg_dir);
>> +	flush_tlb_all();
>> +	start_kernel();
>> +}
> 
> Why do you need to map the kasan page tables into the idmap?
> 

I don't need it. This is some left-over that should be removed.

>> +
>> +static void __init clear_pgds(unsigned long start,
>> +			unsigned long end)
>> +{
>> +	for (; start && start < end; start += PGDIR_SIZE)
>> +		set_pgd(pgd_offset_k(start), __pgd(0));
>> +}
> 
> We have dedicated pgd_clear() macro.
> 

I need to remove references to kasan_zero_p* tables from swapper_pg_dir so
pgd_clear() will not work here because it's noop on CONFIG_PGTABLE_LEVEL <= 3.


[...]

>> +static int __init zero_pgd_populate(unsigned long addr, unsigned long end)
>> +{
>> +	int ret = 0;
>> +	pgd_t *pgd = pgd_offset_k(addr);
>> +
>> +#if CONFIG_PGTABLE_LEVELS > 3
>> +	 while (IS_ALIGNED(addr, PGDIR_SIZE) && addr + PGDIR_SIZE <= end) {
>> +		set_pgd(pgd, __pgd(__pa(kasan_zero_pud)
>> +					| PAGE_KERNEL_RO));
>> +		addr += PGDIR_SIZE;
>> +		pgd++;
>> +	}
>> +#endif
> 
> All these PAGE_KERNEL_RO on table entries are wrong. Please use the
> standard pgd/pud/pmd_populate macros.
> 
> As for the while loops above, we have a standard way to avoid the
> #ifdef's by using pgd_addr_end() etc. See __create_mapping() as an
> example, there are a few others throughout the kernel.
> 

Ok.

[...]

>> +static void cpu_set_ttbr1(unsigned long ttbr1)
>> +{
>> +	asm(
>> +	"	msr	ttbr1_el1, %0\n"
>> +	"	isb"
>> +	:
>> +	: "r" (ttbr1));
>> +}
>> +
>> +void __init kasan_init(void)
>> +{
>> +	struct memblock_region *reg;
>> +
>> +	memcpy(tmp_page_table, swapper_pg_dir, sizeof(tmp_page_table));
>> +	cpu_set_ttbr1(__pa(tmp_page_table));
> 
> Why is this needed? The code lacks comments in several places but here I
> couldn't figure out what the point is.
> 

To setup shadow memory properly we need to unmap early shadow first (clear_pgds() in next line).

But instrumented kernel cannot run with unmaped shadow so this temporary
page table with early shadow until setting up shadow in swapper_pg_dir
will be finished.
I'll add comment about this here.


>> +
>> +	clear_pgds(KASAN_SHADOW_START, KASAN_SHADOW_END);
>> +
>> +	populate_zero_shadow(KASAN_SHADOW_START,
>> +			(unsigned long)kasan_mem_to_shadow((void *)MODULES_VADDR));
>> +
>> +	for_each_memblock(memory, reg) {
>> +		void *start = (void *)__phys_to_virt(reg->base);
>> +		void *end = (void *)__phys_to_virt(reg->base + reg->size);
>> +
>> +		if (start >= end)
>> +			break;
>> +
>> +		vmemmap_populate((unsigned long)kasan_mem_to_shadow(start),
>> +				(unsigned long)kasan_mem_to_shadow(end),
>> +				pfn_to_nid(virt_to_pfn(start)));
>> +	}
>> +
>> +	memset(kasan_zero_page, 0, PAGE_SIZE);
>> +	cpu_set_ttbr1(__pa(swapper_pg_dir));
>> +	init_task.kasan_depth = 0;
>> +}
> 

Thank you for detailed review.

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

* Re: [PATCH 2/2] arm64: add KASan support
  2015-03-24 14:49   ` Andrey Ryabinin
  (?)
@ 2015-04-09 20:17     ` Arnd Bergmann
  -1 siblings, 0 replies; 39+ messages in thread
From: Arnd Bergmann @ 2015-04-09 20:17 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Andrey Ryabinin, Catalin Marinas, Will Deacon, linux-kernel,
	linux-mm, Andrew Morton

On Tuesday 24 March 2015 17:49:04 Andrey Ryabinin wrote:
>  arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
> 

Just one very high-level question: as this code is clearly derived from
the x86 version and nontrivial, could we move most of it out of
arch/{x86,arm64} into mm/kasan/init.c and have the rest in some header
file?

	Arnd

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

* Re: [PATCH 2/2] arm64: add KASan support
@ 2015-04-09 20:17     ` Arnd Bergmann
  0 siblings, 0 replies; 39+ messages in thread
From: Arnd Bergmann @ 2015-04-09 20:17 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Andrey Ryabinin, Catalin Marinas, Will Deacon, linux-kernel,
	linux-mm, Andrew Morton

On Tuesday 24 March 2015 17:49:04 Andrey Ryabinin wrote:
>  arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
> 

Just one very high-level question: as this code is clearly derived from
the x86 version and nontrivial, could we move most of it out of
arch/{x86,arm64} into mm/kasan/init.c and have the rest in some header
file?

	Arnd

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [PATCH 2/2] arm64: add KASan support
@ 2015-04-09 20:17     ` Arnd Bergmann
  0 siblings, 0 replies; 39+ messages in thread
From: Arnd Bergmann @ 2015-04-09 20:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Tuesday 24 March 2015 17:49:04 Andrey Ryabinin wrote:
>  arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
> 

Just one very high-level question: as this code is clearly derived from
the x86 version and nontrivial, could we move most of it out of
arch/{x86,arm64} into mm/kasan/init.c and have the rest in some header
file?

	Arnd

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

* Re: [PATCH 2/2] arm64: add KASan support
  2015-04-09 20:17     ` Arnd Bergmann
  (?)
@ 2015-04-10 10:48       ` Andrey Ryabinin
  -1 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-04-10 10:48 UTC (permalink / raw)
  To: Arnd Bergmann, linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, linux-kernel, linux-mm, Andrew Morton

On 04/09/2015 11:17 PM, Arnd Bergmann wrote:
> On Tuesday 24 March 2015 17:49:04 Andrey Ryabinin wrote:
>>  arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
>>
> 
> Just one very high-level question: as this code is clearly derived from
> the x86 version and nontrivial, could we move most of it out of
> arch/{x86,arm64} into mm/kasan/init.c and have the rest in some header
> file?
> 

I think most of this could be moved out from arch code, but not everything.
E.g. kasan_init() function is too arch-specific.

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

* Re: [PATCH 2/2] arm64: add KASan support
@ 2015-04-10 10:48       ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-04-10 10:48 UTC (permalink / raw)
  To: Arnd Bergmann, linux-arm-kernel
  Cc: Catalin Marinas, Will Deacon, linux-kernel, linux-mm, Andrew Morton

On 04/09/2015 11:17 PM, Arnd Bergmann wrote:
> On Tuesday 24 March 2015 17:49:04 Andrey Ryabinin wrote:
>>  arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
>>
> 
> Just one very high-level question: as this code is clearly derived from
> the x86 version and nontrivial, could we move most of it out of
> arch/{x86,arm64} into mm/kasan/init.c and have the rest in some header
> file?
> 

I think most of this could be moved out from arch code, but not everything.
E.g. kasan_init() function is too arch-specific.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [PATCH 2/2] arm64: add KASan support
@ 2015-04-10 10:48       ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-04-10 10:48 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/09/2015 11:17 PM, Arnd Bergmann wrote:
> On Tuesday 24 March 2015 17:49:04 Andrey Ryabinin wrote:
>>  arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
>>
> 
> Just one very high-level question: as this code is clearly derived from
> the x86 version and nontrivial, could we move most of it out of
> arch/{x86,arm64} into mm/kasan/init.c and have the rest in some header
> file?
> 

I think most of this could be moved out from arch code, but not everything.
E.g. kasan_init() function is too arch-specific.

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

* Re: [PATCH 2/2] arm64: add KASan support
  2015-04-10 10:48       ` Andrey Ryabinin
  (?)
@ 2015-04-10 13:02         ` Arnd Bergmann
  -1 siblings, 0 replies; 39+ messages in thread
From: Arnd Bergmann @ 2015-04-10 13:02 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, linux-kernel,
	linux-mm, Andrew Morton

On Friday 10 April 2015 13:48:52 Andrey Ryabinin wrote:
> On 04/09/2015 11:17 PM, Arnd Bergmann wrote:
> > On Tuesday 24 March 2015 17:49:04 Andrey Ryabinin wrote:
> >>  arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
> >>
> > 
> > Just one very high-level question: as this code is clearly derived from
> > the x86 version and nontrivial, could we move most of it out of
> > arch/{x86,arm64} into mm/kasan/init.c and have the rest in some header
> > file?
> > 
> 
> I think most of this could be moved out from arch code, but not everything.
> E.g. kasan_init() function is too arch-specific.

Right, makes sense. So presumably, populate_zero_shadow could become a global
function by another name, and possibly also handle registering the die
handler, so you can call it from an architecture specific kasan_init() 
function, right?

	Arnd

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

* Re: [PATCH 2/2] arm64: add KASan support
@ 2015-04-10 13:02         ` Arnd Bergmann
  0 siblings, 0 replies; 39+ messages in thread
From: Arnd Bergmann @ 2015-04-10 13:02 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, linux-kernel,
	linux-mm, Andrew Morton

On Friday 10 April 2015 13:48:52 Andrey Ryabinin wrote:
> On 04/09/2015 11:17 PM, Arnd Bergmann wrote:
> > On Tuesday 24 March 2015 17:49:04 Andrey Ryabinin wrote:
> >>  arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
> >>
> > 
> > Just one very high-level question: as this code is clearly derived from
> > the x86 version and nontrivial, could we move most of it out of
> > arch/{x86,arm64} into mm/kasan/init.c and have the rest in some header
> > file?
> > 
> 
> I think most of this could be moved out from arch code, but not everything.
> E.g. kasan_init() function is too arch-specific.

Right, makes sense. So presumably, populate_zero_shadow could become a global
function by another name, and possibly also handle registering the die
handler, so you can call it from an architecture specific kasan_init() 
function, right?

	Arnd

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [PATCH 2/2] arm64: add KASan support
@ 2015-04-10 13:02         ` Arnd Bergmann
  0 siblings, 0 replies; 39+ messages in thread
From: Arnd Bergmann @ 2015-04-10 13:02 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 10 April 2015 13:48:52 Andrey Ryabinin wrote:
> On 04/09/2015 11:17 PM, Arnd Bergmann wrote:
> > On Tuesday 24 March 2015 17:49:04 Andrey Ryabinin wrote:
> >>  arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
> >>
> > 
> > Just one very high-level question: as this code is clearly derived from
> > the x86 version and nontrivial, could we move most of it out of
> > arch/{x86,arm64} into mm/kasan/init.c and have the rest in some header
> > file?
> > 
> 
> I think most of this could be moved out from arch code, but not everything.
> E.g. kasan_init() function is too arch-specific.

Right, makes sense. So presumably, populate_zero_shadow could become a global
function by another name, and possibly also handle registering the die
handler, so you can call it from an architecture specific kasan_init() 
function, right?

	Arnd

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

* Re: [PATCH 2/2] arm64: add KASan support
  2015-04-10 13:02         ` Arnd Bergmann
  (?)
@ 2015-04-10 13:37           ` Andrey Ryabinin
  -1 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-04-10 13:37 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, linux-kernel,
	linux-mm, Andrew Morton

On 04/10/2015 04:02 PM, Arnd Bergmann wrote:
> On Friday 10 April 2015 13:48:52 Andrey Ryabinin wrote:
>> On 04/09/2015 11:17 PM, Arnd Bergmann wrote:
>>> On Tuesday 24 March 2015 17:49:04 Andrey Ryabinin wrote:
>>>>  arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
>>>>
>>>
>>> Just one very high-level question: as this code is clearly derived from
>>> the x86 version and nontrivial, could we move most of it out of
>>> arch/{x86,arm64} into mm/kasan/init.c and have the rest in some header
>>> file?
>>>
>>
>> I think most of this could be moved out from arch code, but not everything.
>> E.g. kasan_init() function is too arch-specific.
> 
> Right, makes sense. So presumably, populate_zero_shadow could become a global
> function by another name, and possibly also handle registering the die
> handler, so you can call it from an architecture specific kasan_init() 
> function, right?
> 

Yep, you are right.

> 	Arnd
> 


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

* Re: [PATCH 2/2] arm64: add KASan support
@ 2015-04-10 13:37           ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-04-10 13:37 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, Catalin Marinas, Will Deacon, linux-kernel,
	linux-mm, Andrew Morton

On 04/10/2015 04:02 PM, Arnd Bergmann wrote:
> On Friday 10 April 2015 13:48:52 Andrey Ryabinin wrote:
>> On 04/09/2015 11:17 PM, Arnd Bergmann wrote:
>>> On Tuesday 24 March 2015 17:49:04 Andrey Ryabinin wrote:
>>>>  arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
>>>>
>>>
>>> Just one very high-level question: as this code is clearly derived from
>>> the x86 version and nontrivial, could we move most of it out of
>>> arch/{x86,arm64} into mm/kasan/init.c and have the rest in some header
>>> file?
>>>
>>
>> I think most of this could be moved out from arch code, but not everything.
>> E.g. kasan_init() function is too arch-specific.
> 
> Right, makes sense. So presumably, populate_zero_shadow could become a global
> function by another name, and possibly also handle registering the die
> handler, so you can call it from an architecture specific kasan_init() 
> function, right?
> 

Yep, you are right.

> 	Arnd
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [PATCH 2/2] arm64: add KASan support
@ 2015-04-10 13:37           ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-04-10 13:37 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/10/2015 04:02 PM, Arnd Bergmann wrote:
> On Friday 10 April 2015 13:48:52 Andrey Ryabinin wrote:
>> On 04/09/2015 11:17 PM, Arnd Bergmann wrote:
>>> On Tuesday 24 March 2015 17:49:04 Andrey Ryabinin wrote:
>>>>  arch/arm64/mm/kasan_init.c           | 211 +++++++++++++++++++++++++++++++++++
>>>>
>>>
>>> Just one very high-level question: as this code is clearly derived from
>>> the x86 version and nontrivial, could we move most of it out of
>>> arch/{x86,arm64} into mm/kasan/init.c and have the rest in some header
>>> file?
>>>
>>
>> I think most of this could be moved out from arch code, but not everything.
>> E.g. kasan_init() function is too arch-specific.
> 
> Right, makes sense. So presumably, populate_zero_shadow could become a global
> function by another name, and possibly also handle registering the die
> handler, so you can call it from an architecture specific kasan_init() 
> function, right?
> 

Yep, you are right.

> 	Arnd
> 

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

* Re: [PATCH 2/2] arm64: add KASan support
  2015-04-03 13:44       ` Andrey Ryabinin
  (?)
@ 2015-04-15  2:37         ` David Keitel
  -1 siblings, 0 replies; 39+ messages in thread
From: David Keitel @ 2015-04-15  2:37 UTC (permalink / raw)
  To: Andrey Ryabinin, Catalin Marinas
  Cc: linux-mm, Andrew Morton, Will Deacon, linux-kernel, linux-arm-kernel

>>> +	pgd = __pgd(__pa(kasan_zero_pmd) | PAGE_KERNEL);
>>> +#else
>>> +	pgd = __pgd(__pa(kasan_zero_pte) | PAGE_KERNEL);
>>> +#endif
>>> +
>>> +	for (i = pgd_index(start); start < end; i++) {
>>> +		set_pgd(&pgdp[i], pgd);
>>> +		start += PGDIR_SIZE;
>>> +	}
>>> +}
>>
>> Same problem as above with PAGE_KERNEL. You should just use
>> pgd_populate().

Any suggestion what the correct flag setting would be here for a 4K mapping?

I tried fixing this by changing this to pud and setting the PMD_TYPE_TABLE flag for kasan_zero_pmd. However the MMU doesn't like it and I get a first level address translation fault.

If you have any updated patches to share I'd be glad to try them out.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH 2/2] arm64: add KASan support
@ 2015-04-15  2:37         ` David Keitel
  0 siblings, 0 replies; 39+ messages in thread
From: David Keitel @ 2015-04-15  2:37 UTC (permalink / raw)
  To: Andrey Ryabinin, Catalin Marinas
  Cc: linux-mm, Andrew Morton, Will Deacon, linux-kernel, linux-arm-kernel

>>> +	pgd = __pgd(__pa(kasan_zero_pmd) | PAGE_KERNEL);
>>> +#else
>>> +	pgd = __pgd(__pa(kasan_zero_pte) | PAGE_KERNEL);
>>> +#endif
>>> +
>>> +	for (i = pgd_index(start); start < end; i++) {
>>> +		set_pgd(&pgdp[i], pgd);
>>> +		start += PGDIR_SIZE;
>>> +	}
>>> +}
>>
>> Same problem as above with PAGE_KERNEL. You should just use
>> pgd_populate().

Any suggestion what the correct flag setting would be here for a 4K mapping?

I tried fixing this by changing this to pud and setting the PMD_TYPE_TABLE flag for kasan_zero_pmd. However the MMU doesn't like it and I get a first level address translation fault.

If you have any updated patches to share I'd be glad to try them out.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [PATCH 2/2] arm64: add KASan support
@ 2015-04-15  2:37         ` David Keitel
  0 siblings, 0 replies; 39+ messages in thread
From: David Keitel @ 2015-04-15  2:37 UTC (permalink / raw)
  To: linux-arm-kernel

>>> +	pgd = __pgd(__pa(kasan_zero_pmd) | PAGE_KERNEL);
>>> +#else
>>> +	pgd = __pgd(__pa(kasan_zero_pte) | PAGE_KERNEL);
>>> +#endif
>>> +
>>> +	for (i = pgd_index(start); start < end; i++) {
>>> +		set_pgd(&pgdp[i], pgd);
>>> +		start += PGDIR_SIZE;
>>> +	}
>>> +}
>>
>> Same problem as above with PAGE_KERNEL. You should just use
>> pgd_populate().

Any suggestion what the correct flag setting would be here for a 4K mapping?

I tried fixing this by changing this to pud and setting the PMD_TYPE_TABLE flag for kasan_zero_pmd. However the MMU doesn't like it and I get a first level address translation fault.

If you have any updated patches to share I'd be glad to try them out.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH 2/2] arm64: add KASan support
  2015-04-15  2:37         ` David Keitel
  (?)
@ 2015-04-15 18:04           ` Andrey Ryabinin
  -1 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-04-15 18:04 UTC (permalink / raw)
  To: David Keitel
  Cc: Catalin Marinas, linux-mm, Andrew Morton, Will Deacon,
	linux-kernel, linux-arm-kernel

On 04/15/2015 05:37 AM, David Keitel wrote:
>>>> +	pgd = __pgd(__pa(kasan_zero_pmd) | PAGE_KERNEL);
>>>> +#else
>>>> +	pgd = __pgd(__pa(kasan_zero_pte) | PAGE_KERNEL);
>>>> +#endif
>>>> +
>>>> +	for (i = pgd_index(start); start < end; i++) {
>>>> +		set_pgd(&pgdp[i], pgd);
>>>> +		start += PGDIR_SIZE;
>>>> +	}
>>>> +}
>>>
>>> Same problem as above with PAGE_KERNEL. You should just use
>>> pgd_populate().
> 
> Any suggestion what the correct flag setting would be here for a 4K mapping?
> 
> I tried fixing this by changing this to pud and setting the PMD_TYPE_TABLE flag for kasan_zero_pmd. However the MMU doesn't like it and I get a first level address translation fault.
> 
> If you have any updated patches to share I'd be glad to try them out.
> 

Sorry, I didn't have much time on work on this yet.

I've pushed the most fresh thing that I have in git:
	git://github.com/aryabinin/linux.git kasan/arm64v1

It's the same patches with two simple but important fixes on top of it.

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

* Re: [PATCH 2/2] arm64: add KASan support
@ 2015-04-15 18:04           ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-04-15 18:04 UTC (permalink / raw)
  To: David Keitel
  Cc: Catalin Marinas, linux-mm, Andrew Morton, Will Deacon,
	linux-kernel, linux-arm-kernel

On 04/15/2015 05:37 AM, David Keitel wrote:
>>>> +	pgd = __pgd(__pa(kasan_zero_pmd) | PAGE_KERNEL);
>>>> +#else
>>>> +	pgd = __pgd(__pa(kasan_zero_pte) | PAGE_KERNEL);
>>>> +#endif
>>>> +
>>>> +	for (i = pgd_index(start); start < end; i++) {
>>>> +		set_pgd(&pgdp[i], pgd);
>>>> +		start += PGDIR_SIZE;
>>>> +	}
>>>> +}
>>>
>>> Same problem as above with PAGE_KERNEL. You should just use
>>> pgd_populate().
> 
> Any suggestion what the correct flag setting would be here for a 4K mapping?
> 
> I tried fixing this by changing this to pud and setting the PMD_TYPE_TABLE flag for kasan_zero_pmd. However the MMU doesn't like it and I get a first level address translation fault.
> 
> If you have any updated patches to share I'd be glad to try them out.
> 

Sorry, I didn't have much time on work on this yet.

I've pushed the most fresh thing that I have in git:
	git://github.com/aryabinin/linux.git kasan/arm64v1

It's the same patches with two simple but important fixes on top of it.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [PATCH 2/2] arm64: add KASan support
@ 2015-04-15 18:04           ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-04-15 18:04 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/15/2015 05:37 AM, David Keitel wrote:
>>>> +	pgd = __pgd(__pa(kasan_zero_pmd) | PAGE_KERNEL);
>>>> +#else
>>>> +	pgd = __pgd(__pa(kasan_zero_pte) | PAGE_KERNEL);
>>>> +#endif
>>>> +
>>>> +	for (i = pgd_index(start); start < end; i++) {
>>>> +		set_pgd(&pgdp[i], pgd);
>>>> +		start += PGDIR_SIZE;
>>>> +	}
>>>> +}
>>>
>>> Same problem as above with PAGE_KERNEL. You should just use
>>> pgd_populate().
> 
> Any suggestion what the correct flag setting would be here for a 4K mapping?
> 
> I tried fixing this by changing this to pud and setting the PMD_TYPE_TABLE flag for kasan_zero_pmd. However the MMU doesn't like it and I get a first level address translation fault.
> 
> If you have any updated patches to share I'd be glad to try them out.
> 

Sorry, I didn't have much time on work on this yet.

I've pushed the most fresh thing that I have in git:
	git://github.com/aryabinin/linux.git kasan/arm64v1

It's the same patches with two simple but important fixes on top of it.

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

* Re: [PATCH 2/2] arm64: add KASan support
  2015-04-15 18:04           ` Andrey Ryabinin
  (?)
@ 2015-04-17 18:39             ` David Keitel
  -1 siblings, 0 replies; 39+ messages in thread
From: David Keitel @ 2015-04-17 18:39 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Catalin Marinas, linux-mm, Andrew Morton, Will Deacon,
	linux-kernel, linux-arm-kernel

On 04/15/2015 11:04 AM, Andrey Ryabinin wrote:
> I've pushed the most fresh thing that I have in git:
> 	git://github.com/aryabinin/linux.git kasan/arm64v1
> 
> It's the same patches with two simple but important fixes on top of it.

Thanks, the two commits do fix compilation issues that I've had worked around to get to my mapping question.

I've addressed the mapping problem using __create_page_tables in arch/arm64/head.S as an example.

The next roadblock I hit was running into kasan_report_error calls in cgroups_early_init. After a short investigation it does seem to be a false positive due the the kasan_zero_page size and tracking bytes being reused for different memory regions.

I worked around that by enabling kasan error reporting only after the kasan_init is run. This let me get to the shell with some real KAsan reports along the way. There were some other fixes and hacks to get there. I'll backtrack to evaluate which ones warrant an RFC.

 - David

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH 2/2] arm64: add KASan support
@ 2015-04-17 18:39             ` David Keitel
  0 siblings, 0 replies; 39+ messages in thread
From: David Keitel @ 2015-04-17 18:39 UTC (permalink / raw)
  To: Andrey Ryabinin
  Cc: Catalin Marinas, linux-mm, Andrew Morton, Will Deacon,
	linux-kernel, linux-arm-kernel

On 04/15/2015 11:04 AM, Andrey Ryabinin wrote:
> I've pushed the most fresh thing that I have in git:
> 	git://github.com/aryabinin/linux.git kasan/arm64v1
> 
> It's the same patches with two simple but important fixes on top of it.

Thanks, the two commits do fix compilation issues that I've had worked around to get to my mapping question.

I've addressed the mapping problem using __create_page_tables in arch/arm64/head.S as an example.

The next roadblock I hit was running into kasan_report_error calls in cgroups_early_init. After a short investigation it does seem to be a false positive due the the kasan_zero_page size and tracking bytes being reused for different memory regions.

I worked around that by enabling kasan error reporting only after the kasan_init is run. This let me get to the shell with some real KAsan reports along the way. There were some other fixes and hacks to get there. I'll backtrack to evaluate which ones warrant an RFC.

 - David

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [PATCH 2/2] arm64: add KASan support
@ 2015-04-17 18:39             ` David Keitel
  0 siblings, 0 replies; 39+ messages in thread
From: David Keitel @ 2015-04-17 18:39 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/15/2015 11:04 AM, Andrey Ryabinin wrote:
> I've pushed the most fresh thing that I have in git:
> 	git://github.com/aryabinin/linux.git kasan/arm64v1
> 
> It's the same patches with two simple but important fixes on top of it.

Thanks, the two commits do fix compilation issues that I've had worked around to get to my mapping question.

I've addressed the mapping problem using __create_page_tables in arch/arm64/head.S as an example.

The next roadblock I hit was running into kasan_report_error calls in cgroups_early_init. After a short investigation it does seem to be a false positive due the the kasan_zero_page size and tracking bytes being reused for different memory regions.

I worked around that by enabling kasan error reporting only after the kasan_init is run. This let me get to the shell with some real KAsan reports along the way. There were some other fixes and hacks to get there. I'll backtrack to evaluate which ones warrant an RFC.

 - David

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH 2/2] arm64: add KASan support
  2015-04-17 18:39             ` David Keitel
  (?)
@ 2015-04-20  6:48               ` Andrey Ryabinin
  -1 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-04-20  6:48 UTC (permalink / raw)
  To: David Keitel
  Cc: Catalin Marinas, linux-mm, Andrew Morton, Will Deacon,
	linux-kernel, linux-arm-kernel

On 04/17/2015 09:39 PM, David Keitel wrote:
> On 04/15/2015 11:04 AM, Andrey Ryabinin wrote:
>> I've pushed the most fresh thing that I have in git:
>> 	git://github.com/aryabinin/linux.git kasan/arm64v1
>>
>> It's the same patches with two simple but important fixes on top of it.
> 
> Thanks, the two commits do fix compilation issues that I've had worked around to get to my mapping question.
> 
> I've addressed the mapping problem using __create_page_tables in arch/arm64/head.S as an example.
> 
> The next roadblock I hit was running into kasan_report_error calls in cgroups_early_init. After a short investigation it does seem to be a false positive due the the kasan_zero_page size and tracking bytes being reused for different memory regions.
> 
> I worked around that by enabling kasan error reporting only after the kasan_init is run. This let me get to the shell with some real KAsan reports along the way.

Reporting already disabled before kasan_init() and the last thing that kasan_init() is enable error reports.
So, how did you managed to get kasan's report before kasan_init()?

> There were some other fixes and hacks to get there. I'll backtrack to evaluate which ones warrant an RFC.
> 
>  - David
> 


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

* Re: [PATCH 2/2] arm64: add KASan support
@ 2015-04-20  6:48               ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-04-20  6:48 UTC (permalink / raw)
  To: David Keitel
  Cc: Catalin Marinas, linux-mm, Andrew Morton, Will Deacon,
	linux-kernel, linux-arm-kernel

On 04/17/2015 09:39 PM, David Keitel wrote:
> On 04/15/2015 11:04 AM, Andrey Ryabinin wrote:
>> I've pushed the most fresh thing that I have in git:
>> 	git://github.com/aryabinin/linux.git kasan/arm64v1
>>
>> It's the same patches with two simple but important fixes on top of it.
> 
> Thanks, the two commits do fix compilation issues that I've had worked around to get to my mapping question.
> 
> I've addressed the mapping problem using __create_page_tables in arch/arm64/head.S as an example.
> 
> The next roadblock I hit was running into kasan_report_error calls in cgroups_early_init. After a short investigation it does seem to be a false positive due the the kasan_zero_page size and tracking bytes being reused for different memory regions.
> 
> I worked around that by enabling kasan error reporting only after the kasan_init is run. This let me get to the shell with some real KAsan reports along the way.

Reporting already disabled before kasan_init() and the last thing that kasan_init() is enable error reports.
So, how did you managed to get kasan's report before kasan_init()?

> There were some other fixes and hacks to get there. I'll backtrack to evaluate which ones warrant an RFC.
> 
>  - David
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [PATCH 2/2] arm64: add KASan support
@ 2015-04-20  6:48               ` Andrey Ryabinin
  0 siblings, 0 replies; 39+ messages in thread
From: Andrey Ryabinin @ 2015-04-20  6:48 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/17/2015 09:39 PM, David Keitel wrote:
> On 04/15/2015 11:04 AM, Andrey Ryabinin wrote:
>> I've pushed the most fresh thing that I have in git:
>> 	git://github.com/aryabinin/linux.git kasan/arm64v1
>>
>> It's the same patches with two simple but important fixes on top of it.
> 
> Thanks, the two commits do fix compilation issues that I've had worked around to get to my mapping question.
> 
> I've addressed the mapping problem using __create_page_tables in arch/arm64/head.S as an example.
> 
> The next roadblock I hit was running into kasan_report_error calls in cgroups_early_init. After a short investigation it does seem to be a false positive due the the kasan_zero_page size and tracking bytes being reused for different memory regions.
> 
> I worked around that by enabling kasan error reporting only after the kasan_init is run. This let me get to the shell with some real KAsan reports along the way.

Reporting already disabled before kasan_init() and the last thing that kasan_init() is enable error reports.
So, how did you managed to get kasan's report before kasan_init()?

> There were some other fixes and hacks to get there. I'll backtrack to evaluate which ones warrant an RFC.
> 
>  - David
> 

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

end of thread, other threads:[~2015-04-20  6:48 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-24 14:49 [PATCH 0/2] KASan for arm64 Andrey Ryabinin
2015-03-24 14:49 ` Andrey Ryabinin
2015-03-24 14:49 ` Andrey Ryabinin
2015-03-24 14:49 ` [PATCH 1/2] kasan, x86: move KASAN_SHADOW_OFFSET to the arch Kconfig Andrey Ryabinin
2015-03-24 14:49   ` Andrey Ryabinin
2015-03-24 14:49   ` Andrey Ryabinin
2015-03-24 14:49 ` [PATCH 2/2] arm64: add KASan support Andrey Ryabinin
2015-03-24 14:49   ` Andrey Ryabinin
2015-03-24 14:49   ` Andrey Ryabinin
2015-04-01 12:28   ` Catalin Marinas
2015-04-01 12:28     ` Catalin Marinas
2015-04-01 12:28     ` Catalin Marinas
2015-04-03 13:44     ` Andrey Ryabinin
2015-04-03 13:44       ` Andrey Ryabinin
2015-04-03 13:44       ` Andrey Ryabinin
2015-04-15  2:37       ` David Keitel
2015-04-15  2:37         ` David Keitel
2015-04-15  2:37         ` David Keitel
2015-04-15 18:04         ` Andrey Ryabinin
2015-04-15 18:04           ` Andrey Ryabinin
2015-04-15 18:04           ` Andrey Ryabinin
2015-04-17 18:39           ` David Keitel
2015-04-17 18:39             ` David Keitel
2015-04-17 18:39             ` David Keitel
2015-04-20  6:48             ` Andrey Ryabinin
2015-04-20  6:48               ` Andrey Ryabinin
2015-04-20  6:48               ` Andrey Ryabinin
2015-04-09 20:17   ` Arnd Bergmann
2015-04-09 20:17     ` Arnd Bergmann
2015-04-09 20:17     ` Arnd Bergmann
2015-04-10 10:48     ` Andrey Ryabinin
2015-04-10 10:48       ` Andrey Ryabinin
2015-04-10 10:48       ` Andrey Ryabinin
2015-04-10 13:02       ` Arnd Bergmann
2015-04-10 13:02         ` Arnd Bergmann
2015-04-10 13:02         ` Arnd Bergmann
2015-04-10 13:37         ` Andrey Ryabinin
2015-04-10 13:37           ` Andrey Ryabinin
2015-04-10 13:37           ` Andrey Ryabinin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.