linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/3] KASAN for powerpc/32
@ 2019-02-12 13:36 Christophe Leroy
  2019-02-12 13:36 ` [PATCH v5 1/3] powerpc/mm: prepare kernel for KAsan on PPC32 Christophe Leroy
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Christophe Leroy @ 2019-02-12 13:36 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin,
	Alexander Potapenko, Dmitry Vyukov, Daniel Axtens
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev

This serie adds KASAN support to powerpc/32

Tested on nohash/32 (8xx) and book3s/32 (mpc832x ie 603)

Changes in v5:
- Added KASAN_SHADOW_OFFSET in Makefile, otherwise we fallback to KASAN_MINIMAL
and some stuff like stack instrumentation is not performed
- Moved calls to kasan_early_init() in head.S because stack instrumentation
in machine_init was performed before the call to kasan_early_init()
- Mapping kasan_early_shadow_page RW in kasan_early_init() and
remaping RO later in kasan_init()
- Allocating a big memblock() for shadow area, falling back to PAGE_SIZE blocks in case of failure.

Changes in v4:
- Comments from Andrey (DISABLE_BRANCH_PROFILING, Activation of reports)
- Proper initialisation of shadow area in kasan_init()
- Panic in case Hash table is required.
- Added comments in patch one to explain why *t = *s becomes memcpy(t, s, ...)
- Call of kasan_init_tags()

Changes in v3:
- Removed the printk() in kasan_early_init() to avoid build failure (see https://github.com/linuxppc/issues/issues/218)
- Added necessary changes in asm/book3s/32/pgtable.h to get it work on powerpc 603 family
- Added a few KASAN_SANITIZE_xxx.o := n to successfully boot on powerpc 603 family

Changes in v2:
- Rebased.
- Using __set_pte_at() to build the early table.
- Worked around and got rid of the patch adding asm/page.h in asm/pgtable-types.h
    ==> might be fixed independently but not needed for this serie.

For book3s/32 (not 603), it cannot work as is because due to HASHPTE flag, we
can't use the same pagetable for several PGD entries, and because Hash table
management is not not active early enough at the time being.

Christophe Leroy (3):
  powerpc/mm: prepare kernel for KAsan on PPC32
  powerpc/32: Move early_init() in a separate file
  powerpc/32: Add KASAN support

 arch/powerpc/Kconfig                         |   1 +
 arch/powerpc/Makefile                        |   7 ++
 arch/powerpc/include/asm/book3s/32/pgtable.h |   2 +
 arch/powerpc/include/asm/kasan.h             |  24 ++++++
 arch/powerpc/include/asm/nohash/32/pgtable.h |   2 +
 arch/powerpc/include/asm/ppc_asm.h           |   4 +
 arch/powerpc/include/asm/setup.h             |   5 ++
 arch/powerpc/include/asm/string.h            |  14 ++++
 arch/powerpc/kernel/Makefile                 |  11 ++-
 arch/powerpc/kernel/asm-offsets.c            |   4 +
 arch/powerpc/kernel/cputable.c               |  13 ++-
 arch/powerpc/kernel/early_32.c               |  35 ++++++++
 arch/powerpc/kernel/head_32.S                |   3 +
 arch/powerpc/kernel/head_40x.S               |   3 +
 arch/powerpc/kernel/head_44x.S               |   3 +
 arch/powerpc/kernel/head_8xx.S               |   3 +
 arch/powerpc/kernel/head_fsl_booke.S         |   3 +
 arch/powerpc/kernel/prom_init_check.sh       |  10 ++-
 arch/powerpc/kernel/setup-common.c           |   2 +
 arch/powerpc/kernel/setup_32.c               |  28 -------
 arch/powerpc/lib/Makefile                    |   8 ++
 arch/powerpc/lib/copy_32.S                   |   9 ++-
 arch/powerpc/mm/Makefile                     |   3 +
 arch/powerpc/mm/dump_linuxpagetables.c       |   8 ++
 arch/powerpc/mm/kasan_init.c                 | 114 +++++++++++++++++++++++++++
 arch/powerpc/mm/mem.c                        |   4 +
 26 files changed, 285 insertions(+), 38 deletions(-)
 create mode 100644 arch/powerpc/include/asm/kasan.h
 create mode 100644 arch/powerpc/kernel/early_32.c
 create mode 100644 arch/powerpc/mm/kasan_init.c

-- 
2.13.3


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

* [PATCH v5 1/3] powerpc/mm: prepare kernel for KAsan on PPC32
  2019-02-12 13:36 [PATCH v5 0/3] KASAN for powerpc/32 Christophe Leroy
@ 2019-02-12 13:36 ` Christophe Leroy
  2019-02-12 13:36 ` [PATCH v5 2/3] powerpc/32: Move early_init() in a separate file Christophe Leroy
  2019-02-12 13:36 ` [PATCH v5 3/3] powerpc/32: Add KASAN support Christophe Leroy
  2 siblings, 0 replies; 11+ messages in thread
From: Christophe Leroy @ 2019-02-12 13:36 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin,
	Alexander Potapenko, Dmitry Vyukov, Daniel Axtens
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev

In kernel/cputable.c, explicitly use memcpy() in order
to allow GCC to replace it with __memcpy() when KASAN is
selected.

Since commit 400c47d81ca38 ("powerpc32: memset: only use dcbz once cache is
enabled"), memset() can be used before activation of the cache,
so no need to use memset_io() for zeroing the BSS.

Acked-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/cputable.c | 13 ++++++++++---
 arch/powerpc/kernel/setup_32.c |  6 ++----
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 1eab54bc6ee9..cd12f362b61f 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -2147,7 +2147,11 @@ void __init set_cur_cpu_spec(struct cpu_spec *s)
 	struct cpu_spec *t = &the_cpu_spec;
 
 	t = PTRRELOC(t);
-	*t = *s;
+	/*
+	 * use memcpy() instead of *t = *s so that GCC replaces it
+	 * by __memcpy() when KASAN is active
+	 */
+	memcpy(t, s, sizeof(*t));
 
 	*PTRRELOC(&cur_cpu_spec) = &the_cpu_spec;
 }
@@ -2161,8 +2165,11 @@ static struct cpu_spec * __init setup_cpu_spec(unsigned long offset,
 	t = PTRRELOC(t);
 	old = *t;
 
-	/* Copy everything, then do fixups */
-	*t = *s;
+	/*
+	 * Copy everything, then do fixups. Use memcpy() instead of *t = *s
+	 * so that GCC replaces it by __memcpy() when KASAN is active
+	 */
+	memcpy(t, s, sizeof(*t));
 
 	/*
 	 * If we are overriding a previous value derived from the real
diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index 947f904688b0..5e761eb16a6d 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -73,10 +73,8 @@ notrace unsigned long __init early_init(unsigned long dt_ptr)
 {
 	unsigned long offset = reloc_offset();
 
-	/* First zero the BSS -- use memset_io, some platforms don't have
-	 * caches on yet */
-	memset_io((void __iomem *)PTRRELOC(&__bss_start), 0,
-			__bss_stop - __bss_start);
+	/* First zero the BSS */
+	memset(PTRRELOC(&__bss_start), 0, __bss_stop - __bss_start);
 
 	/*
 	 * Identify the CPU type and fix up code sections
-- 
2.13.3


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

* [PATCH v5 2/3] powerpc/32: Move early_init() in a separate file
  2019-02-12 13:36 [PATCH v5 0/3] KASAN for powerpc/32 Christophe Leroy
  2019-02-12 13:36 ` [PATCH v5 1/3] powerpc/mm: prepare kernel for KAsan on PPC32 Christophe Leroy
@ 2019-02-12 13:36 ` Christophe Leroy
  2019-02-12 13:36 ` [PATCH v5 3/3] powerpc/32: Add KASAN support Christophe Leroy
  2 siblings, 0 replies; 11+ messages in thread
From: Christophe Leroy @ 2019-02-12 13:36 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin,
	Alexander Potapenko, Dmitry Vyukov, Daniel Axtens
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev

In preparation of KASAN, move early_init() into a separate
file in order to allow deactivation of KASAN for that function.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/Makefile   |  2 +-
 arch/powerpc/kernel/early_32.c | 35 +++++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/setup_32.c | 26 --------------------------
 3 files changed, 36 insertions(+), 27 deletions(-)
 create mode 100644 arch/powerpc/kernel/early_32.c

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index cb7f0bb9ee71..879b36602748 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -93,7 +93,7 @@ extra-y				+= vmlinux.lds
 
 obj-$(CONFIG_RELOCATABLE)	+= reloc_$(BITS).o
 
-obj-$(CONFIG_PPC32)		+= entry_32.o setup_32.o
+obj-$(CONFIG_PPC32)		+= entry_32.o setup_32.o early_32.o
 obj-$(CONFIG_PPC64)		+= dma-iommu.o iommu.o
 obj-$(CONFIG_KGDB)		+= kgdb.o
 obj-$(CONFIG_BOOTX_TEXT)	+= btext.o
diff --git a/arch/powerpc/kernel/early_32.c b/arch/powerpc/kernel/early_32.c
new file mode 100644
index 000000000000..b3e40d6d651c
--- /dev/null
+++ b/arch/powerpc/kernel/early_32.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Early init before relocation
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <asm/setup.h>
+#include <asm/sections.h>
+
+/*
+ * We're called here very early in the boot.
+ *
+ * Note that the kernel may be running at an address which is different
+ * from the address that it was linked at, so we must use RELOC/PTRRELOC
+ * to access static data (including strings).  -- paulus
+ */
+notrace unsigned long __init early_init(unsigned long dt_ptr)
+{
+	unsigned long offset = reloc_offset();
+
+	/* First zero the BSS */
+	memset(PTRRELOC(&__bss_start), 0, __bss_stop - __bss_start);
+
+	/*
+	 * Identify the CPU type and fix up code sections
+	 * that depend on which cpu we have.
+	 */
+	identify_cpu(offset, mfspr(SPRN_PVR));
+
+	apply_feature_fixups();
+
+	return KERNELBASE + offset;
+}
diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index 5e761eb16a6d..b46a9a33225b 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -63,32 +63,6 @@ EXPORT_SYMBOL(DMA_MODE_READ);
 EXPORT_SYMBOL(DMA_MODE_WRITE);
 
 /*
- * We're called here very early in the boot.
- *
- * Note that the kernel may be running at an address which is different
- * from the address that it was linked at, so we must use RELOC/PTRRELOC
- * to access static data (including strings).  -- paulus
- */
-notrace unsigned long __init early_init(unsigned long dt_ptr)
-{
-	unsigned long offset = reloc_offset();
-
-	/* First zero the BSS */
-	memset(PTRRELOC(&__bss_start), 0, __bss_stop - __bss_start);
-
-	/*
-	 * Identify the CPU type and fix up code sections
-	 * that depend on which cpu we have.
-	 */
-	identify_cpu(offset, mfspr(SPRN_PVR));
-
-	apply_feature_fixups();
-
-	return KERNELBASE + offset;
-}
-
-
-/*
  * This is run before start_kernel(), the kernel has been relocated
  * and we are running with enough of the MMU enabled to have our
  * proper kernel virtual addresses
-- 
2.13.3


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

* [PATCH v5 3/3] powerpc/32: Add KASAN support
  2019-02-12 13:36 [PATCH v5 0/3] KASAN for powerpc/32 Christophe Leroy
  2019-02-12 13:36 ` [PATCH v5 1/3] powerpc/mm: prepare kernel for KAsan on PPC32 Christophe Leroy
  2019-02-12 13:36 ` [PATCH v5 2/3] powerpc/32: Move early_init() in a separate file Christophe Leroy
@ 2019-02-12 13:36 ` Christophe Leroy
  2019-02-14 22:04   ` Daniel Axtens
  2019-02-18  9:27   ` Michael Ellerman
  2 siblings, 2 replies; 11+ messages in thread
From: Christophe Leroy @ 2019-02-12 13:36 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin,
	Alexander Potapenko, Dmitry Vyukov, Daniel Axtens
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev

This patch adds KASAN support for PPC32.

The KASAN shadow area is located between the vmalloc area and the
fixmap area.

KASAN_SHADOW_OFFSET is calculated in asm/kasan.h and extracted
by Makefile prepare rule via asm-offsets.h

Note that on book3s it will only work on the 603 because the other
ones use hash table and can therefore not share a single PTE table
covering the entire early KASAN shadow area.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/Kconfig                         |   1 +
 arch/powerpc/Makefile                        |   7 ++
 arch/powerpc/include/asm/book3s/32/pgtable.h |   2 +
 arch/powerpc/include/asm/kasan.h             |  24 ++++++
 arch/powerpc/include/asm/nohash/32/pgtable.h |   2 +
 arch/powerpc/include/asm/ppc_asm.h           |   4 +
 arch/powerpc/include/asm/setup.h             |   5 ++
 arch/powerpc/include/asm/string.h            |  14 ++++
 arch/powerpc/kernel/Makefile                 |   9 ++-
 arch/powerpc/kernel/asm-offsets.c            |   4 +
 arch/powerpc/kernel/head_32.S                |   3 +
 arch/powerpc/kernel/head_40x.S               |   3 +
 arch/powerpc/kernel/head_44x.S               |   3 +
 arch/powerpc/kernel/head_8xx.S               |   3 +
 arch/powerpc/kernel/head_fsl_booke.S         |   3 +
 arch/powerpc/kernel/prom_init_check.sh       |  10 ++-
 arch/powerpc/kernel/setup-common.c           |   2 +
 arch/powerpc/lib/Makefile                    |   8 ++
 arch/powerpc/lib/copy_32.S                   |   9 ++-
 arch/powerpc/mm/Makefile                     |   3 +
 arch/powerpc/mm/dump_linuxpagetables.c       |   8 ++
 arch/powerpc/mm/kasan_init.c                 | 114 +++++++++++++++++++++++++++
 arch/powerpc/mm/mem.c                        |   4 +
 23 files changed, 239 insertions(+), 6 deletions(-)
 create mode 100644 arch/powerpc/include/asm/kasan.h
 create mode 100644 arch/powerpc/mm/kasan_init.c

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 08908219fba9..850b06def84f 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -175,6 +175,7 @@ config PPC
 	select GENERIC_TIME_VSYSCALL
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_ARCH_JUMP_LABEL
+	select HAVE_ARCH_KASAN			if PPC32
 	select HAVE_ARCH_KGDB
 	select HAVE_ARCH_MMAP_RND_BITS
 	select HAVE_ARCH_MMAP_RND_COMPAT_BITS	if COMPAT
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index ac033341ed55..f0738099e31e 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -427,6 +427,13 @@ else
 endif
 endif
 
+ifdef CONFIG_KASAN
+prepare: kasan_prepare
+
+kasan_prepare: prepare0
+       $(eval KASAN_SHADOW_OFFSET = $(shell awk '{if ($$2 == "KASAN_SHADOW_OFFSET") print $$3;}' include/generated/asm-offsets.h))
+endif
+
 # Check toolchain versions:
 # - gcc-4.6 is the minimum kernel-wide version so nothing required.
 checkbin:
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 49d76adb9bc5..4543016f80ca 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -141,6 +141,8 @@ static inline bool pte_user(pte_t pte)
  */
 #ifdef CONFIG_HIGHMEM
 #define KVIRT_TOP	PKMAP_BASE
+#elif defined(CONFIG_KASAN)
+#define KVIRT_TOP	KASAN_SHADOW_START
 #else
 #define KVIRT_TOP	(0xfe000000UL)	/* for now, could be FIXMAP_BASE ? */
 #endif
diff --git a/arch/powerpc/include/asm/kasan.h b/arch/powerpc/include/asm/kasan.h
new file mode 100644
index 000000000000..5d0088429b62
--- /dev/null
+++ b/arch/powerpc/include/asm/kasan.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_KASAN_H
+#define __ASM_KASAN_H
+
+#ifndef __ASSEMBLY__
+
+#include <asm/page.h>
+#include <asm/pgtable-types.h>
+#include <asm/fixmap.h>
+
+#define KASAN_SHADOW_SCALE_SHIFT	3
+#define KASAN_SHADOW_SIZE	((~0UL - PAGE_OFFSET + 1) >> KASAN_SHADOW_SCALE_SHIFT)
+
+#define KASAN_SHADOW_START	(ALIGN_DOWN(FIXADDR_START - KASAN_SHADOW_SIZE, \
+					    PGDIR_SIZE))
+#define KASAN_SHADOW_END	(KASAN_SHADOW_START + KASAN_SHADOW_SIZE)
+#define KASAN_SHADOW_OFFSET	(KASAN_SHADOW_START - \
+				 (PAGE_OFFSET >> KASAN_SHADOW_SCALE_SHIFT))
+
+void kasan_early_init(void);
+void kasan_init(void);
+
+#endif
+#endif
diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h
index bed433358260..b3b52f02be1a 100644
--- a/arch/powerpc/include/asm/nohash/32/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
@@ -71,6 +71,8 @@ extern int icache_44x_need_flush;
  */
 #ifdef CONFIG_HIGHMEM
 #define KVIRT_TOP	PKMAP_BASE
+#elif defined(CONFIG_KASAN)
+#define KVIRT_TOP	KASAN_SHADOW_START
 #else
 #define KVIRT_TOP	(0xfe000000UL)	/* for now, could be FIXMAP_BASE ? */
 #endif
diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
index e0637730a8e7..dba2c1038363 100644
--- a/arch/powerpc/include/asm/ppc_asm.h
+++ b/arch/powerpc/include/asm/ppc_asm.h
@@ -251,6 +251,10 @@ GLUE(.,name):
 
 #define _GLOBAL_TOC(name) _GLOBAL(name)
 
+#define KASAN_OVERRIDE(x, y) \
+	.weak x;	     \
+	.set x, y
+
 #endif
 
 /*
diff --git a/arch/powerpc/include/asm/setup.h b/arch/powerpc/include/asm/setup.h
index 65676e2325b8..da7768aa996a 100644
--- a/arch/powerpc/include/asm/setup.h
+++ b/arch/powerpc/include/asm/setup.h
@@ -74,6 +74,11 @@ static inline void setup_spectre_v2(void) {};
 #endif
 void do_btb_flush_fixups(void);
 
+#ifndef CONFIG_KASAN
+static inline void kasan_early_init(void) { }
+static inline void kasan_init(void) { }
+#endif
+
 #endif /* !__ASSEMBLY__ */
 
 #endif	/* _ASM_POWERPC_SETUP_H */
diff --git a/arch/powerpc/include/asm/string.h b/arch/powerpc/include/asm/string.h
index 1647de15a31e..64d44d4836b4 100644
--- a/arch/powerpc/include/asm/string.h
+++ b/arch/powerpc/include/asm/string.h
@@ -27,6 +27,20 @@ extern int memcmp(const void *,const void *,__kernel_size_t);
 extern void * memchr(const void *,int,__kernel_size_t);
 extern void * memcpy_flushcache(void *,const void *,__kernel_size_t);
 
+void *__memset(void *s, int c, __kernel_size_t count);
+void *__memcpy(void *to, const void *from, __kernel_size_t n);
+void *__memmove(void *to, const void *from, __kernel_size_t n);
+
+#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
+/*
+ * For files that are 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
+
 #ifdef CONFIG_PPC64
 #define __HAVE_ARCH_MEMSET32
 #define __HAVE_ARCH_MEMSET64
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 879b36602748..fc4c42262694 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -16,8 +16,9 @@ CFLAGS_prom_init.o      += -fPIC
 CFLAGS_btext.o		+= -fPIC
 endif
 
-CFLAGS_cputable.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
-CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
+CFLAGS_early_32.o += -DDISABLE_BRANCH_PROFILING
+CFLAGS_cputable.o += $(DISABLE_LATENT_ENTROPY_PLUGIN) -DDISABLE_BRANCH_PROFILING
+CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN) -DDISABLE_BRANCH_PROFILING
 CFLAGS_btext.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
 
@@ -31,6 +32,10 @@ CFLAGS_REMOVE_btext.o = $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_prom.o = $(CC_FLAGS_FTRACE)
 endif
 
+KASAN_SANITIZE_early_32.o := n
+KASAN_SANITIZE_cputable.o := n
+KASAN_SANITIZE_prom_init.o := n
+
 obj-y				:= cputable.o ptrace.o syscalls.o \
 				   irq.o align.o signal_32.o pmc.o vdso.o \
 				   process.o systbl.o idle.o \
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 9ffc72ded73a..846fb30b1190 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -783,5 +783,9 @@ int main(void)
 	DEFINE(VIRT_IMMR_BASE, (u64)__fix_to_virt(FIX_IMMR_BASE));
 #endif
 
+#ifdef CONFIG_KASAN
+	DEFINE(KASAN_SHADOW_OFFSET, KASAN_SHADOW_OFFSET);
+#endif
+
 	return 0;
 }
diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index 05b08db3901d..0ec9dec06bc2 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -962,6 +962,9 @@ start_here:
  * Do early platform-specific initialization,
  * and set up the MMU.
  */
+#ifdef CONFIG_KASAN
+	bl	kasan_early_init
+#endif
 	li	r3,0
 	mr	r4,r31
 	bl	machine_init
diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
index b19d78410511..5d6ff8fa7e2b 100644
--- a/arch/powerpc/kernel/head_40x.S
+++ b/arch/powerpc/kernel/head_40x.S
@@ -848,6 +848,9 @@ start_here:
 /*
  * Decide what sort of machine this is and initialize the MMU.
  */
+#ifdef CONFIG_KASAN
+	bl	kasan_early_init
+#endif
 	li	r3,0
 	mr	r4,r31
 	bl	machine_init
diff --git a/arch/powerpc/kernel/head_44x.S b/arch/powerpc/kernel/head_44x.S
index bf23c19c92d6..7ca14dff6192 100644
--- a/arch/powerpc/kernel/head_44x.S
+++ b/arch/powerpc/kernel/head_44x.S
@@ -203,6 +203,9 @@ _ENTRY(_start);
 /*
  * Decide what sort of machine this is and initialize the MMU.
  */
+#ifdef CONFIG_KASAN
+	bl	kasan_early_init
+#endif
 	li	r3,0
 	mr	r4,r31
 	bl	machine_init
diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
index 0fea10491f3a..6a644ea2e6b6 100644
--- a/arch/powerpc/kernel/head_8xx.S
+++ b/arch/powerpc/kernel/head_8xx.S
@@ -823,6 +823,9 @@ start_here:
 /*
  * Decide what sort of machine this is and initialize the MMU.
  */
+#ifdef CONFIG_KASAN
+	bl	kasan_early_init
+#endif
 	li	r3,0
 	mr	r4,r31
 	bl	machine_init
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 2386ce2a9c6e..4f4585a68850 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -274,6 +274,9 @@ set_ivor:
 /*
  * Decide what sort of machine this is and initialize the MMU.
  */
+#ifdef CONFIG_KASAN
+	bl	kasan_early_init
+#endif
 	mr	r3,r30
 	mr	r4,r31
 	bl	machine_init
diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
index 667df97d2595..da6bb16e0876 100644
--- a/arch/powerpc/kernel/prom_init_check.sh
+++ b/arch/powerpc/kernel/prom_init_check.sh
@@ -16,8 +16,16 @@
 # If you really need to reference something from prom_init.o add
 # it to the list below:
 
+grep CONFIG_KASAN=y .config >/dev/null
+if [ $? -eq 0 ]
+then
+	MEMFCT="__memcpy __memset"
+else
+	MEMFCT="memcpy memset"
+fi
+
 WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
-_end enter_prom memcpy memset reloc_offset __secondary_hold
+_end enter_prom $MEMFCT reloc_offset __secondary_hold
 __secondary_hold_acknowledge __secondary_hold_spinloop __start
 strcmp strcpy strlcpy strlen strncmp strstr kstrtobool logo_linux_clut224
 reloc_got2 kernstart_addr memstart_addr linux_banner _stext
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index ca00fbb97cf8..16ff1ea66805 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -978,6 +978,8 @@ void __init setup_arch(char **cmdline_p)
 
 	paging_init();
 
+	kasan_init();
+
 	/* Initialize the MMU context management stuff. */
 	mmu_context_init();
 
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 3bf9fc6fd36c..ce8d4a9f810a 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -8,6 +8,14 @@ ccflags-$(CONFIG_PPC64)	:= $(NO_MINIMAL_TOC)
 CFLAGS_REMOVE_code-patching.o = $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_feature-fixups.o = $(CC_FLAGS_FTRACE)
 
+KASAN_SANITIZE_code-patching.o := n
+KASAN_SANITIZE_feature-fixups.o := n
+
+ifdef CONFIG_KASAN
+CFLAGS_code-patching.o += -DDISABLE_BRANCH_PROFILING
+CFLAGS_feature-fixups.o += -DDISABLE_BRANCH_PROFILING
+endif
+
 obj-y += string.o alloc.o code-patching.o feature-fixups.o
 
 obj-$(CONFIG_PPC32)	+= div64.o copy_32.o crtsavres.o strlen_32.o
diff --git a/arch/powerpc/lib/copy_32.S b/arch/powerpc/lib/copy_32.S
index ba66846fe973..4d8a1c73b4cf 100644
--- a/arch/powerpc/lib/copy_32.S
+++ b/arch/powerpc/lib/copy_32.S
@@ -91,7 +91,8 @@ EXPORT_SYMBOL(memset16)
  * We therefore skip the optimised bloc that uses dcbz. This jump is
  * replaced by a nop once cache is active. This is done in machine_init()
  */
-_GLOBAL(memset)
+_GLOBAL(__memset)
+KASAN_OVERRIDE(memset, __memset)
 	cmplwi	0,r5,4
 	blt	7f
 
@@ -163,12 +164,14 @@ EXPORT_SYMBOL(memset)
  * We therefore jump to generic_memcpy which doesn't use dcbz. This jump is
  * replaced by a nop once cache is active. This is done in machine_init()
  */
-_GLOBAL(memmove)
+_GLOBAL(__memmove)
+KASAN_OVERRIDE(memmove, __memmove)
 	cmplw	0,r3,r4
 	bgt	backwards_memcpy
 	/* fall through */
 
-_GLOBAL(memcpy)
+_GLOBAL(__memcpy)
+KASAN_OVERRIDE(memcpy, __memcpy)
 1:	b	generic_memcpy
 	patch_site	1b, patch__memcpy_nocache
 
diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
index f965fc33a8b7..d6b76f25f6de 100644
--- a/arch/powerpc/mm/Makefile
+++ b/arch/powerpc/mm/Makefile
@@ -7,6 +7,8 @@ ccflags-$(CONFIG_PPC64)	:= $(NO_MINIMAL_TOC)
 
 CFLAGS_REMOVE_slb.o = $(CC_FLAGS_FTRACE)
 
+KASAN_SANITIZE_kasan_init.o := n
+
 obj-y				:= fault.o mem.o pgtable.o mmap.o \
 				   init_$(BITS).o pgtable_$(BITS).o \
 				   init-common.o mmu_context.o drmem.o
@@ -55,3 +57,4 @@ obj-$(CONFIG_PPC_BOOK3S_64)	+= dump_linuxpagetables-book3s64.o
 endif
 obj-$(CONFIG_PPC_HTDUMP)	+= dump_hashpagetable.o
 obj-$(CONFIG_PPC_MEM_KEYS)	+= pkeys.o
+obj-$(CONFIG_KASAN)		+= kasan_init.o
diff --git a/arch/powerpc/mm/dump_linuxpagetables.c b/arch/powerpc/mm/dump_linuxpagetables.c
index 6aa41669ac1a..c862b48118f1 100644
--- a/arch/powerpc/mm/dump_linuxpagetables.c
+++ b/arch/powerpc/mm/dump_linuxpagetables.c
@@ -94,6 +94,10 @@ static struct addr_marker address_markers[] = {
 	{ 0,	"Consistent mem start" },
 	{ 0,	"Consistent mem end" },
 #endif
+#ifdef CONFIG_KASAN
+	{ 0,	"kasan shadow mem start" },
+	{ 0,	"kasan shadow mem end" },
+#endif
 #ifdef CONFIG_HIGHMEM
 	{ 0,	"Highmem PTEs start" },
 	{ 0,	"Highmem PTEs end" },
@@ -310,6 +314,10 @@ static void populate_markers(void)
 	address_markers[i++].start_address = IOREMAP_TOP +
 					     CONFIG_CONSISTENT_SIZE;
 #endif
+#ifdef CONFIG_KASAN
+	address_markers[i++].start_address = KASAN_SHADOW_START;
+	address_markers[i++].start_address = KASAN_SHADOW_END;
+#endif
 #ifdef CONFIG_HIGHMEM
 	address_markers[i++].start_address = PKMAP_BASE;
 	address_markers[i++].start_address = PKMAP_ADDR(LAST_PKMAP);
diff --git a/arch/powerpc/mm/kasan_init.c b/arch/powerpc/mm/kasan_init.c
new file mode 100644
index 000000000000..bd8e0a263e12
--- /dev/null
+++ b/arch/powerpc/mm/kasan_init.c
@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define DISABLE_BRANCH_PROFILING
+
+#include <linux/kasan.h>
+#include <linux/printk.h>
+#include <linux/memblock.h>
+#include <linux/sched/task.h>
+#include <asm/pgalloc.h>
+
+void __init kasan_early_init(void)
+{
+	unsigned long addr = KASAN_SHADOW_START;
+	unsigned long end = KASAN_SHADOW_END;
+	unsigned long next;
+	pmd_t *pmd = pmd_offset(pud_offset(pgd_offset_k(addr), addr), addr);
+	int i;
+	phys_addr_t pa = __pa(kasan_early_shadow_page);
+
+	BUILD_BUG_ON(KASAN_SHADOW_START & ~PGDIR_MASK);
+
+	if (early_mmu_has_feature(MMU_FTR_HPTE_TABLE))
+		panic("KASAN not supported with Hash MMU\n");
+
+	for (i = 0; i < PTRS_PER_PTE; i++)
+		__set_pte_at(&init_mm, (unsigned long)kasan_early_shadow_page,
+			     kasan_early_shadow_pte + i,
+			     pfn_pte(PHYS_PFN(pa), PAGE_KERNEL), 0);
+
+	do {
+		next = pgd_addr_end(addr, end);
+		pmd_populate_kernel(&init_mm, pmd, kasan_early_shadow_pte);
+	} while (pmd++, addr = next, addr != end);
+}
+
+static void __init kasan_init_region(struct memblock_region *reg)
+{
+	void *start = __va(reg->base);
+	void *end = __va(reg->base + reg->size);
+	unsigned long k_start, k_end, k_cur, k_next;
+	pmd_t *pmd;
+	void *block;
+
+	if (start >= end)
+		return;
+
+	k_start = (unsigned long)kasan_mem_to_shadow(start);
+	k_end = (unsigned long)kasan_mem_to_shadow(end);
+	pmd = pmd_offset(pud_offset(pgd_offset_k(k_start), k_start), k_start);
+
+	for (k_cur = k_start; k_cur != k_end; k_cur = k_next, pmd++) {
+		k_next = pgd_addr_end(k_cur, k_end);
+		if ((void *)pmd_page_vaddr(*pmd) == kasan_early_shadow_pte) {
+			pte_t *new = pte_alloc_one_kernel(&init_mm);
+
+			if (!new)
+				panic("kasan: pte_alloc_one_kernel() failed");
+			memcpy(new, kasan_early_shadow_pte, PTE_TABLE_SIZE);
+			pmd_populate_kernel(&init_mm, pmd, new);
+		}
+	};
+
+	block = memblock_alloc(k_end - k_start, PAGE_SIZE);
+	for (k_cur = k_start; k_cur < k_end; k_cur += PAGE_SIZE) {
+		void *va = block ? block + k_cur - k_start :
+				   memblock_alloc(PAGE_SIZE, PAGE_SIZE);
+		pte_t pte = pfn_pte(PHYS_PFN(__pa(va)), PAGE_KERNEL);
+
+		if (!va)
+			panic("kasan: memblock_alloc() failed");
+		pmd = pmd_offset(pud_offset(pgd_offset_k(k_cur), k_cur), k_cur);
+		pte_update(pte_offset_kernel(pmd, k_cur), ~0, pte_val(pte));
+	}
+	flush_tlb_kernel_range(k_start, k_end);
+}
+
+static void __init kasan_remap_early_shadow_ro(void)
+{
+	unsigned long k_cur;
+	phys_addr_t pa = __pa(kasan_early_shadow_page);
+	int i;
+
+	for (i = 0; i < PTRS_PER_PTE; i++)
+		ptep_set_wrprotect(&init_mm, 0, kasan_early_shadow_pte + i);
+
+	for (k_cur = PAGE_OFFSET & PAGE_MASK; k_cur; k_cur += PAGE_SIZE) {
+		pmd_t *pmd = pmd_offset(pud_offset(pgd_offset_k(k_cur), k_cur), k_cur);
+		pte_t *ptep = pte_offset_kernel(pmd, k_cur);
+
+		if ((void *)pmd_page_vaddr(*pmd) == kasan_early_shadow_pte)
+			continue;
+		if ((pte_val(*ptep) & PAGE_MASK) != pa)
+			continue;
+
+		ptep_set_wrprotect(&init_mm, k_cur, ptep);
+	}
+	flush_tlb_mm(&init_mm);
+}
+
+void __init kasan_init(void)
+{
+	struct memblock_region *reg;
+
+	for_each_memblock(memory, reg)
+		kasan_init_region(reg);
+
+	kasan_remap_early_shadow_ro();
+
+	clear_page(kasan_early_shadow_page);
+
+	/* At this point kasan is fully initialized. Enable error messages */
+	init_task.kasan_depth = 0;
+	pr_info("KASAN init done\n");
+}
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 81f251fc4169..1bb055775e60 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -336,6 +336,10 @@ void __init mem_init(void)
 	pr_info("  * 0x%08lx..0x%08lx  : highmem PTEs\n",
 		PKMAP_BASE, PKMAP_ADDR(LAST_PKMAP));
 #endif /* CONFIG_HIGHMEM */
+#ifdef CONFIG_KASAN
+	pr_info("  * 0x%08lx..0x%08lx  : kasan shadow mem\n",
+		KASAN_SHADOW_START, KASAN_SHADOW_END);
+#endif
 #ifdef CONFIG_NOT_COHERENT_CACHE
 	pr_info("  * 0x%08lx..0x%08lx  : consistent mem\n",
 		IOREMAP_TOP, IOREMAP_TOP + CONFIG_CONSISTENT_SIZE);
-- 
2.13.3


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

* Re: [PATCH v5 3/3] powerpc/32: Add KASAN support
  2019-02-12 13:36 ` [PATCH v5 3/3] powerpc/32: Add KASAN support Christophe Leroy
@ 2019-02-14 22:04   ` Daniel Axtens
  2019-02-15  8:41     ` Christophe Leroy
  2019-02-18  9:27   ` Michael Ellerman
  1 sibling, 1 reply; 11+ messages in thread
From: Daniel Axtens @ 2019-02-14 22:04 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Nicholas Piggin, Aneesh Kumar K.V,
	Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev

Hi Christophe,

> --- a/arch/powerpc/include/asm/string.h
> +++ b/arch/powerpc/include/asm/string.h
> @@ -27,6 +27,20 @@ extern int memcmp(const void *,const void *,__kernel_size_t);
>  extern void * memchr(const void *,int,__kernel_size_t);
>  extern void * memcpy_flushcache(void *,const void *,__kernel_size_t);
>  
> +void *__memset(void *s, int c, __kernel_size_t count);
> +void *__memcpy(void *to, const void *from, __kernel_size_t n);
> +void *__memmove(void *to, const void *from, __kernel_size_t n);
> +
> +#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
> +/*
> + * For files that are 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
> +

I'm finding that I miss tests like 'kasan test: kasan_memcmp
out-of-bounds in memcmp' because the uninstrumented asm version is used
instead of an instrumented C version. I ended up guarding the relevant
__HAVE_ARCH_x symbols behind a #ifndef CONFIG_KASAN and only exporting
the arch versions if we're not compiled with KASAN.

I find I need to guard and unexport strncpy, strncmp, memchr and
memcmp. Do you need to do this on 32bit as well, or are those tests
passing anyway for some reason?

Regards,
Daniel


>  #ifdef CONFIG_PPC64
>  #define __HAVE_ARCH_MEMSET32
>  #define __HAVE_ARCH_MEMSET64
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 879b36602748..fc4c42262694 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -16,8 +16,9 @@ CFLAGS_prom_init.o      += -fPIC
>  CFLAGS_btext.o		+= -fPIC
>  endif
>  
> -CFLAGS_cputable.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
> -CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
> +CFLAGS_early_32.o += -DDISABLE_BRANCH_PROFILING
> +CFLAGS_cputable.o += $(DISABLE_LATENT_ENTROPY_PLUGIN) -DDISABLE_BRANCH_PROFILING
> +CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN) -DDISABLE_BRANCH_PROFILING
>  CFLAGS_btext.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
>  CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
>  
> @@ -31,6 +32,10 @@ CFLAGS_REMOVE_btext.o = $(CC_FLAGS_FTRACE)
>  CFLAGS_REMOVE_prom.o = $(CC_FLAGS_FTRACE)
>  endif
>  
> +KASAN_SANITIZE_early_32.o := n
> +KASAN_SANITIZE_cputable.o := n
> +KASAN_SANITIZE_prom_init.o := n
> +
>  obj-y				:= cputable.o ptrace.o syscalls.o \
>  				   irq.o align.o signal_32.o pmc.o vdso.o \
>  				   process.o systbl.o idle.o \
> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> index 9ffc72ded73a..846fb30b1190 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -783,5 +783,9 @@ int main(void)
>  	DEFINE(VIRT_IMMR_BASE, (u64)__fix_to_virt(FIX_IMMR_BASE));
>  #endif
>  
> +#ifdef CONFIG_KASAN
> +	DEFINE(KASAN_SHADOW_OFFSET, KASAN_SHADOW_OFFSET);
> +#endif
> +
>  	return 0;
>  }
> diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
> index 05b08db3901d..0ec9dec06bc2 100644
> --- a/arch/powerpc/kernel/head_32.S
> +++ b/arch/powerpc/kernel/head_32.S
> @@ -962,6 +962,9 @@ start_here:
>   * Do early platform-specific initialization,
>   * and set up the MMU.
>   */
> +#ifdef CONFIG_KASAN
> +	bl	kasan_early_init
> +#endif
>  	li	r3,0
>  	mr	r4,r31
>  	bl	machine_init
> diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
> index b19d78410511..5d6ff8fa7e2b 100644
> --- a/arch/powerpc/kernel/head_40x.S
> +++ b/arch/powerpc/kernel/head_40x.S
> @@ -848,6 +848,9 @@ start_here:
>  /*
>   * Decide what sort of machine this is and initialize the MMU.
>   */
> +#ifdef CONFIG_KASAN
> +	bl	kasan_early_init
> +#endif
>  	li	r3,0
>  	mr	r4,r31
>  	bl	machine_init
> diff --git a/arch/powerpc/kernel/head_44x.S b/arch/powerpc/kernel/head_44x.S
> index bf23c19c92d6..7ca14dff6192 100644
> --- a/arch/powerpc/kernel/head_44x.S
> +++ b/arch/powerpc/kernel/head_44x.S
> @@ -203,6 +203,9 @@ _ENTRY(_start);
>  /*
>   * Decide what sort of machine this is and initialize the MMU.
>   */
> +#ifdef CONFIG_KASAN
> +	bl	kasan_early_init
> +#endif
>  	li	r3,0
>  	mr	r4,r31
>  	bl	machine_init
> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
> index 0fea10491f3a..6a644ea2e6b6 100644
> --- a/arch/powerpc/kernel/head_8xx.S
> +++ b/arch/powerpc/kernel/head_8xx.S
> @@ -823,6 +823,9 @@ start_here:
>  /*
>   * Decide what sort of machine this is and initialize the MMU.
>   */
> +#ifdef CONFIG_KASAN
> +	bl	kasan_early_init
> +#endif
>  	li	r3,0
>  	mr	r4,r31
>  	bl	machine_init
> diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
> index 2386ce2a9c6e..4f4585a68850 100644
> --- a/arch/powerpc/kernel/head_fsl_booke.S
> +++ b/arch/powerpc/kernel/head_fsl_booke.S
> @@ -274,6 +274,9 @@ set_ivor:
>  /*
>   * Decide what sort of machine this is and initialize the MMU.
>   */
> +#ifdef CONFIG_KASAN
> +	bl	kasan_early_init
> +#endif
>  	mr	r3,r30
>  	mr	r4,r31
>  	bl	machine_init
> diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
> index 667df97d2595..da6bb16e0876 100644
> --- a/arch/powerpc/kernel/prom_init_check.sh
> +++ b/arch/powerpc/kernel/prom_init_check.sh
> @@ -16,8 +16,16 @@
>  # If you really need to reference something from prom_init.o add
>  # it to the list below:
>  
> +grep CONFIG_KASAN=y .config >/dev/null
> +if [ $? -eq 0 ]
> +then
> +	MEMFCT="__memcpy __memset"
> +else
> +	MEMFCT="memcpy memset"
> +fi
> +
>  WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
> -_end enter_prom memcpy memset reloc_offset __secondary_hold
> +_end enter_prom $MEMFCT reloc_offset __secondary_hold
>  __secondary_hold_acknowledge __secondary_hold_spinloop __start
>  strcmp strcpy strlcpy strlen strncmp strstr kstrtobool logo_linux_clut224
>  reloc_got2 kernstart_addr memstart_addr linux_banner _stext
> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> index ca00fbb97cf8..16ff1ea66805 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -978,6 +978,8 @@ void __init setup_arch(char **cmdline_p)
>  
>  	paging_init();
>  
> +	kasan_init();
> +
>  	/* Initialize the MMU context management stuff. */
>  	mmu_context_init();
>  
> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> index 3bf9fc6fd36c..ce8d4a9f810a 100644
> --- a/arch/powerpc/lib/Makefile
> +++ b/arch/powerpc/lib/Makefile
> @@ -8,6 +8,14 @@ ccflags-$(CONFIG_PPC64)	:= $(NO_MINIMAL_TOC)
>  CFLAGS_REMOVE_code-patching.o = $(CC_FLAGS_FTRACE)
>  CFLAGS_REMOVE_feature-fixups.o = $(CC_FLAGS_FTRACE)
>  
> +KASAN_SANITIZE_code-patching.o := n
> +KASAN_SANITIZE_feature-fixups.o := n
> +
> +ifdef CONFIG_KASAN
> +CFLAGS_code-patching.o += -DDISABLE_BRANCH_PROFILING
> +CFLAGS_feature-fixups.o += -DDISABLE_BRANCH_PROFILING
> +endif
> +
>  obj-y += string.o alloc.o code-patching.o feature-fixups.o
>  
>  obj-$(CONFIG_PPC32)	+= div64.o copy_32.o crtsavres.o strlen_32.o
> diff --git a/arch/powerpc/lib/copy_32.S b/arch/powerpc/lib/copy_32.S
> index ba66846fe973..4d8a1c73b4cf 100644
> --- a/arch/powerpc/lib/copy_32.S
> +++ b/arch/powerpc/lib/copy_32.S
> @@ -91,7 +91,8 @@ EXPORT_SYMBOL(memset16)
>   * We therefore skip the optimised bloc that uses dcbz. This jump is
>   * replaced by a nop once cache is active. This is done in machine_init()
>   */
> -_GLOBAL(memset)
> +_GLOBAL(__memset)
> +KASAN_OVERRIDE(memset, __memset)
>  	cmplwi	0,r5,4
>  	blt	7f
>  
> @@ -163,12 +164,14 @@ EXPORT_SYMBOL(memset)
>   * We therefore jump to generic_memcpy which doesn't use dcbz. This jump is
>   * replaced by a nop once cache is active. This is done in machine_init()
>   */
> -_GLOBAL(memmove)
> +_GLOBAL(__memmove)
> +KASAN_OVERRIDE(memmove, __memmove)
>  	cmplw	0,r3,r4
>  	bgt	backwards_memcpy
>  	/* fall through */
>  
> -_GLOBAL(memcpy)
> +_GLOBAL(__memcpy)
> +KASAN_OVERRIDE(memcpy, __memcpy)
>  1:	b	generic_memcpy
>  	patch_site	1b, patch__memcpy_nocache
>  
> diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
> index f965fc33a8b7..d6b76f25f6de 100644
> --- a/arch/powerpc/mm/Makefile
> +++ b/arch/powerpc/mm/Makefile
> @@ -7,6 +7,8 @@ ccflags-$(CONFIG_PPC64)	:= $(NO_MINIMAL_TOC)
>  
>  CFLAGS_REMOVE_slb.o = $(CC_FLAGS_FTRACE)
>  
> +KASAN_SANITIZE_kasan_init.o := n
> +
>  obj-y				:= fault.o mem.o pgtable.o mmap.o \
>  				   init_$(BITS).o pgtable_$(BITS).o \
>  				   init-common.o mmu_context.o drmem.o
> @@ -55,3 +57,4 @@ obj-$(CONFIG_PPC_BOOK3S_64)	+= dump_linuxpagetables-book3s64.o
>  endif
>  obj-$(CONFIG_PPC_HTDUMP)	+= dump_hashpagetable.o
>  obj-$(CONFIG_PPC_MEM_KEYS)	+= pkeys.o
> +obj-$(CONFIG_KASAN)		+= kasan_init.o
> diff --git a/arch/powerpc/mm/dump_linuxpagetables.c b/arch/powerpc/mm/dump_linuxpagetables.c
> index 6aa41669ac1a..c862b48118f1 100644
> --- a/arch/powerpc/mm/dump_linuxpagetables.c
> +++ b/arch/powerpc/mm/dump_linuxpagetables.c
> @@ -94,6 +94,10 @@ static struct addr_marker address_markers[] = {
>  	{ 0,	"Consistent mem start" },
>  	{ 0,	"Consistent mem end" },
>  #endif
> +#ifdef CONFIG_KASAN
> +	{ 0,	"kasan shadow mem start" },
> +	{ 0,	"kasan shadow mem end" },
> +#endif
>  #ifdef CONFIG_HIGHMEM
>  	{ 0,	"Highmem PTEs start" },
>  	{ 0,	"Highmem PTEs end" },
> @@ -310,6 +314,10 @@ static void populate_markers(void)
>  	address_markers[i++].start_address = IOREMAP_TOP +
>  					     CONFIG_CONSISTENT_SIZE;
>  #endif
> +#ifdef CONFIG_KASAN
> +	address_markers[i++].start_address = KASAN_SHADOW_START;
> +	address_markers[i++].start_address = KASAN_SHADOW_END;
> +#endif
>  #ifdef CONFIG_HIGHMEM
>  	address_markers[i++].start_address = PKMAP_BASE;
>  	address_markers[i++].start_address = PKMAP_ADDR(LAST_PKMAP);
> diff --git a/arch/powerpc/mm/kasan_init.c b/arch/powerpc/mm/kasan_init.c
> new file mode 100644
> index 000000000000..bd8e0a263e12
> --- /dev/null
> +++ b/arch/powerpc/mm/kasan_init.c
> @@ -0,0 +1,114 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#define DISABLE_BRANCH_PROFILING
> +
> +#include <linux/kasan.h>
> +#include <linux/printk.h>
> +#include <linux/memblock.h>
> +#include <linux/sched/task.h>
> +#include <asm/pgalloc.h>
> +
> +void __init kasan_early_init(void)
> +{
> +	unsigned long addr = KASAN_SHADOW_START;
> +	unsigned long end = KASAN_SHADOW_END;
> +	unsigned long next;
> +	pmd_t *pmd = pmd_offset(pud_offset(pgd_offset_k(addr), addr), addr);
> +	int i;
> +	phys_addr_t pa = __pa(kasan_early_shadow_page);
> +
> +	BUILD_BUG_ON(KASAN_SHADOW_START & ~PGDIR_MASK);
> +
> +	if (early_mmu_has_feature(MMU_FTR_HPTE_TABLE))
> +		panic("KASAN not supported with Hash MMU\n");
> +
> +	for (i = 0; i < PTRS_PER_PTE; i++)
> +		__set_pte_at(&init_mm, (unsigned long)kasan_early_shadow_page,
> +			     kasan_early_shadow_pte + i,
> +			     pfn_pte(PHYS_PFN(pa), PAGE_KERNEL), 0);
> +
> +	do {
> +		next = pgd_addr_end(addr, end);
> +		pmd_populate_kernel(&init_mm, pmd, kasan_early_shadow_pte);
> +	} while (pmd++, addr = next, addr != end);
> +}
> +
> +static void __init kasan_init_region(struct memblock_region *reg)
> +{
> +	void *start = __va(reg->base);
> +	void *end = __va(reg->base + reg->size);
> +	unsigned long k_start, k_end, k_cur, k_next;
> +	pmd_t *pmd;
> +	void *block;
> +
> +	if (start >= end)
> +		return;
> +
> +	k_start = (unsigned long)kasan_mem_to_shadow(start);
> +	k_end = (unsigned long)kasan_mem_to_shadow(end);
> +	pmd = pmd_offset(pud_offset(pgd_offset_k(k_start), k_start), k_start);
> +
> +	for (k_cur = k_start; k_cur != k_end; k_cur = k_next, pmd++) {
> +		k_next = pgd_addr_end(k_cur, k_end);
> +		if ((void *)pmd_page_vaddr(*pmd) == kasan_early_shadow_pte) {
> +			pte_t *new = pte_alloc_one_kernel(&init_mm);
> +
> +			if (!new)
> +				panic("kasan: pte_alloc_one_kernel() failed");
> +			memcpy(new, kasan_early_shadow_pte, PTE_TABLE_SIZE);
> +			pmd_populate_kernel(&init_mm, pmd, new);
> +		}
> +	};
> +
> +	block = memblock_alloc(k_end - k_start, PAGE_SIZE);
> +	for (k_cur = k_start; k_cur < k_end; k_cur += PAGE_SIZE) {
> +		void *va = block ? block + k_cur - k_start :
> +				   memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> +		pte_t pte = pfn_pte(PHYS_PFN(__pa(va)), PAGE_KERNEL);
> +
> +		if (!va)
> +			panic("kasan: memblock_alloc() failed");
> +		pmd = pmd_offset(pud_offset(pgd_offset_k(k_cur), k_cur), k_cur);
> +		pte_update(pte_offset_kernel(pmd, k_cur), ~0, pte_val(pte));
> +	}
> +	flush_tlb_kernel_range(k_start, k_end);
> +}
> +
> +static void __init kasan_remap_early_shadow_ro(void)
> +{
> +	unsigned long k_cur;
> +	phys_addr_t pa = __pa(kasan_early_shadow_page);
> +	int i;
> +
> +	for (i = 0; i < PTRS_PER_PTE; i++)
> +		ptep_set_wrprotect(&init_mm, 0, kasan_early_shadow_pte + i);
> +
> +	for (k_cur = PAGE_OFFSET & PAGE_MASK; k_cur; k_cur += PAGE_SIZE) {
> +		pmd_t *pmd = pmd_offset(pud_offset(pgd_offset_k(k_cur), k_cur), k_cur);
> +		pte_t *ptep = pte_offset_kernel(pmd, k_cur);
> +
> +		if ((void *)pmd_page_vaddr(*pmd) == kasan_early_shadow_pte)
> +			continue;
> +		if ((pte_val(*ptep) & PAGE_MASK) != pa)
> +			continue;
> +
> +		ptep_set_wrprotect(&init_mm, k_cur, ptep);
> +	}
> +	flush_tlb_mm(&init_mm);
> +}
> +
> +void __init kasan_init(void)
> +{
> +	struct memblock_region *reg;
> +
> +	for_each_memblock(memory, reg)
> +		kasan_init_region(reg);
> +
> +	kasan_remap_early_shadow_ro();
> +
> +	clear_page(kasan_early_shadow_page);
> +
> +	/* At this point kasan is fully initialized. Enable error messages */
> +	init_task.kasan_depth = 0;
> +	pr_info("KASAN init done\n");
> +}
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index 81f251fc4169..1bb055775e60 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -336,6 +336,10 @@ void __init mem_init(void)
>  	pr_info("  * 0x%08lx..0x%08lx  : highmem PTEs\n",
>  		PKMAP_BASE, PKMAP_ADDR(LAST_PKMAP));
>  #endif /* CONFIG_HIGHMEM */
> +#ifdef CONFIG_KASAN
> +	pr_info("  * 0x%08lx..0x%08lx  : kasan shadow mem\n",
> +		KASAN_SHADOW_START, KASAN_SHADOW_END);
> +#endif
>  #ifdef CONFIG_NOT_COHERENT_CACHE
>  	pr_info("  * 0x%08lx..0x%08lx  : consistent mem\n",
>  		IOREMAP_TOP, IOREMAP_TOP + CONFIG_CONSISTENT_SIZE);
> -- 
> 2.13.3

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

* Re: [PATCH v5 3/3] powerpc/32: Add KASAN support
  2019-02-14 22:04   ` Daniel Axtens
@ 2019-02-15  8:41     ` Christophe Leroy
  2019-02-15 10:01       ` Andrey Ryabinin
  0 siblings, 1 reply; 11+ messages in thread
From: Christophe Leroy @ 2019-02-15  8:41 UTC (permalink / raw)
  To: Daniel Axtens, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Nicholas Piggin, Aneesh Kumar K.V,
	Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev



Le 14/02/2019 à 23:04, Daniel Axtens a écrit :
> Hi Christophe,
> 
>> --- a/arch/powerpc/include/asm/string.h
>> +++ b/arch/powerpc/include/asm/string.h
>> @@ -27,6 +27,20 @@ extern int memcmp(const void *,const void *,__kernel_size_t);
>>   extern void * memchr(const void *,int,__kernel_size_t);
>>   extern void * memcpy_flushcache(void *,const void *,__kernel_size_t);
>>   
>> +void *__memset(void *s, int c, __kernel_size_t count);
>> +void *__memcpy(void *to, const void *from, __kernel_size_t n);
>> +void *__memmove(void *to, const void *from, __kernel_size_t n);
>> +
>> +#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
>> +/*
>> + * For files that are 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
>> +
> 
> I'm finding that I miss tests like 'kasan test: kasan_memcmp
> out-of-bounds in memcmp' because the uninstrumented asm version is used
> instead of an instrumented C version. I ended up guarding the relevant
> __HAVE_ARCH_x symbols behind a #ifndef CONFIG_KASAN and only exporting
> the arch versions if we're not compiled with KASAN.
> 
> I find I need to guard and unexport strncpy, strncmp, memchr and
> memcmp. Do you need to do this on 32bit as well, or are those tests
> passing anyway for some reason?

Indeed, I didn't try the KASAN test module recently, because my configs 
don't have CONFIG_MODULE by default.

Trying to test it now, I am discovering that module loading oopses with 
latest version of my series, I need to figure out exactly why. Here 
below the oops by modprobing test_module (the one supposed to just say 
hello to the world).

What we see is an access to the RO kasan zero area.

The shadow mem is 0xf7c00000..0xffc00000
Linear kernel memory is shadowed by 0xf7c00000-0xf8bfffff
0xf8c00000-0xffc00000 is shadowed read only by the kasan zero page.

Why is kasan trying to access that ? Isn't kasan supposed to not check 
stuff in vmalloc area ?

[  189.087499] BUG: Unable to handle kernel data access at 0xf8eb7818
[  189.093455] Faulting instruction address: 0xc001ab60
[  189.098383] Oops: Kernel access of bad area, sig: 11 [#1]
[  189.103732] BE PAGE_SIZE=16K PREEMPT CMPC885
[  189.111414] Modules linked in: test_module(+)
[  189.115817] CPU: 0 PID: 514 Comm: modprobe Not tainted 
5.0.0-rc5-s3k-dev-00645-g1dd3acf23952 #1016
[  189.124627] NIP:  c001ab60 LR: c0194fe8 CTR: 00000003
[  189.129641] REGS: c5645b90 TRAP: 0300   Not tainted 
(5.0.0-rc5-s3k-dev-00645-g1dd3acf23952)
[  189.137924] MSR:  00009032 <EE,ME,IR,DR,RI>  CR: 44002884  XER: 00000000
[  189.144571] DAR: f8eb7818 DSISR: 8a000000
[  189.144571] GPR00: c0196620 c5645c40 c5aac000 f8eb7818 00000000 
00000003 f8eb7817 c01950d0
[  189.144571] GPR08: c00c2720 c95bc010 00000000 c95bc1a0 c01965ec 
100d7b30 c0802b80 c5ae0308
[  189.144571] GPR16: c5990480 00000124 0000000f c00bcf7c c5ae0324 
c95bc32c 000006b8 00000001
[  189.144571] GPR24: c95bc364 c95bc360 c95bc2c0 c95bc1a0 00000002 
00000000 00000018 f8eb781b
[  189.182611] NIP [c001ab60] __memset+0xb4/0xc0
[  189.186922] LR [c0194fe8] kasan_unpoison_shadow+0x34/0x54
[  189.192136] Call Trace:
[  189.194682] [c5645c50] [c0196620] __asan_register_globals+0x34/0x74
[  189.200900] [c5645c70] [c00c27a4] do_init_module+0xbc/0x5a4
[  189.206392] [c5645ca0] [c00c0d08] load_module+0x2b5c/0x3194
[  189.211901] [c5645e70] [c00c14c8] sys_init_module+0x188/0x1bc
[  189.217572] [c5645f40] [c001311c] ret_from_syscall+0x0/0x38
[  189.223049] --- interrupt: c01 at 0xfda2b50
[  189.223049]     LR = 0x10014b18
[  189.230175] Instruction dump:
[  189.233117] 4200fffc 70a50003 4d820020 7ca903a6 38c60003 9c860001 
4200fffc 4e800020
[  189.240859] 2c050000 4d820020 7ca903a6 38c3ffff <9c860001> 4200fffc 
4e800020 7c032040
[  189.248809] ---[ end trace 45cbb1b3215e5959 ]---

Christophe

> 
> Regards,
> Daniel
> 
> 
>>   #ifdef CONFIG_PPC64
>>   #define __HAVE_ARCH_MEMSET32
>>   #define __HAVE_ARCH_MEMSET64
>> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
>> index 879b36602748..fc4c42262694 100644
>> --- a/arch/powerpc/kernel/Makefile
>> +++ b/arch/powerpc/kernel/Makefile
>> @@ -16,8 +16,9 @@ CFLAGS_prom_init.o      += -fPIC
>>   CFLAGS_btext.o		+= -fPIC
>>   endif
>>   
>> -CFLAGS_cputable.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
>> -CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
>> +CFLAGS_early_32.o += -DDISABLE_BRANCH_PROFILING
>> +CFLAGS_cputable.o += $(DISABLE_LATENT_ENTROPY_PLUGIN) -DDISABLE_BRANCH_PROFILING
>> +CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN) -DDISABLE_BRANCH_PROFILING
>>   CFLAGS_btext.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
>>   CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
>>   
>> @@ -31,6 +32,10 @@ CFLAGS_REMOVE_btext.o = $(CC_FLAGS_FTRACE)
>>   CFLAGS_REMOVE_prom.o = $(CC_FLAGS_FTRACE)
>>   endif
>>   
>> +KASAN_SANITIZE_early_32.o := n
>> +KASAN_SANITIZE_cputable.o := n
>> +KASAN_SANITIZE_prom_init.o := n
>> +
>>   obj-y				:= cputable.o ptrace.o syscalls.o \
>>   				   irq.o align.o signal_32.o pmc.o vdso.o \
>>   				   process.o systbl.o idle.o \
>> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
>> index 9ffc72ded73a..846fb30b1190 100644
>> --- a/arch/powerpc/kernel/asm-offsets.c
>> +++ b/arch/powerpc/kernel/asm-offsets.c
>> @@ -783,5 +783,9 @@ int main(void)
>>   	DEFINE(VIRT_IMMR_BASE, (u64)__fix_to_virt(FIX_IMMR_BASE));
>>   #endif
>>   
>> +#ifdef CONFIG_KASAN
>> +	DEFINE(KASAN_SHADOW_OFFSET, KASAN_SHADOW_OFFSET);
>> +#endif
>> +
>>   	return 0;
>>   }
>> diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
>> index 05b08db3901d..0ec9dec06bc2 100644
>> --- a/arch/powerpc/kernel/head_32.S
>> +++ b/arch/powerpc/kernel/head_32.S
>> @@ -962,6 +962,9 @@ start_here:
>>    * Do early platform-specific initialization,
>>    * and set up the MMU.
>>    */
>> +#ifdef CONFIG_KASAN
>> +	bl	kasan_early_init
>> +#endif
>>   	li	r3,0
>>   	mr	r4,r31
>>   	bl	machine_init
>> diff --git a/arch/powerpc/kernel/head_40x.S b/arch/powerpc/kernel/head_40x.S
>> index b19d78410511..5d6ff8fa7e2b 100644
>> --- a/arch/powerpc/kernel/head_40x.S
>> +++ b/arch/powerpc/kernel/head_40x.S
>> @@ -848,6 +848,9 @@ start_here:
>>   /*
>>    * Decide what sort of machine this is and initialize the MMU.
>>    */
>> +#ifdef CONFIG_KASAN
>> +	bl	kasan_early_init
>> +#endif
>>   	li	r3,0
>>   	mr	r4,r31
>>   	bl	machine_init
>> diff --git a/arch/powerpc/kernel/head_44x.S b/arch/powerpc/kernel/head_44x.S
>> index bf23c19c92d6..7ca14dff6192 100644
>> --- a/arch/powerpc/kernel/head_44x.S
>> +++ b/arch/powerpc/kernel/head_44x.S
>> @@ -203,6 +203,9 @@ _ENTRY(_start);
>>   /*
>>    * Decide what sort of machine this is and initialize the MMU.
>>    */
>> +#ifdef CONFIG_KASAN
>> +	bl	kasan_early_init
>> +#endif
>>   	li	r3,0
>>   	mr	r4,r31
>>   	bl	machine_init
>> diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S
>> index 0fea10491f3a..6a644ea2e6b6 100644
>> --- a/arch/powerpc/kernel/head_8xx.S
>> +++ b/arch/powerpc/kernel/head_8xx.S
>> @@ -823,6 +823,9 @@ start_here:
>>   /*
>>    * Decide what sort of machine this is and initialize the MMU.
>>    */
>> +#ifdef CONFIG_KASAN
>> +	bl	kasan_early_init
>> +#endif
>>   	li	r3,0
>>   	mr	r4,r31
>>   	bl	machine_init
>> diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
>> index 2386ce2a9c6e..4f4585a68850 100644
>> --- a/arch/powerpc/kernel/head_fsl_booke.S
>> +++ b/arch/powerpc/kernel/head_fsl_booke.S
>> @@ -274,6 +274,9 @@ set_ivor:
>>   /*
>>    * Decide what sort of machine this is and initialize the MMU.
>>    */
>> +#ifdef CONFIG_KASAN
>> +	bl	kasan_early_init
>> +#endif
>>   	mr	r3,r30
>>   	mr	r4,r31
>>   	bl	machine_init
>> diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
>> index 667df97d2595..da6bb16e0876 100644
>> --- a/arch/powerpc/kernel/prom_init_check.sh
>> +++ b/arch/powerpc/kernel/prom_init_check.sh
>> @@ -16,8 +16,16 @@
>>   # If you really need to reference something from prom_init.o add
>>   # it to the list below:
>>   
>> +grep CONFIG_KASAN=y .config >/dev/null
>> +if [ $? -eq 0 ]
>> +then
>> +	MEMFCT="__memcpy __memset"
>> +else
>> +	MEMFCT="memcpy memset"
>> +fi
>> +
>>   WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
>> -_end enter_prom memcpy memset reloc_offset __secondary_hold
>> +_end enter_prom $MEMFCT reloc_offset __secondary_hold
>>   __secondary_hold_acknowledge __secondary_hold_spinloop __start
>>   strcmp strcpy strlcpy strlen strncmp strstr kstrtobool logo_linux_clut224
>>   reloc_got2 kernstart_addr memstart_addr linux_banner _stext
>> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
>> index ca00fbb97cf8..16ff1ea66805 100644
>> --- a/arch/powerpc/kernel/setup-common.c
>> +++ b/arch/powerpc/kernel/setup-common.c
>> @@ -978,6 +978,8 @@ void __init setup_arch(char **cmdline_p)
>>   
>>   	paging_init();
>>   
>> +	kasan_init();
>> +
>>   	/* Initialize the MMU context management stuff. */
>>   	mmu_context_init();
>>   
>> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
>> index 3bf9fc6fd36c..ce8d4a9f810a 100644
>> --- a/arch/powerpc/lib/Makefile
>> +++ b/arch/powerpc/lib/Makefile
>> @@ -8,6 +8,14 @@ ccflags-$(CONFIG_PPC64)	:= $(NO_MINIMAL_TOC)
>>   CFLAGS_REMOVE_code-patching.o = $(CC_FLAGS_FTRACE)
>>   CFLAGS_REMOVE_feature-fixups.o = $(CC_FLAGS_FTRACE)
>>   
>> +KASAN_SANITIZE_code-patching.o := n
>> +KASAN_SANITIZE_feature-fixups.o := n
>> +
>> +ifdef CONFIG_KASAN
>> +CFLAGS_code-patching.o += -DDISABLE_BRANCH_PROFILING
>> +CFLAGS_feature-fixups.o += -DDISABLE_BRANCH_PROFILING
>> +endif
>> +
>>   obj-y += string.o alloc.o code-patching.o feature-fixups.o
>>   
>>   obj-$(CONFIG_PPC32)	+= div64.o copy_32.o crtsavres.o strlen_32.o
>> diff --git a/arch/powerpc/lib/copy_32.S b/arch/powerpc/lib/copy_32.S
>> index ba66846fe973..4d8a1c73b4cf 100644
>> --- a/arch/powerpc/lib/copy_32.S
>> +++ b/arch/powerpc/lib/copy_32.S
>> @@ -91,7 +91,8 @@ EXPORT_SYMBOL(memset16)
>>    * We therefore skip the optimised bloc that uses dcbz. This jump is
>>    * replaced by a nop once cache is active. This is done in machine_init()
>>    */
>> -_GLOBAL(memset)
>> +_GLOBAL(__memset)
>> +KASAN_OVERRIDE(memset, __memset)
>>   	cmplwi	0,r5,4
>>   	blt	7f
>>   
>> @@ -163,12 +164,14 @@ EXPORT_SYMBOL(memset)
>>    * We therefore jump to generic_memcpy which doesn't use dcbz. This jump is
>>    * replaced by a nop once cache is active. This is done in machine_init()
>>    */
>> -_GLOBAL(memmove)
>> +_GLOBAL(__memmove)
>> +KASAN_OVERRIDE(memmove, __memmove)
>>   	cmplw	0,r3,r4
>>   	bgt	backwards_memcpy
>>   	/* fall through */
>>   
>> -_GLOBAL(memcpy)
>> +_GLOBAL(__memcpy)
>> +KASAN_OVERRIDE(memcpy, __memcpy)
>>   1:	b	generic_memcpy
>>   	patch_site	1b, patch__memcpy_nocache
>>   
>> diff --git a/arch/powerpc/mm/Makefile b/arch/powerpc/mm/Makefile
>> index f965fc33a8b7..d6b76f25f6de 100644
>> --- a/arch/powerpc/mm/Makefile
>> +++ b/arch/powerpc/mm/Makefile
>> @@ -7,6 +7,8 @@ ccflags-$(CONFIG_PPC64)	:= $(NO_MINIMAL_TOC)
>>   
>>   CFLAGS_REMOVE_slb.o = $(CC_FLAGS_FTRACE)
>>   
>> +KASAN_SANITIZE_kasan_init.o := n
>> +
>>   obj-y				:= fault.o mem.o pgtable.o mmap.o \
>>   				   init_$(BITS).o pgtable_$(BITS).o \
>>   				   init-common.o mmu_context.o drmem.o
>> @@ -55,3 +57,4 @@ obj-$(CONFIG_PPC_BOOK3S_64)	+= dump_linuxpagetables-book3s64.o
>>   endif
>>   obj-$(CONFIG_PPC_HTDUMP)	+= dump_hashpagetable.o
>>   obj-$(CONFIG_PPC_MEM_KEYS)	+= pkeys.o
>> +obj-$(CONFIG_KASAN)		+= kasan_init.o
>> diff --git a/arch/powerpc/mm/dump_linuxpagetables.c b/arch/powerpc/mm/dump_linuxpagetables.c
>> index 6aa41669ac1a..c862b48118f1 100644
>> --- a/arch/powerpc/mm/dump_linuxpagetables.c
>> +++ b/arch/powerpc/mm/dump_linuxpagetables.c
>> @@ -94,6 +94,10 @@ static struct addr_marker address_markers[] = {
>>   	{ 0,	"Consistent mem start" },
>>   	{ 0,	"Consistent mem end" },
>>   #endif
>> +#ifdef CONFIG_KASAN
>> +	{ 0,	"kasan shadow mem start" },
>> +	{ 0,	"kasan shadow mem end" },
>> +#endif
>>   #ifdef CONFIG_HIGHMEM
>>   	{ 0,	"Highmem PTEs start" },
>>   	{ 0,	"Highmem PTEs end" },
>> @@ -310,6 +314,10 @@ static void populate_markers(void)
>>   	address_markers[i++].start_address = IOREMAP_TOP +
>>   					     CONFIG_CONSISTENT_SIZE;
>>   #endif
>> +#ifdef CONFIG_KASAN
>> +	address_markers[i++].start_address = KASAN_SHADOW_START;
>> +	address_markers[i++].start_address = KASAN_SHADOW_END;
>> +#endif
>>   #ifdef CONFIG_HIGHMEM
>>   	address_markers[i++].start_address = PKMAP_BASE;
>>   	address_markers[i++].start_address = PKMAP_ADDR(LAST_PKMAP);
>> diff --git a/arch/powerpc/mm/kasan_init.c b/arch/powerpc/mm/kasan_init.c
>> new file mode 100644
>> index 000000000000..bd8e0a263e12
>> --- /dev/null
>> +++ b/arch/powerpc/mm/kasan_init.c
>> @@ -0,0 +1,114 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#define DISABLE_BRANCH_PROFILING
>> +
>> +#include <linux/kasan.h>
>> +#include <linux/printk.h>
>> +#include <linux/memblock.h>
>> +#include <linux/sched/task.h>
>> +#include <asm/pgalloc.h>
>> +
>> +void __init kasan_early_init(void)
>> +{
>> +	unsigned long addr = KASAN_SHADOW_START;
>> +	unsigned long end = KASAN_SHADOW_END;
>> +	unsigned long next;
>> +	pmd_t *pmd = pmd_offset(pud_offset(pgd_offset_k(addr), addr), addr);
>> +	int i;
>> +	phys_addr_t pa = __pa(kasan_early_shadow_page);
>> +
>> +	BUILD_BUG_ON(KASAN_SHADOW_START & ~PGDIR_MASK);
>> +
>> +	if (early_mmu_has_feature(MMU_FTR_HPTE_TABLE))
>> +		panic("KASAN not supported with Hash MMU\n");
>> +
>> +	for (i = 0; i < PTRS_PER_PTE; i++)
>> +		__set_pte_at(&init_mm, (unsigned long)kasan_early_shadow_page,
>> +			     kasan_early_shadow_pte + i,
>> +			     pfn_pte(PHYS_PFN(pa), PAGE_KERNEL), 0);
>> +
>> +	do {
>> +		next = pgd_addr_end(addr, end);
>> +		pmd_populate_kernel(&init_mm, pmd, kasan_early_shadow_pte);
>> +	} while (pmd++, addr = next, addr != end);
>> +}
>> +
>> +static void __init kasan_init_region(struct memblock_region *reg)
>> +{
>> +	void *start = __va(reg->base);
>> +	void *end = __va(reg->base + reg->size);
>> +	unsigned long k_start, k_end, k_cur, k_next;
>> +	pmd_t *pmd;
>> +	void *block;
>> +
>> +	if (start >= end)
>> +		return;
>> +
>> +	k_start = (unsigned long)kasan_mem_to_shadow(start);
>> +	k_end = (unsigned long)kasan_mem_to_shadow(end);
>> +	pmd = pmd_offset(pud_offset(pgd_offset_k(k_start), k_start), k_start);
>> +
>> +	for (k_cur = k_start; k_cur != k_end; k_cur = k_next, pmd++) {
>> +		k_next = pgd_addr_end(k_cur, k_end);
>> +		if ((void *)pmd_page_vaddr(*pmd) == kasan_early_shadow_pte) {
>> +			pte_t *new = pte_alloc_one_kernel(&init_mm);
>> +
>> +			if (!new)
>> +				panic("kasan: pte_alloc_one_kernel() failed");
>> +			memcpy(new, kasan_early_shadow_pte, PTE_TABLE_SIZE);
>> +			pmd_populate_kernel(&init_mm, pmd, new);
>> +		}
>> +	};
>> +
>> +	block = memblock_alloc(k_end - k_start, PAGE_SIZE);
>> +	for (k_cur = k_start; k_cur < k_end; k_cur += PAGE_SIZE) {
>> +		void *va = block ? block + k_cur - k_start :
>> +				   memblock_alloc(PAGE_SIZE, PAGE_SIZE);
>> +		pte_t pte = pfn_pte(PHYS_PFN(__pa(va)), PAGE_KERNEL);
>> +
>> +		if (!va)
>> +			panic("kasan: memblock_alloc() failed");
>> +		pmd = pmd_offset(pud_offset(pgd_offset_k(k_cur), k_cur), k_cur);
>> +		pte_update(pte_offset_kernel(pmd, k_cur), ~0, pte_val(pte));
>> +	}
>> +	flush_tlb_kernel_range(k_start, k_end);
>> +}
>> +
>> +static void __init kasan_remap_early_shadow_ro(void)
>> +{
>> +	unsigned long k_cur;
>> +	phys_addr_t pa = __pa(kasan_early_shadow_page);
>> +	int i;
>> +
>> +	for (i = 0; i < PTRS_PER_PTE; i++)
>> +		ptep_set_wrprotect(&init_mm, 0, kasan_early_shadow_pte + i);
>> +
>> +	for (k_cur = PAGE_OFFSET & PAGE_MASK; k_cur; k_cur += PAGE_SIZE) {
>> +		pmd_t *pmd = pmd_offset(pud_offset(pgd_offset_k(k_cur), k_cur), k_cur);
>> +		pte_t *ptep = pte_offset_kernel(pmd, k_cur);
>> +
>> +		if ((void *)pmd_page_vaddr(*pmd) == kasan_early_shadow_pte)
>> +			continue;
>> +		if ((pte_val(*ptep) & PAGE_MASK) != pa)
>> +			continue;
>> +
>> +		ptep_set_wrprotect(&init_mm, k_cur, ptep);
>> +	}
>> +	flush_tlb_mm(&init_mm);
>> +}
>> +
>> +void __init kasan_init(void)
>> +{
>> +	struct memblock_region *reg;
>> +
>> +	for_each_memblock(memory, reg)
>> +		kasan_init_region(reg);
>> +
>> +	kasan_remap_early_shadow_ro();
>> +
>> +	clear_page(kasan_early_shadow_page);
>> +
>> +	/* At this point kasan is fully initialized. Enable error messages */
>> +	init_task.kasan_depth = 0;
>> +	pr_info("KASAN init done\n");
>> +}
>> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
>> index 81f251fc4169..1bb055775e60 100644
>> --- a/arch/powerpc/mm/mem.c
>> +++ b/arch/powerpc/mm/mem.c
>> @@ -336,6 +336,10 @@ void __init mem_init(void)
>>   	pr_info("  * 0x%08lx..0x%08lx  : highmem PTEs\n",
>>   		PKMAP_BASE, PKMAP_ADDR(LAST_PKMAP));
>>   #endif /* CONFIG_HIGHMEM */
>> +#ifdef CONFIG_KASAN
>> +	pr_info("  * 0x%08lx..0x%08lx  : kasan shadow mem\n",
>> +		KASAN_SHADOW_START, KASAN_SHADOW_END);
>> +#endif
>>   #ifdef CONFIG_NOT_COHERENT_CACHE
>>   	pr_info("  * 0x%08lx..0x%08lx  : consistent mem\n",
>>   		IOREMAP_TOP, IOREMAP_TOP + CONFIG_CONSISTENT_SIZE);
>> -- 
>> 2.13.3

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

* Re: [PATCH v5 3/3] powerpc/32: Add KASAN support
  2019-02-15  8:41     ` Christophe Leroy
@ 2019-02-15 10:01       ` Andrey Ryabinin
  2019-02-15 10:10         ` Christophe Leroy
  0 siblings, 1 reply; 11+ messages in thread
From: Andrey Ryabinin @ 2019-02-15 10:01 UTC (permalink / raw)
  To: Christophe Leroy, Daniel Axtens, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Nicholas Piggin,
	Aneesh Kumar K.V, Alexander Potapenko, Dmitry Vyukov
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev



On 2/15/19 11:41 AM, Christophe Leroy wrote:
> 
> 
> Le 14/02/2019 à 23:04, Daniel Axtens a écrit :
>> Hi Christophe,
>>
>>> --- a/arch/powerpc/include/asm/string.h
>>> +++ b/arch/powerpc/include/asm/string.h
>>> @@ -27,6 +27,20 @@ extern int memcmp(const void *,const void *,__kernel_size_t);
>>>   extern void * memchr(const void *,int,__kernel_size_t);
>>>   extern void * memcpy_flushcache(void *,const void *,__kernel_size_t);
>>>   +void *__memset(void *s, int c, __kernel_size_t count);
>>> +void *__memcpy(void *to, const void *from, __kernel_size_t n);
>>> +void *__memmove(void *to, const void *from, __kernel_size_t n);
>>> +
>>> +#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
>>> +/*
>>> + * For files that are 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
>>> +
>>
>> I'm finding that I miss tests like 'kasan test: kasan_memcmp
>> out-of-bounds in memcmp' because the uninstrumented asm version is used
>> instead of an instrumented C version. I ended up guarding the relevant
>> __HAVE_ARCH_x symbols behind a #ifndef CONFIG_KASAN and only exporting
>> the arch versions if we're not compiled with KASAN.
>>
>> I find I need to guard and unexport strncpy, strncmp, memchr and
>> memcmp. Do you need to do this on 32bit as well, or are those tests
>> passing anyway for some reason?
> 
> Indeed, I didn't try the KASAN test module recently, because my configs don't have CONFIG_MODULE by default.
> 
> Trying to test it now, I am discovering that module loading oopses with latest version of my series, I need to figure out exactly why. Here below the oops by modprobing test_module (the one supposed to just say hello to the world).
> 
> What we see is an access to the RO kasan zero area.
> 
> The shadow mem is 0xf7c00000..0xffc00000
> Linear kernel memory is shadowed by 0xf7c00000-0xf8bfffff
> 0xf8c00000-0xffc00000 is shadowed read only by the kasan zero page.
> 
> Why is kasan trying to access that ? Isn't kasan supposed to not check stuff in vmalloc area ?

It tries to poison global variables in modules. If module is in vmalloc, than it will try to poison vmalloc.
Given that the vmalloc area is not so big on 32bits, the easiest solution is to cover all vmalloc with RW shadow.




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

* Re: [PATCH v5 3/3] powerpc/32: Add KASAN support
  2019-02-15 10:01       ` Andrey Ryabinin
@ 2019-02-15 10:10         ` Christophe Leroy
  2019-02-15 10:38           ` Andrey Ryabinin
  0 siblings, 1 reply; 11+ messages in thread
From: Christophe Leroy @ 2019-02-15 10:10 UTC (permalink / raw)
  To: Andrey Ryabinin, Daniel Axtens, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Nicholas Piggin,
	Aneesh Kumar K.V, Alexander Potapenko, Dmitry Vyukov
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev



Le 15/02/2019 à 11:01, Andrey Ryabinin a écrit :
> 
> 
> On 2/15/19 11:41 AM, Christophe Leroy wrote:
>>
>>
>> Le 14/02/2019 à 23:04, Daniel Axtens a écrit :
>>> Hi Christophe,
>>>
>>>> --- a/arch/powerpc/include/asm/string.h
>>>> +++ b/arch/powerpc/include/asm/string.h
>>>> @@ -27,6 +27,20 @@ extern int memcmp(const void *,const void *,__kernel_size_t);
>>>>    extern void * memchr(const void *,int,__kernel_size_t);
>>>>    extern void * memcpy_flushcache(void *,const void *,__kernel_size_t);
>>>>    +void *__memset(void *s, int c, __kernel_size_t count);
>>>> +void *__memcpy(void *to, const void *from, __kernel_size_t n);
>>>> +void *__memmove(void *to, const void *from, __kernel_size_t n);
>>>> +
>>>> +#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
>>>> +/*
>>>> + * For files that are 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
>>>> +
>>>
>>> I'm finding that I miss tests like 'kasan test: kasan_memcmp
>>> out-of-bounds in memcmp' because the uninstrumented asm version is used
>>> instead of an instrumented C version. I ended up guarding the relevant
>>> __HAVE_ARCH_x symbols behind a #ifndef CONFIG_KASAN and only exporting
>>> the arch versions if we're not compiled with KASAN.
>>>
>>> I find I need to guard and unexport strncpy, strncmp, memchr and
>>> memcmp. Do you need to do this on 32bit as well, or are those tests
>>> passing anyway for some reason?
>>
>> Indeed, I didn't try the KASAN test module recently, because my configs don't have CONFIG_MODULE by default.
>>
>> Trying to test it now, I am discovering that module loading oopses with latest version of my series, I need to figure out exactly why. Here below the oops by modprobing test_module (the one supposed to just say hello to the world).
>>
>> What we see is an access to the RO kasan zero area.
>>
>> The shadow mem is 0xf7c00000..0xffc00000
>> Linear kernel memory is shadowed by 0xf7c00000-0xf8bfffff
>> 0xf8c00000-0xffc00000 is shadowed read only by the kasan zero page.
>>
>> Why is kasan trying to access that ? Isn't kasan supposed to not check stuff in vmalloc area ?
> 
> It tries to poison global variables in modules. If module is in vmalloc, than it will try to poison vmalloc.
> Given that the vmalloc area is not so big on 32bits, the easiest solution is to cover all vmalloc with RW shadow.
> 

Euh ... Not so big ?

Memory: 96448K/131072K available (8016K kernel code, 1680K rwdata
, 2720K rodata, 624K init, 4678K bss, 34624K reserved, 0K cma-reserved)
Kernel virtual memory layout:
   * 0xffefc000..0xffffc000  : fixmap
   * 0xf7c00000..0xffc00000  : kasan shadow mem
   * 0xf7a00000..0xf7c00000  : consistent mem
   * 0xf7a00000..0xf7a00000  : early ioremap
   * 0xc9000000..0xf7a00000  : vmalloc & ioremap

Here, vmalloc area size 0x2ea00000, that is 746Mbytes. Shadow for this 
would be 93Mbytes and we are already using 16Mbytes to shadow the linear 
memory area .... this poor board has 128Mbytes RAM in total.

So another solution is needed.

Christophe

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

* Re: [PATCH v5 3/3] powerpc/32: Add KASAN support
  2019-02-15 10:10         ` Christophe Leroy
@ 2019-02-15 10:38           ` Andrey Ryabinin
  0 siblings, 0 replies; 11+ messages in thread
From: Andrey Ryabinin @ 2019-02-15 10:38 UTC (permalink / raw)
  To: Christophe Leroy, Daniel Axtens, Benjamin Herrenschmidt,
	Paul Mackerras, Michael Ellerman, Nicholas Piggin,
	Aneesh Kumar K.V, Alexander Potapenko, Dmitry Vyukov
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev



On 2/15/19 1:10 PM, Christophe Leroy wrote:
> 
> 
> Le 15/02/2019 à 11:01, Andrey Ryabinin a écrit :
>>
>>
>> On 2/15/19 11:41 AM, Christophe Leroy wrote:
>>>
>>>
>>> Le 14/02/2019 à 23:04, Daniel Axtens a écrit :
>>>> Hi Christophe,
>>>>
>>>>> --- a/arch/powerpc/include/asm/string.h
>>>>> +++ b/arch/powerpc/include/asm/string.h
>>>>> @@ -27,6 +27,20 @@ extern int memcmp(const void *,const void *,__kernel_size_t);
>>>>>    extern void * memchr(const void *,int,__kernel_size_t);
>>>>>    extern void * memcpy_flushcache(void *,const void *,__kernel_size_t);
>>>>>    +void *__memset(void *s, int c, __kernel_size_t count);
>>>>> +void *__memcpy(void *to, const void *from, __kernel_size_t n);
>>>>> +void *__memmove(void *to, const void *from, __kernel_size_t n);
>>>>> +
>>>>> +#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
>>>>> +/*
>>>>> + * For files that are 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
>>>>> +
>>>>
>>>> I'm finding that I miss tests like 'kasan test: kasan_memcmp
>>>> out-of-bounds in memcmp' because the uninstrumented asm version is used
>>>> instead of an instrumented C version. I ended up guarding the relevant
>>>> __HAVE_ARCH_x symbols behind a #ifndef CONFIG_KASAN and only exporting
>>>> the arch versions if we're not compiled with KASAN.
>>>>
>>>> I find I need to guard and unexport strncpy, strncmp, memchr and
>>>> memcmp. Do you need to do this on 32bit as well, or are those tests
>>>> passing anyway for some reason?
>>>
>>> Indeed, I didn't try the KASAN test module recently, because my configs don't have CONFIG_MODULE by default.
>>>
>>> Trying to test it now, I am discovering that module loading oopses with latest version of my series, I need to figure out exactly why. Here below the oops by modprobing test_module (the one supposed to just say hello to the world).
>>>
>>> What we see is an access to the RO kasan zero area.
>>>
>>> The shadow mem is 0xf7c00000..0xffc00000
>>> Linear kernel memory is shadowed by 0xf7c00000-0xf8bfffff
>>> 0xf8c00000-0xffc00000 is shadowed read only by the kasan zero page.
>>>
>>> Why is kasan trying to access that ? Isn't kasan supposed to not check stuff in vmalloc area ?
>>
>> It tries to poison global variables in modules. If module is in vmalloc, than it will try to poison vmalloc.
>> Given that the vmalloc area is not so big on 32bits, the easiest solution is to cover all vmalloc with RW shadow.
>>
> 
> Euh ... Not so big ?
> 
> Memory: 96448K/131072K available (8016K kernel code, 1680K rwdata
> , 2720K rodata, 624K init, 4678K bss, 34624K reserved, 0K cma-reserved)
> Kernel virtual memory layout:
>   * 0xffefc000..0xffffc000  : fixmap
>   * 0xf7c00000..0xffc00000  : kasan shadow mem
>   * 0xf7a00000..0xf7c00000  : consistent mem
>   * 0xf7a00000..0xf7a00000  : early ioremap
>   * 0xc9000000..0xf7a00000  : vmalloc & ioremap
> 
> Here, vmalloc area size 0x2ea00000, that is 746Mbytes. Shadow for this would be 93Mbytes and we are already using 16Mbytes to shadow the linear memory area .... this poor board has 128Mbytes RAM in total.
> 
> So another solution is needed.
> 

Ok.
As a temporary workaround your can make __asan_register_globals() to skip globals in vmalloc(). 
Obviously it means that out-of-bounds accesses to in modules will be missed.

Non temporary solution would making kasan to fully support vmalloc, i.e. remove RO shadow and allocate/free shadow on vmalloc()/vfree().
But this feels like separate task, out of scope of this patch set.

It is also possible to follow some other arches - dedicate separate address range for modules, allocate/free shadow in module_alloc/free.
But it doesn't seem worthy to implement this only for the sake of kasan, since vmalloc support needs to be done anyway.

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

* Re: [PATCH v5 3/3] powerpc/32: Add KASAN support
  2019-02-12 13:36 ` [PATCH v5 3/3] powerpc/32: Add KASAN support Christophe Leroy
  2019-02-14 22:04   ` Daniel Axtens
@ 2019-02-18  9:27   ` Michael Ellerman
  2019-02-19 18:03     ` Christophe Leroy
  1 sibling, 1 reply; 11+ messages in thread
From: Michael Ellerman @ 2019-02-18  9:27 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin,
	Alexander Potapenko, Dmitry Vyukov, Daniel Axtens
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev

Christophe Leroy <christophe.leroy@c-s.fr> writes:

> diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
> index e0637730a8e7..dba2c1038363 100644
> --- a/arch/powerpc/include/asm/ppc_asm.h
> +++ b/arch/powerpc/include/asm/ppc_asm.h
> @@ -251,6 +251,10 @@ GLUE(.,name):
>  
>  #define _GLOBAL_TOC(name) _GLOBAL(name)
>  
> +#define KASAN_OVERRIDE(x, y) \
> +	.weak x;	     \
> +	.set x, y
> +

Can you add a comment describing what that does and why?

> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 879b36602748..fc4c42262694 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -16,8 +16,9 @@ CFLAGS_prom_init.o      += -fPIC
>  CFLAGS_btext.o		+= -fPIC
>  endif
>  
> -CFLAGS_cputable.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
> -CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
> +CFLAGS_early_32.o += -DDISABLE_BRANCH_PROFILING
> +CFLAGS_cputable.o += $(DISABLE_LATENT_ENTROPY_PLUGIN) -DDISABLE_BRANCH_PROFILING
> +CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN) -DDISABLE_BRANCH_PROFILING

Why do we need to disable branch profiling now?

I'd probably be happier if all the CFLAGS changes were done in a leadup
patch to make them more obvious.

> diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
> index 667df97d2595..da6bb16e0876 100644
> --- a/arch/powerpc/kernel/prom_init_check.sh
> +++ b/arch/powerpc/kernel/prom_init_check.sh
> @@ -16,8 +16,16 @@
>  # If you really need to reference something from prom_init.o add
>  # it to the list below:
>  
> +grep CONFIG_KASAN=y .config >/dev/null

Just to be safe "^CONFIG_KASAN=y$" ?

> +if [ $? -eq 0 ]
> +then
> +	MEMFCT="__memcpy __memset"
> +else
> +	MEMFCT="memcpy memset"
> +fi

MEM_FUNCS ?

> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
> index 3bf9fc6fd36c..ce8d4a9f810a 100644
> --- a/arch/powerpc/lib/Makefile
> +++ b/arch/powerpc/lib/Makefile
> @@ -8,6 +8,14 @@ ccflags-$(CONFIG_PPC64)	:= $(NO_MINIMAL_TOC)
>  CFLAGS_REMOVE_code-patching.o = $(CC_FLAGS_FTRACE)
>  CFLAGS_REMOVE_feature-fixups.o = $(CC_FLAGS_FTRACE)
>  
> +KASAN_SANITIZE_code-patching.o := n
> +KASAN_SANITIZE_feature-fixups.o := n
> +
> +ifdef CONFIG_KASAN
> +CFLAGS_code-patching.o += -DDISABLE_BRANCH_PROFILING
> +CFLAGS_feature-fixups.o += -DDISABLE_BRANCH_PROFILING
> +endif

There's that branch profiling again, though here it's only if KASAN is enabled.

> diff --git a/arch/powerpc/mm/kasan_init.c b/arch/powerpc/mm/kasan_init.c
> new file mode 100644
> index 000000000000..bd8e0a263e12
> --- /dev/null
> +++ b/arch/powerpc/mm/kasan_init.c
> @@ -0,0 +1,114 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#define DISABLE_BRANCH_PROFILING
> +
> +#include <linux/kasan.h>
> +#include <linux/printk.h>
> +#include <linux/memblock.h>
> +#include <linux/sched/task.h>
> +#include <asm/pgalloc.h>
> +
> +void __init kasan_early_init(void)
> +{
> +	unsigned long addr = KASAN_SHADOW_START;
> +	unsigned long end = KASAN_SHADOW_END;
> +	unsigned long next;
> +	pmd_t *pmd = pmd_offset(pud_offset(pgd_offset_k(addr), addr), addr);

Can none of those fail?


cheers

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

* Re: [PATCH v5 3/3] powerpc/32: Add KASAN support
  2019-02-18  9:27   ` Michael Ellerman
@ 2019-02-19 18:03     ` Christophe Leroy
  0 siblings, 0 replies; 11+ messages in thread
From: Christophe Leroy @ 2019-02-19 18:03 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin,
	Alexander Potapenko, Dmitry Vyukov, Daniel Axtens
  Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev



Le 18/02/2019 à 10:27, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
> 
>> diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
>> index e0637730a8e7..dba2c1038363 100644
>> --- a/arch/powerpc/include/asm/ppc_asm.h
>> +++ b/arch/powerpc/include/asm/ppc_asm.h
>> @@ -251,6 +251,10 @@ GLUE(.,name):
>>   
>>   #define _GLOBAL_TOC(name) _GLOBAL(name)
>>   
>> +#define KASAN_OVERRIDE(x, y) \
>> +	.weak x;	     \
>> +	.set x, y
>> +
> 
> Can you add a comment describing what that does and why?

It's gone. Hope the new approach is more clear. It's now in a dedicated 
patch.

> 
>> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
>> index 879b36602748..fc4c42262694 100644
>> --- a/arch/powerpc/kernel/Makefile
>> +++ b/arch/powerpc/kernel/Makefile
>> @@ -16,8 +16,9 @@ CFLAGS_prom_init.o      += -fPIC
>>   CFLAGS_btext.o		+= -fPIC
>>   endif
>>   
>> -CFLAGS_cputable.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
>> -CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
>> +CFLAGS_early_32.o += -DDISABLE_BRANCH_PROFILING
>> +CFLAGS_cputable.o += $(DISABLE_LATENT_ENTROPY_PLUGIN) -DDISABLE_BRANCH_PROFILING
>> +CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN) -DDISABLE_BRANCH_PROFILING
> 
> Why do we need to disable branch profiling now?

Recommended by Andrey, see https://patchwork.ozlabs.org/patch/1023887/

Maybe it should be only when KASAN is active ? For prom_init it should 
probably be all the time, for the others I don't know. Can't remember 
why I did it that way.

> 
> I'd probably be happier if all the CFLAGS changes were done in a leadup
> patch to make them more obvious.

Oops, I forgot to read your mail entirely before sending out v6. Indeed 
I only read first part. Anyway, that's probably not the last run.

> 
>> diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
>> index 667df97d2595..da6bb16e0876 100644
>> --- a/arch/powerpc/kernel/prom_init_check.sh
>> +++ b/arch/powerpc/kernel/prom_init_check.sh
>> @@ -16,8 +16,16 @@
>>   # If you really need to reference something from prom_init.o add
>>   # it to the list below:
>>   
>> +grep CONFIG_KASAN=y .config >/dev/null
> 
> Just to be safe "^CONFIG_KASAN=y$" ?

ok

> 
>> +if [ $? -eq 0 ]
>> +then
>> +	MEMFCT="__memcpy __memset"
>> +else
>> +	MEMFCT="memcpy memset"
>> +fi
> 
> MEM_FUNCS ?

Yes, I change it now before I forget.

> 
>> diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
>> index 3bf9fc6fd36c..ce8d4a9f810a 100644
>> --- a/arch/powerpc/lib/Makefile
>> +++ b/arch/powerpc/lib/Makefile
>> @@ -8,6 +8,14 @@ ccflags-$(CONFIG_PPC64)	:= $(NO_MINIMAL_TOC)
>>   CFLAGS_REMOVE_code-patching.o = $(CC_FLAGS_FTRACE)
>>   CFLAGS_REMOVE_feature-fixups.o = $(CC_FLAGS_FTRACE)
>>   
>> +KASAN_SANITIZE_code-patching.o := n
>> +KASAN_SANITIZE_feature-fixups.o := n
>> +
>> +ifdef CONFIG_KASAN
>> +CFLAGS_code-patching.o += -DDISABLE_BRANCH_PROFILING
>> +CFLAGS_feature-fixups.o += -DDISABLE_BRANCH_PROFILING
>> +endif
> 
> There's that branch profiling again, though here it's only if KASAN is enabled.
> 
>> diff --git a/arch/powerpc/mm/kasan_init.c b/arch/powerpc/mm/kasan_init.c
>> new file mode 100644
>> index 000000000000..bd8e0a263e12
>> --- /dev/null
>> +++ b/arch/powerpc/mm/kasan_init.c
>> @@ -0,0 +1,114 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +#define DISABLE_BRANCH_PROFILING
>> +
>> +#include <linux/kasan.h>
>> +#include <linux/printk.h>
>> +#include <linux/memblock.h>
>> +#include <linux/sched/task.h>
>> +#include <asm/pgalloc.h>
>> +
>> +void __init kasan_early_init(void)
>> +{
>> +	unsigned long addr = KASAN_SHADOW_START;
>> +	unsigned long end = KASAN_SHADOW_END;
>> +	unsigned long next;
>> +	pmd_t *pmd = pmd_offset(pud_offset(pgd_offset_k(addr), addr), addr);
> 
> Can none of those fail?

map_kernel_page() in pgtable_32.c does exactly the same.

pud_offset() and pmd_offset() are no-ops and only serve as type 
modifiers, so pmd will get the value returned by pgd_offset_k() which 
should always be valid unless init_mm->pgd is bad.

Christophe

> 
> 
> cheers
> 

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

end of thread, other threads:[~2019-02-19 18:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-12 13:36 [PATCH v5 0/3] KASAN for powerpc/32 Christophe Leroy
2019-02-12 13:36 ` [PATCH v5 1/3] powerpc/mm: prepare kernel for KAsan on PPC32 Christophe Leroy
2019-02-12 13:36 ` [PATCH v5 2/3] powerpc/32: Move early_init() in a separate file Christophe Leroy
2019-02-12 13:36 ` [PATCH v5 3/3] powerpc/32: Add KASAN support Christophe Leroy
2019-02-14 22:04   ` Daniel Axtens
2019-02-15  8:41     ` Christophe Leroy
2019-02-15 10:01       ` Andrey Ryabinin
2019-02-15 10:10         ` Christophe Leroy
2019-02-15 10:38           ` Andrey Ryabinin
2019-02-18  9:27   ` Michael Ellerman
2019-02-19 18:03     ` Christophe Leroy

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