This series implements KASLR for powerpc/fsl_booke/32, as a security
feature that deters exploit attempts relying on knowledge of the location
of kernel internals.
Since CONFIG_RELOCATABLE has already supported, what we need to do is
map or copy kernel to a proper place and relocate. Freescale Book-E
parts expect lowmem to be mapped by fixed TLB entries(TLB1). The TLB1
entries are not suitable to map the kernel directly in a randomized
region, so we chose to copy the kernel to a proper place and restart to
relocate.
Entropy is derived from the banner and timer base, which will change every
build and boot. This not so much safe so additionally the bootloader may
pass entropy via the /chosen/kaslr-seed node in device tree.
We will use the first 512M of the low memory to randomize the kernel
image. The memory will be split in 64M zones. We will use the lower 8
bit of the entropy to decide the index of the 64M zone. Then we chose a
16K aligned offset inside the 64M zone to put the kernel in.
KERNELBASE
|--> 64M <--|
| |
+---------------+ +----------------+---------------+
| |....| |kernel| | |
+---------------+ +----------------+---------------+
| |
|-----> offset <-----|
kimage_vaddr
We also check if we will overlap with some areas like the dtb area, the
initrd area or the crashkernel area. If we cannot find a proper area,
kaslr will be disabled and boot from the original kernel.
Changes since v4:
- Add Reviewed-by tag from Christophe
- Remove an unnecessary cast
- Remove unnecessary parenthesis
- Fix checkpatch warning
Changes since v3:
- Add Reviewed-by and Tested-by tag from Diana
- Change the comment in fsl_booke_entry_mapping.S to be consistent
with the new code.
Changes since v2:
- Remove unnecessary #ifdef
- Use SZ_64M instead of0x4000000
- Call early_init_dt_scan_chosen() to init boot_command_line
- Rename kaslr_second_init() to kaslr_late_init()
Changes since v1:
- Remove some useless 'extern' keyword.
- Replace EXPORT_SYMBOL with EXPORT_SYMBOL_GPL
- Improve some assembly code
- Use memzero_explicit instead of memset
- Use boot_command_line and remove early_command_line
- Do not print kaslr offset if kaslr is disabled
Jason Yan (10):
powerpc: unify definition of M_IF_NEEDED
powerpc: move memstart_addr and kernstart_addr to init-common.c
powerpc: introduce kimage_vaddr to store the kernel base
powerpc/fsl_booke/32: introduce create_tlb_entry() helper
powerpc/fsl_booke/32: introduce reloc_kernel_entry() helper
powerpc/fsl_booke/32: implement KASLR infrastructure
powerpc/fsl_booke/32: randomize the kernel image offset
powerpc/fsl_booke/kaslr: clear the original kernel if randomized
powerpc/fsl_booke/kaslr: support nokaslr cmdline parameter
powerpc/fsl_booke/kaslr: dump out kernel offset information on panic
arch/powerpc/Kconfig | 11 +
arch/powerpc/include/asm/nohash/mmu-book3e.h | 10 +
arch/powerpc/include/asm/page.h | 7 +
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/early_32.c | 2 +-
arch/powerpc/kernel/exceptions-64e.S | 10 -
arch/powerpc/kernel/fsl_booke_entry_mapping.S | 27 +-
arch/powerpc/kernel/head_fsl_booke.S | 55 ++-
arch/powerpc/kernel/kaslr_booke.c | 427 ++++++++++++++++++
arch/powerpc/kernel/machine_kexec.c | 1 +
arch/powerpc/kernel/misc_64.S | 5 -
arch/powerpc/kernel/setup-common.c | 19 +
arch/powerpc/mm/init-common.c | 7 +
arch/powerpc/mm/init_32.c | 5 -
arch/powerpc/mm/init_64.c | 5 -
arch/powerpc/mm/mmu_decl.h | 10 +
arch/powerpc/mm/nohash/fsl_booke.c | 8 +-
17 files changed, 560 insertions(+), 50 deletions(-)
create mode 100644 arch/powerpc/kernel/kaslr_booke.c
--
2.17.2
M_IF_NEEDED is defined too many times. Move it to a common place.
Signed-off-by: Jason Yan <[email protected]>
Cc: Diana Craciun <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Cc: Kees Cook <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
Reviewed-by: Diana Craciun <[email protected]>
Tested-by: Diana Craciun <[email protected]>
---
arch/powerpc/include/asm/nohash/mmu-book3e.h | 10 ++++++++++
arch/powerpc/kernel/exceptions-64e.S | 10 ----------
arch/powerpc/kernel/fsl_booke_entry_mapping.S | 10 ----------
arch/powerpc/kernel/misc_64.S | 5 -----
4 files changed, 10 insertions(+), 25 deletions(-)
diff --git a/arch/powerpc/include/asm/nohash/mmu-book3e.h b/arch/powerpc/include/asm/nohash/mmu-book3e.h
index 4c9777d256fb..0877362e48fa 100644
--- a/arch/powerpc/include/asm/nohash/mmu-book3e.h
+++ b/arch/powerpc/include/asm/nohash/mmu-book3e.h
@@ -221,6 +221,16 @@
#define TLBILX_T_CLASS2 6
#define TLBILX_T_CLASS3 7
+/*
+ * The mapping only needs to be cache-coherent on SMP, except on
+ * Freescale e500mc derivatives where it's also needed for coherent DMA.
+ */
+#if defined(CONFIG_SMP) || defined(CONFIG_PPC_E500MC)
+#define M_IF_NEEDED MAS2_M
+#else
+#define M_IF_NEEDED 0
+#endif
+
#ifndef __ASSEMBLY__
#include <asm/bug.h>
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 1cfb3da4a84a..fd49ec07ce4a 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -1342,16 +1342,6 @@ skpinv: addi r6,r6,1 /* Increment */
sync
isync
-/*
- * The mapping only needs to be cache-coherent on SMP, except on
- * Freescale e500mc derivatives where it's also needed for coherent DMA.
- */
-#if defined(CONFIG_SMP) || defined(CONFIG_PPC_E500MC)
-#define M_IF_NEEDED MAS2_M
-#else
-#define M_IF_NEEDED 0
-#endif
-
/* 6. Setup KERNELBASE mapping in TLB[0]
*
* r3 = MAS0 w/TLBSEL & ESEL for the entry we started in
diff --git a/arch/powerpc/kernel/fsl_booke_entry_mapping.S b/arch/powerpc/kernel/fsl_booke_entry_mapping.S
index ea065282b303..de0980945510 100644
--- a/arch/powerpc/kernel/fsl_booke_entry_mapping.S
+++ b/arch/powerpc/kernel/fsl_booke_entry_mapping.S
@@ -153,16 +153,6 @@ skpinv: addi r6,r6,1 /* Increment */
tlbivax 0,r9
TLBSYNC
-/*
- * The mapping only needs to be cache-coherent on SMP, except on
- * Freescale e500mc derivatives where it's also needed for coherent DMA.
- */
-#if defined(CONFIG_SMP) || defined(CONFIG_PPC_E500MC)
-#define M_IF_NEEDED MAS2_M
-#else
-#define M_IF_NEEDED 0
-#endif
-
#if defined(ENTRY_MAPPING_BOOT_SETUP)
/* 6. Setup KERNELBASE mapping in TLB1[0] */
diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
index b55a7b4cb543..26074f92d4bc 100644
--- a/arch/powerpc/kernel/misc_64.S
+++ b/arch/powerpc/kernel/misc_64.S
@@ -432,11 +432,6 @@ kexec_create_tlb:
rlwimi r9,r10,16,4,15 /* Setup MAS0 = TLBSEL | ESEL(r9) */
/* Set up a temp identity mapping v:0 to p:0 and return to it. */
-#if defined(CONFIG_SMP) || defined(CONFIG_PPC_E500MC)
-#define M_IF_NEEDED MAS2_M
-#else
-#define M_IF_NEEDED 0
-#endif
mtspr SPRN_MAS0,r9
lis r9,(MAS1_VALID|MAS1_IPROT)@h
--
2.17.2
These two variables are both defined in init_32.c and init_64.c. Move
them to init-common.c.
Signed-off-by: Jason Yan <[email protected]>
Cc: Diana Craciun <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Cc: Kees Cook <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
Reviewed-by: Diana Craciun <[email protected]>
Tested-by: Diana Craciun <[email protected]>
---
arch/powerpc/mm/init-common.c | 5 +++++
arch/powerpc/mm/init_32.c | 5 -----
arch/powerpc/mm/init_64.c | 5 -----
3 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
index a84da92920f7..152ae0d21435 100644
--- a/arch/powerpc/mm/init-common.c
+++ b/arch/powerpc/mm/init-common.c
@@ -21,6 +21,11 @@
#include <asm/pgtable.h>
#include <asm/kup.h>
+phys_addr_t memstart_addr = (phys_addr_t)~0ull;
+EXPORT_SYMBOL_GPL(memstart_addr);
+phys_addr_t kernstart_addr;
+EXPORT_SYMBOL_GPL(kernstart_addr);
+
static bool disable_kuep = !IS_ENABLED(CONFIG_PPC_KUEP);
static bool disable_kuap = !IS_ENABLED(CONFIG_PPC_KUAP);
diff --git a/arch/powerpc/mm/init_32.c b/arch/powerpc/mm/init_32.c
index b04896a88d79..872df48ae41b 100644
--- a/arch/powerpc/mm/init_32.c
+++ b/arch/powerpc/mm/init_32.c
@@ -56,11 +56,6 @@
phys_addr_t total_memory;
phys_addr_t total_lowmem;
-phys_addr_t memstart_addr = (phys_addr_t)~0ull;
-EXPORT_SYMBOL(memstart_addr);
-phys_addr_t kernstart_addr;
-EXPORT_SYMBOL(kernstart_addr);
-
#ifdef CONFIG_RELOCATABLE
/* Used in __va()/__pa() */
long long virt_phys_offset;
diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c
index a44f6281ca3a..c836f1269ee7 100644
--- a/arch/powerpc/mm/init_64.c
+++ b/arch/powerpc/mm/init_64.c
@@ -63,11 +63,6 @@
#include <mm/mmu_decl.h>
-phys_addr_t memstart_addr = ~0;
-EXPORT_SYMBOL_GPL(memstart_addr);
-phys_addr_t kernstart_addr;
-EXPORT_SYMBOL_GPL(kernstart_addr);
-
#ifdef CONFIG_SPARSEMEM_VMEMMAP
/*
* Given an address within the vmemmap, determine the pfn of the page that
--
2.17.2
This patch add support to boot kernel from places other than KERNELBASE.
Since CONFIG_RELOCATABLE has already supported, what we need to do is
map or copy kernel to a proper place and relocate. Freescale Book-E
parts expect lowmem to be mapped by fixed TLB entries(TLB1). The TLB1
entries are not suitable to map the kernel directly in a randomized
region, so we chose to copy the kernel to a proper place and restart to
relocate.
The offset of the kernel was not randomized yet(a fixed 64M is set). We
will randomize it in the next patch.
Signed-off-by: Jason Yan <[email protected]>
Cc: Diana Craciun <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Cc: Kees Cook <[email protected]>
Tested-by: Diana Craciun <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
---
arch/powerpc/Kconfig | 11 +++
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/early_32.c | 2 +-
arch/powerpc/kernel/fsl_booke_entry_mapping.S | 17 ++--
arch/powerpc/kernel/head_fsl_booke.S | 13 ++-
arch/powerpc/kernel/kaslr_booke.c | 84 +++++++++++++++++++
arch/powerpc/mm/mmu_decl.h | 6 ++
arch/powerpc/mm/nohash/fsl_booke.c | 7 +-
8 files changed, 126 insertions(+), 15 deletions(-)
create mode 100644 arch/powerpc/kernel/kaslr_booke.c
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 77f6ebf97113..755378887912 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -548,6 +548,17 @@ config RELOCATABLE
setting can still be useful to bootwrappers that need to know the
load address of the kernel (eg. u-boot/mkimage).
+config RANDOMIZE_BASE
+ bool "Randomize the address of the kernel image"
+ depends on (FSL_BOOKE && FLATMEM && PPC32)
+ select RELOCATABLE
+ help
+ Randomizes the virtual address at which the kernel image is
+ loaded, as a security feature that deters exploit attempts
+ relying on knowledge of the location of kernel internals.
+
+ If unsure, say N.
+
config RELOCATABLE_TEST
bool "Test relocatable kernel"
depends on (PPC64 && RELOCATABLE)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index ea0c69236789..32f6c5b99307 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -106,6 +106,7 @@ extra-$(CONFIG_PPC_8xx) := head_8xx.o
extra-y += vmlinux.lds
obj-$(CONFIG_RELOCATABLE) += reloc_$(BITS).o
+obj-$(CONFIG_RANDOMIZE_BASE) += kaslr_booke.o
obj-$(CONFIG_PPC32) += entry_32.o setup_32.o early_32.o
obj-$(CONFIG_PPC64) += dma-iommu.o iommu.o
diff --git a/arch/powerpc/kernel/early_32.c b/arch/powerpc/kernel/early_32.c
index 3482118ffe76..fe8347cdc07d 100644
--- a/arch/powerpc/kernel/early_32.c
+++ b/arch/powerpc/kernel/early_32.c
@@ -32,5 +32,5 @@ notrace unsigned long __init early_init(unsigned long dt_ptr)
apply_feature_fixups();
- return KERNELBASE + offset;
+ return kimage_vaddr + offset;
}
diff --git a/arch/powerpc/kernel/fsl_booke_entry_mapping.S b/arch/powerpc/kernel/fsl_booke_entry_mapping.S
index de0980945510..de7ee682bb4a 100644
--- a/arch/powerpc/kernel/fsl_booke_entry_mapping.S
+++ b/arch/powerpc/kernel/fsl_booke_entry_mapping.S
@@ -155,23 +155,22 @@ skpinv: addi r6,r6,1 /* Increment */
#if defined(ENTRY_MAPPING_BOOT_SETUP)
-/* 6. Setup KERNELBASE mapping in TLB1[0] */
+/* 6. Setup kimage_vaddr mapping in TLB1[0] */
lis r6,0x1000 /* Set MAS0(TLBSEL) = TLB1(1), ESEL = 0 */
mtspr SPRN_MAS0,r6
lis r6,(MAS1_VALID|MAS1_IPROT)@h
ori r6,r6,(MAS1_TSIZE(BOOK3E_PAGESZ_64M))@l
mtspr SPRN_MAS1,r6
- lis r6,MAS2_VAL(PAGE_OFFSET, BOOK3E_PAGESZ_64M, M_IF_NEEDED)@h
- ori r6,r6,MAS2_VAL(PAGE_OFFSET, BOOK3E_PAGESZ_64M, M_IF_NEEDED)@l
- mtspr SPRN_MAS2,r6
+ lis r6,MAS2_EPN_MASK(BOOK3E_PAGESZ_64M)@h
+ ori r6,r6,MAS2_EPN_MASK(BOOK3E_PAGESZ_64M)@l
+ and r6,r6,r20
+ ori r6,r6,M_IF_NEEDED@l
+ mtspr SPRN_MAS2,r6
mtspr SPRN_MAS3,r8
tlbwe
-/* 7. Jump to KERNELBASE mapping */
- lis r6,(KERNELBASE & ~0xfff)@h
- ori r6,r6,(KERNELBASE & ~0xfff)@l
- rlwinm r7,r25,0,0x03ffffff
- add r6,r7,r6
+/* 7. Jump to kimage_vaddr mapping */
+ mr r6,r20
#elif defined(ENTRY_MAPPING_KEXEC_SETUP)
/*
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 2083382dd662..aa55832e7506 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -155,6 +155,8 @@ _ENTRY(_start);
*/
_ENTRY(__early_start)
+ LOAD_REG_ADDR_PIC(r20, kimage_vaddr)
+ lwz r20,0(r20)
#define ENTRY_MAPPING_BOOT_SETUP
#include "fsl_booke_entry_mapping.S"
@@ -277,8 +279,8 @@ set_ivor:
ori r6, r6, swapper_pg_dir@l
lis r5, abatron_pteptrs@h
ori r5, r5, abatron_pteptrs@l
- lis r4, KERNELBASE@h
- ori r4, r4, KERNELBASE@l
+ lis r3, kimage_vaddr@ha
+ lwz r4, kimage_vaddr@l(r3)
stw r5, 0(r4) /* Save abatron_pteptrs at a fixed location */
stw r6, 0(r5)
@@ -1067,7 +1069,12 @@ __secondary_start:
mr r5,r25 /* phys kernel start */
rlwinm r5,r5,0,~0x3ffffff /* aligned 64M */
subf r4,r5,r4 /* memstart_addr - phys kernel start */
- li r5,0 /* no device tree */
+ lis r7,KERNELBASE@h
+ ori r7,r7,KERNELBASE@l
+ cmpw r20,r7 /* if kimage_vaddr != KERNELBASE, randomized */
+ beq 2f
+ li r4,0
+2: li r5,0 /* no device tree */
li r6,0 /* not boot cpu */
bl restore_to_as0
diff --git a/arch/powerpc/kernel/kaslr_booke.c b/arch/powerpc/kernel/kaslr_booke.c
new file mode 100644
index 000000000000..30f84c0321b2
--- /dev/null
+++ b/arch/powerpc/kernel/kaslr_booke.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2019 Jason Yan <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/signal.h>
+#include <linux/sched.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <linux/ptrace.h>
+#include <linux/mman.h>
+#include <linux/mm.h>
+#include <linux/swap.h>
+#include <linux/stddef.h>
+#include <linux/vmalloc.h>
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/highmem.h>
+#include <linux/memblock.h>
+#include <asm/pgalloc.h>
+#include <asm/prom.h>
+#include <asm/io.h>
+#include <asm/mmu_context.h>
+#include <asm/pgtable.h>
+#include <asm/mmu.h>
+#include <linux/uaccess.h>
+#include <asm/smp.h>
+#include <asm/machdep.h>
+#include <asm/setup.h>
+#include <asm/paca.h>
+#include <mm/mmu_decl.h>
+
+extern int is_second_reloc;
+
+static unsigned long __init kaslr_choose_location(void *dt_ptr, phys_addr_t size,
+ unsigned long kernel_sz)
+{
+ /* return a fixed offset of 64M for now */
+ return SZ_64M;
+}
+
+/*
+ * To see if we need to relocate the kernel to a random offset
+ * void *dt_ptr - address of the device tree
+ * phys_addr_t size - size of the first memory block
+ */
+notrace void __init kaslr_early_init(void *dt_ptr, phys_addr_t size)
+{
+ unsigned long tlb_virt;
+ phys_addr_t tlb_phys;
+ unsigned long offset;
+ unsigned long kernel_sz;
+
+ kernel_sz = (unsigned long)_end - KERNELBASE;
+
+ offset = kaslr_choose_location(dt_ptr, size, kernel_sz);
+
+ if (offset == 0)
+ return;
+
+ kimage_vaddr += offset;
+ kernstart_addr += offset;
+
+ is_second_reloc = 1;
+
+ if (offset >= SZ_64M) {
+ tlb_virt = round_down(kimage_vaddr, SZ_64M);
+ tlb_phys = round_down(kernstart_addr, SZ_64M);
+
+ /* Create kernel map to relocate in */
+ create_tlb_entry(tlb_phys, tlb_virt, 1);
+ }
+
+ /* Copy the kernel to it's new location and run */
+ memcpy((void *)kimage_vaddr, (void *)KERNELBASE, kernel_sz);
+
+ reloc_kernel_entry(dt_ptr, kimage_vaddr);
+}
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index 804da298beb3..9332772c8a66 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -148,6 +148,12 @@ void reloc_kernel_entry(void *fdt, int addr);
extern void loadcam_entry(unsigned int index);
extern void loadcam_multi(int first_idx, int num, int tmp_idx);
+#ifdef CONFIG_RANDOMIZE_BASE
+void kaslr_early_init(void *dt_ptr, phys_addr_t size);
+#else
+static inline void kaslr_early_init(void *dt_ptr, phys_addr_t size) {}
+#endif
+
struct tlbcam {
u32 MAS0;
u32 MAS1;
diff --git a/arch/powerpc/mm/nohash/fsl_booke.c b/arch/powerpc/mm/nohash/fsl_booke.c
index 556e3cd52a35..8d25a8dc965f 100644
--- a/arch/powerpc/mm/nohash/fsl_booke.c
+++ b/arch/powerpc/mm/nohash/fsl_booke.c
@@ -263,7 +263,8 @@ void setup_initial_memory_limit(phys_addr_t first_memblock_base,
int __initdata is_second_reloc;
notrace void __init relocate_init(u64 dt_ptr, phys_addr_t start)
{
- unsigned long base = KERNELBASE;
+ unsigned long base = kimage_vaddr;
+ phys_addr_t size;
kernstart_addr = start;
if (is_second_reloc) {
@@ -291,7 +292,7 @@ notrace void __init relocate_init(u64 dt_ptr, phys_addr_t start)
start &= ~0x3ffffff;
base &= ~0x3ffffff;
virt_phys_offset = base - start;
- early_get_first_memblock_info(__va(dt_ptr), NULL);
+ early_get_first_memblock_info(__va(dt_ptr), &size);
/*
* We now get the memstart_addr, then we should check if this
* address is the same as what the PAGE_OFFSET map to now. If
@@ -316,6 +317,8 @@ notrace void __init relocate_init(u64 dt_ptr, phys_addr_t start)
/* We should never reach here */
panic("Relocation error");
}
+
+ kaslr_early_init(__va(dt_ptr), size);
}
#endif
#endif
--
2.17.2
Now the kernel base is a fixed value - KERNELBASE. To support KASLR, we
need a variable to store the kernel base.
Signed-off-by: Jason Yan <[email protected]>
Cc: Diana Craciun <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Cc: Kees Cook <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
Reviewed-by: Diana Craciun <[email protected]>
Tested-by: Diana Craciun <[email protected]>
---
arch/powerpc/include/asm/page.h | 2 ++
arch/powerpc/mm/init-common.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 0d52f57fca04..60a68d3a54b1 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -315,6 +315,8 @@ void arch_free_page(struct page *page, int order);
struct vm_area_struct;
+extern unsigned long kimage_vaddr;
+
#include <asm-generic/memory_model.h>
#endif /* __ASSEMBLY__ */
#include <asm/slice.h>
diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
index 152ae0d21435..d4801ce48dc5 100644
--- a/arch/powerpc/mm/init-common.c
+++ b/arch/powerpc/mm/init-common.c
@@ -25,6 +25,8 @@ phys_addr_t memstart_addr = (phys_addr_t)~0ull;
EXPORT_SYMBOL_GPL(memstart_addr);
phys_addr_t kernstart_addr;
EXPORT_SYMBOL_GPL(kernstart_addr);
+unsigned long kimage_vaddr = KERNELBASE;
+EXPORT_SYMBOL_GPL(kimage_vaddr);
static bool disable_kuep = !IS_ENABLED(CONFIG_PPC_KUEP);
static bool disable_kuap = !IS_ENABLED(CONFIG_PPC_KUAP);
--
2.17.2
The original kernel still exists in the memory, clear it now.
Signed-off-by: Jason Yan <[email protected]>
Cc: Diana Craciun <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Cc: Kees Cook <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
Reviewed-by: Diana Craciun <[email protected]>
Tested-by: Diana Craciun <[email protected]>
---
arch/powerpc/kernel/kaslr_booke.c | 11 +++++++++++
arch/powerpc/mm/mmu_decl.h | 2 ++
arch/powerpc/mm/nohash/fsl_booke.c | 1 +
3 files changed, 14 insertions(+)
diff --git a/arch/powerpc/kernel/kaslr_booke.c b/arch/powerpc/kernel/kaslr_booke.c
index 52b59b05f906..c6b326424b54 100644
--- a/arch/powerpc/kernel/kaslr_booke.c
+++ b/arch/powerpc/kernel/kaslr_booke.c
@@ -400,3 +400,14 @@ notrace void __init kaslr_early_init(void *dt_ptr, phys_addr_t size)
reloc_kernel_entry(dt_ptr, kimage_vaddr);
}
+
+void __init kaslr_late_init(void)
+{
+ /* If randomized, clear the original kernel */
+ if (kimage_vaddr != KERNELBASE) {
+ unsigned long kernel_sz;
+
+ kernel_sz = (unsigned long)_end - kimage_vaddr;
+ memzero_explicit((void *)KERNELBASE, kernel_sz);
+ }
+}
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index 9332772c8a66..f0a461482dba 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -150,8 +150,10 @@ extern void loadcam_multi(int first_idx, int num, int tmp_idx);
#ifdef CONFIG_RANDOMIZE_BASE
void kaslr_early_init(void *dt_ptr, phys_addr_t size);
+void kaslr_late_init(void);
#else
static inline void kaslr_early_init(void *dt_ptr, phys_addr_t size) {}
+static inline void kaslr_late_init(void) {}
#endif
struct tlbcam {
diff --git a/arch/powerpc/mm/nohash/fsl_booke.c b/arch/powerpc/mm/nohash/fsl_booke.c
index 8d25a8dc965f..e88fcc367600 100644
--- a/arch/powerpc/mm/nohash/fsl_booke.c
+++ b/arch/powerpc/mm/nohash/fsl_booke.c
@@ -269,6 +269,7 @@ notrace void __init relocate_init(u64 dt_ptr, phys_addr_t start)
kernstart_addr = start;
if (is_second_reloc) {
virt_phys_offset = PAGE_OFFSET - memstart_addr;
+ kaslr_late_init();
return;
}
--
2.17.2
One may want to disable kaslr when boot, so provide a cmdline parameter
'nokaslr' to support this.
Signed-off-by: Jason Yan <[email protected]>
Cc: Diana Craciun <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Cc: Kees Cook <[email protected]>
Reviewed-by: Diana Craciun <[email protected]>
Tested-by: Diana Craciun <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/kaslr_booke.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/powerpc/kernel/kaslr_booke.c b/arch/powerpc/kernel/kaslr_booke.c
index c6b326424b54..436f9a03f385 100644
--- a/arch/powerpc/kernel/kaslr_booke.c
+++ b/arch/powerpc/kernel/kaslr_booke.c
@@ -361,6 +361,18 @@ static unsigned long __init kaslr_choose_location(void *dt_ptr, phys_addr_t size
return kaslr_offset;
}
+static inline __init bool kaslr_disabled(void)
+{
+ char *str;
+
+ str = strstr(boot_command_line, "nokaslr");
+ if (str == boot_command_line ||
+ (str > boot_command_line && *(str - 1) == ' '))
+ return true;
+
+ return false;
+}
+
/*
* To see if we need to relocate the kernel to a random offset
* void *dt_ptr - address of the device tree
@@ -376,6 +388,8 @@ notrace void __init kaslr_early_init(void *dt_ptr, phys_addr_t size)
kernel_sz = (unsigned long)_end - KERNELBASE;
kaslr_get_cmdline(dt_ptr);
+ if (kaslr_disabled())
+ return;
offset = kaslr_choose_location(dt_ptr, size, kernel_sz);
--
2.17.2
Add a new helper reloc_kernel_entry() to jump back to the start of the
new kernel. After we put the new kernel in a randomized place we can use
this new helper to enter the kernel and begin to relocate again.
Signed-off-by: Jason Yan <[email protected]>
Cc: Diana Craciun <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Cc: Kees Cook <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
Reviewed-by: Diana Craciun <[email protected]>
Tested-by: Diana Craciun <[email protected]>
---
arch/powerpc/kernel/head_fsl_booke.S | 13 +++++++++++++
arch/powerpc/mm/mmu_decl.h | 1 +
2 files changed, 14 insertions(+)
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index 04d124fee17d..2083382dd662 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -1143,6 +1143,19 @@ _GLOBAL(create_tlb_entry)
sync
blr
+/*
+ * Return to the start of the relocated kernel and run again
+ * r3 - virtual address of fdt
+ * r4 - entry of the kernel
+ */
+_GLOBAL(reloc_kernel_entry)
+ mfmsr r7
+ rlwinm r7, r7, 0, ~(MSR_IS | MSR_DS)
+
+ mtspr SPRN_SRR0,r4
+ mtspr SPRN_SRR1,r7
+ rfi
+
/*
* Create a tlb entry with the same effective and physical address as
* the tlb entry used by the current running code. But set the TS to 1.
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index a09f89d3aa0f..804da298beb3 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -143,6 +143,7 @@ extern void adjust_total_lowmem(void);
extern int switch_to_as1(void);
extern void restore_to_as0(int esel, int offset, void *dt_ptr, int bootcpu);
void create_tlb_entry(phys_addr_t phys, unsigned long virt, int entry);
+void reloc_kernel_entry(void *fdt, int addr);
#endif
extern void loadcam_entry(unsigned int index);
extern void loadcam_multi(int first_idx, int num, int tmp_idx);
--
2.17.2
When kaslr is enabled, the kernel offset is different for every boot.
This brings some difficult to debug the kernel. Dump out the kernel
offset when panic so that we can easily debug the kernel.
Signed-off-by: Jason Yan <[email protected]>
Cc: Diana Craciun <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Cc: Kees Cook <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
Reviewed-by: Diana Craciun <[email protected]>
Tested-by: Diana Craciun <[email protected]>
---
arch/powerpc/include/asm/page.h | 5 +++++
arch/powerpc/kernel/machine_kexec.c | 1 +
arch/powerpc/kernel/setup-common.c | 19 +++++++++++++++++++
3 files changed, 25 insertions(+)
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 60a68d3a54b1..cd3ac530e58d 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -317,6 +317,11 @@ struct vm_area_struct;
extern unsigned long kimage_vaddr;
+static inline unsigned long kaslr_offset(void)
+{
+ return kimage_vaddr - KERNELBASE;
+}
+
#include <asm-generic/memory_model.h>
#endif /* __ASSEMBLY__ */
#include <asm/slice.h>
diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
index c4ed328a7b96..078fe3d76feb 100644
--- a/arch/powerpc/kernel/machine_kexec.c
+++ b/arch/powerpc/kernel/machine_kexec.c
@@ -86,6 +86,7 @@ void arch_crash_save_vmcoreinfo(void)
VMCOREINFO_STRUCT_SIZE(mmu_psize_def);
VMCOREINFO_OFFSET(mmu_psize_def, shift);
#endif
+ vmcoreinfo_append_str("KERNELOFFSET=%lx\n", kaslr_offset());
}
/*
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 1f8db666468d..064075f02837 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -715,12 +715,31 @@ static struct notifier_block ppc_panic_block = {
.priority = INT_MIN /* may not return; must be done last */
};
+/*
+ * Dump out kernel offset information on panic.
+ */
+static int dump_kernel_offset(struct notifier_block *self, unsigned long v,
+ void *p)
+{
+ pr_emerg("Kernel Offset: 0x%lx from 0x%lx\n",
+ kaslr_offset(), KERNELBASE);
+
+ return 0;
+}
+
+static struct notifier_block kernel_offset_notifier = {
+ .notifier_call = dump_kernel_offset
+};
+
void __init setup_panic(void)
{
/* PPC64 always does a hard irq disable in its panic handler */
if (!IS_ENABLED(CONFIG_PPC64) && !ppc_md.panic)
return;
atomic_notifier_chain_register(&panic_notifier_list, &ppc_panic_block);
+ if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && kaslr_offset() > 0)
+ atomic_notifier_chain_register(&panic_notifier_list,
+ &kernel_offset_notifier);
}
#ifdef CONFIG_CHECK_CACHE_COHERENCY
--
2.17.2
Add a new helper create_tlb_entry() to create a tlb entry by the virtual
and physical address. This is a preparation to support boot kernel at a
randomized address.
Signed-off-by: Jason Yan <[email protected]>
Cc: Diana Craciun <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Cc: Kees Cook <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
Reviewed-by: Diana Craciun <[email protected]>
Tested-by: Diana Craciun <[email protected]>
---
arch/powerpc/kernel/head_fsl_booke.S | 29 ++++++++++++++++++++++++++++
arch/powerpc/mm/mmu_decl.h | 1 +
2 files changed, 30 insertions(+)
diff --git a/arch/powerpc/kernel/head_fsl_booke.S b/arch/powerpc/kernel/head_fsl_booke.S
index adf0505dbe02..04d124fee17d 100644
--- a/arch/powerpc/kernel/head_fsl_booke.S
+++ b/arch/powerpc/kernel/head_fsl_booke.S
@@ -1114,6 +1114,35 @@ __secondary_hold_acknowledge:
.long -1
#endif
+/*
+ * Create a 64M tlb by address and entry
+ * r3/r4 - physical address
+ * r5 - virtual address
+ * r6 - entry
+ */
+_GLOBAL(create_tlb_entry)
+ lis r7,0x1000 /* Set MAS0(TLBSEL) = 1 */
+ rlwimi r7,r6,16,4,15 /* Setup MAS0 = TLBSEL | ESEL(r6) */
+ mtspr SPRN_MAS0,r7 /* Write MAS0 */
+
+ lis r6,(MAS1_VALID|MAS1_IPROT)@h
+ ori r6,r6,(MAS1_TSIZE(BOOK3E_PAGESZ_64M))@l
+ mtspr SPRN_MAS1,r6 /* Write MAS1 */
+
+ lis r6,MAS2_EPN_MASK(BOOK3E_PAGESZ_64M)@h
+ ori r6,r6,MAS2_EPN_MASK(BOOK3E_PAGESZ_64M)@l
+ and r6,r6,r5
+ ori r6,r6,MAS2_M@l
+ mtspr SPRN_MAS2,r6 /* Write MAS2(EPN) */
+
+ ori r8,r4,(MAS3_SW|MAS3_SR|MAS3_SX)
+ mtspr SPRN_MAS3,r8 /* Write MAS3(RPN) */
+
+ tlbwe /* Write TLB */
+ isync
+ sync
+ blr
+
/*
* Create a tlb entry with the same effective and physical address as
* the tlb entry used by the current running code. But set the TS to 1.
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index 32c1a191c28a..a09f89d3aa0f 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -142,6 +142,7 @@ extern unsigned long calc_cam_sz(unsigned long ram, unsigned long virt,
extern void adjust_total_lowmem(void);
extern int switch_to_as1(void);
extern void restore_to_as0(int esel, int offset, void *dt_ptr, int bootcpu);
+void create_tlb_entry(phys_addr_t phys, unsigned long virt, int entry);
#endif
extern void loadcam_entry(unsigned int index);
extern void loadcam_multi(int first_idx, int num, int tmp_idx);
--
2.17.2
After we have the basic support of relocate the kernel in some
appropriate place, we can start to randomize the offset now.
Entropy is derived from the banner and timer, which will change every
build and boot. This not so much safe so additionally the bootloader may
pass entropy via the /chosen/kaslr-seed node in device tree.
We will use the first 512M of the low memory to randomize the kernel
image. The memory will be split in 64M zones. We will use the lower 8
bit of the entropy to decide the index of the 64M zone. Then we chose a
16K aligned offset inside the 64M zone to put the kernel in.
KERNELBASE
|--> 64M <--|
| |
+---------------+ +----------------+---------------+
| |....| |kernel| | |
+---------------+ +----------------+---------------+
| |
|-----> offset <-----|
kimage_vaddr
We also check if we will overlap with some areas like the dtb area, the
initrd area or the crashkernel area. If we cannot find a proper area,
kaslr will be disabled and boot from the original kernel.
Signed-off-by: Jason Yan <[email protected]>
Cc: Diana Craciun <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Cc: Kees Cook <[email protected]>
Reviewed-by: Diana Craciun <[email protected]>
Tested-by: Diana Craciun <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/kaslr_booke.c | 322 +++++++++++++++++++++++++++++-
1 file changed, 320 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/kaslr_booke.c b/arch/powerpc/kernel/kaslr_booke.c
index 30f84c0321b2..52b59b05f906 100644
--- a/arch/powerpc/kernel/kaslr_booke.c
+++ b/arch/powerpc/kernel/kaslr_booke.c
@@ -23,6 +23,8 @@
#include <linux/delay.h>
#include <linux/highmem.h>
#include <linux/memblock.h>
+#include <linux/libfdt.h>
+#include <linux/crash_core.h>
#include <asm/pgalloc.h>
#include <asm/prom.h>
#include <asm/io.h>
@@ -34,15 +36,329 @@
#include <asm/machdep.h>
#include <asm/setup.h>
#include <asm/paca.h>
+#include <asm/kdump.h>
#include <mm/mmu_decl.h>
+#include <generated/compile.h>
+#include <generated/utsrelease.h>
+
+#ifdef DEBUG
+#define DBG(fmt...) pr_info(fmt)
+#else
+#define DBG(fmt...)
+#endif
+
+struct regions {
+ unsigned long pa_start;
+ unsigned long pa_end;
+ unsigned long kernel_size;
+ unsigned long dtb_start;
+ unsigned long dtb_end;
+ unsigned long initrd_start;
+ unsigned long initrd_end;
+ unsigned long crash_start;
+ unsigned long crash_end;
+ int reserved_mem;
+ int reserved_mem_addr_cells;
+ int reserved_mem_size_cells;
+};
extern int is_second_reloc;
+/* Simplified build-specific string for starting entropy. */
+static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
+ LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
+
+static __init void kaslr_get_cmdline(void *fdt)
+{
+ int node = fdt_path_offset(fdt, "/chosen");
+
+ early_init_dt_scan_chosen(node, "chosen", 1, boot_command_line);
+}
+
+static unsigned long __init rotate_xor(unsigned long hash, const void *area,
+ size_t size)
+{
+ size_t i;
+ const unsigned long *ptr = area;
+
+ for (i = 0; i < size / sizeof(hash); i++) {
+ /* Rotate by odd number of bits and XOR. */
+ hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
+ hash ^= ptr[i];
+ }
+
+ return hash;
+}
+
+/* Attempt to create a simple but unpredictable starting entropy. */
+static unsigned long __init get_boot_seed(void *fdt)
+{
+ unsigned long hash = 0;
+
+ hash = rotate_xor(hash, build_str, sizeof(build_str));
+ hash = rotate_xor(hash, fdt, fdt_totalsize(fdt));
+
+ return hash;
+}
+
+static __init u64 get_kaslr_seed(void *fdt)
+{
+ int node, len;
+ fdt64_t *prop;
+ u64 ret;
+
+ node = fdt_path_offset(fdt, "/chosen");
+ if (node < 0)
+ return 0;
+
+ prop = fdt_getprop_w(fdt, node, "kaslr-seed", &len);
+ if (!prop || len != sizeof(u64))
+ return 0;
+
+ ret = fdt64_to_cpu(*prop);
+ *prop = 0;
+ return ret;
+}
+
+static __init bool regions_overlap(u32 s1, u32 e1, u32 s2, u32 e2)
+{
+ return e1 >= s2 && e2 >= s1;
+}
+
+static __init bool overlaps_reserved_region(const void *fdt, u32 start,
+ u32 end, struct regions *regions)
+{
+ int subnode, len, i;
+ u64 base, size;
+
+ /* check for overlap with /memreserve/ entries */
+ for (i = 0; i < fdt_num_mem_rsv(fdt); i++) {
+ if (fdt_get_mem_rsv(fdt, i, &base, &size) < 0)
+ continue;
+ if (regions_overlap(start, end, base, base + size))
+ return true;
+ }
+
+ if (regions->reserved_mem < 0)
+ return false;
+
+ /* check for overlap with static reservations in /reserved-memory */
+ for (subnode = fdt_first_subnode(fdt, regions->reserved_mem);
+ subnode >= 0;
+ subnode = fdt_next_subnode(fdt, subnode)) {
+ const fdt32_t *reg;
+ u64 rsv_end;
+
+ len = 0;
+ reg = fdt_getprop(fdt, subnode, "reg", &len);
+ while (len >= (regions->reserved_mem_addr_cells +
+ regions->reserved_mem_size_cells)) {
+ base = fdt32_to_cpu(reg[0]);
+ if (regions->reserved_mem_addr_cells == 2)
+ base = (base << 32) | fdt32_to_cpu(reg[1]);
+
+ reg += regions->reserved_mem_addr_cells;
+ len -= 4 * regions->reserved_mem_addr_cells;
+
+ size = fdt32_to_cpu(reg[0]);
+ if (regions->reserved_mem_size_cells == 2)
+ size = (size << 32) | fdt32_to_cpu(reg[1]);
+
+ reg += regions->reserved_mem_size_cells;
+ len -= 4 * regions->reserved_mem_size_cells;
+
+ if (base >= regions->pa_end)
+ continue;
+
+ rsv_end = min(base + size, (u64)U32_MAX);
+
+ if (regions_overlap(start, end, base, rsv_end))
+ return true;
+ }
+ }
+ return false;
+}
+
+static __init bool overlaps_region(const void *fdt, u32 start,
+ u32 end, struct regions *regions)
+{
+ if (regions_overlap(start, end, regions->dtb_start,
+ regions->dtb_end))
+ return true;
+
+ if (regions_overlap(start, end, regions->initrd_start,
+ regions->initrd_end))
+ return true;
+
+ if (regions_overlap(start, end, regions->crash_start,
+ regions->crash_end))
+ return true;
+
+ return overlaps_reserved_region(fdt, start, end, regions);
+}
+
+static void __init get_crash_kernel(void *fdt, unsigned long size,
+ struct regions *regions)
+{
+#ifdef CONFIG_CRASH_CORE
+ unsigned long long crash_size, crash_base;
+ int ret;
+
+ ret = parse_crashkernel(boot_command_line, size, &crash_size,
+ &crash_base);
+ if (ret != 0 || crash_size == 0)
+ return;
+ if (crash_base == 0)
+ crash_base = KDUMP_KERNELBASE;
+
+ regions->crash_start = (unsigned long)crash_base;
+ regions->crash_end = (unsigned long)(crash_base + crash_size);
+
+ DBG("crash_base=0x%llx crash_size=0x%llx\n", crash_base, crash_size);
+#endif
+}
+
+static void __init get_initrd_range(void *fdt, struct regions *regions)
+{
+ u64 start, end;
+ int node, len;
+ const __be32 *prop;
+
+ node = fdt_path_offset(fdt, "/chosen");
+ if (node < 0)
+ return;
+
+ prop = fdt_getprop(fdt, node, "linux,initrd-start", &len);
+ if (!prop)
+ return;
+ start = of_read_number(prop, len / 4);
+
+ prop = fdt_getprop(fdt, node, "linux,initrd-end", &len);
+ if (!prop)
+ return;
+ end = of_read_number(prop, len / 4);
+
+ regions->initrd_start = (unsigned long)start;
+ regions->initrd_end = (unsigned long)end;
+
+ DBG("initrd_start=0x%llx initrd_end=0x%llx\n", start, end);
+}
+
+static __init unsigned long get_usable_offset(const void *fdt, struct regions *regions,
+ unsigned long start)
+{
+ unsigned long pa;
+ unsigned long pa_end;
+
+ for (pa = start; pa > regions->pa_start; pa -= SZ_16K) {
+ pa_end = pa + regions->kernel_size;
+ if (overlaps_region(fdt, pa, pa_end, regions))
+ continue;
+
+ return pa;
+ }
+ return 0;
+}
+
+static __init void get_cell_sizes(const void *fdt, int node, int *addr_cells,
+ int *size_cells)
+{
+ const int *prop;
+ int len;
+
+ /*
+ * Retrieve the #address-cells and #size-cells properties
+ * from the 'node', or use the default if not provided.
+ */
+ *addr_cells = *size_cells = 1;
+
+ prop = fdt_getprop(fdt, node, "#address-cells", &len);
+ if (len == 4)
+ *addr_cells = fdt32_to_cpu(*prop);
+ prop = fdt_getprop(fdt, node, "#size-cells", &len);
+ if (len == 4)
+ *size_cells = fdt32_to_cpu(*prop);
+}
+
static unsigned long __init kaslr_choose_location(void *dt_ptr, phys_addr_t size,
unsigned long kernel_sz)
{
- /* return a fixed offset of 64M for now */
- return SZ_64M;
+ unsigned long offset, random;
+ unsigned long ram, linear_sz;
+ unsigned long kaslr_offset;
+ u64 seed;
+ struct regions regions;
+ unsigned long index;
+
+ random = get_boot_seed(dt_ptr);
+
+ seed = get_tb() << 32;
+ seed ^= get_tb();
+ random = rotate_xor(random, &seed, sizeof(seed));
+
+ /*
+ * Retrieve (and wipe) the seed from the FDT
+ */
+ seed = get_kaslr_seed(dt_ptr);
+ if (seed)
+ random = rotate_xor(random, &seed, sizeof(seed));
+
+ ram = min_t(phys_addr_t, __max_low_memory, size);
+ ram = map_mem_in_cams(ram, CONFIG_LOWMEM_CAM_NUM, true);
+ linear_sz = min_t(unsigned long, ram, SZ_512M);
+
+ /* If the linear size is smaller than 64M, do not randmize */
+ if (linear_sz < SZ_64M)
+ return 0;
+
+ memset(®ions, 0, sizeof(regions));
+
+ /* check for a reserved-memory node and record its cell sizes */
+ regions.reserved_mem = fdt_path_offset(dt_ptr, "/reserved-memory");
+ if (regions.reserved_mem >= 0)
+ get_cell_sizes(dt_ptr, regions.reserved_mem,
+ ®ions.reserved_mem_addr_cells,
+ ®ions.reserved_mem_size_cells);
+
+ regions.pa_start = 0;
+ regions.pa_end = linear_sz;
+ regions.dtb_start = __pa(dt_ptr);
+ regions.dtb_end = __pa(dt_ptr) + fdt_totalsize(dt_ptr);
+ regions.kernel_size = kernel_sz;
+
+ get_initrd_range(dt_ptr, ®ions);
+ get_crash_kernel(dt_ptr, ram, ®ions);
+
+ /*
+ * Decide which 64M we want to start
+ * Only use the low 8 bits of the random seed
+ */
+ index = random & 0xFF;
+ index %= linear_sz / SZ_64M;
+
+ /* Decide offset inside 64M */
+ if (index == 0) {
+ offset = random % (SZ_64M - round_up(kernel_sz, SZ_16K) * 2);
+ offset += round_up(kernel_sz, SZ_16K);
+ offset = round_up(offset, SZ_16K);
+ } else {
+ offset = random % (SZ_64M - kernel_sz);
+ offset = round_down(offset, SZ_16K);
+ }
+
+ while (index >= 0) {
+ offset = offset + index * SZ_64M;
+ kaslr_offset = get_usable_offset(dt_ptr, ®ions, offset);
+ if (kaslr_offset)
+ break;
+ index--;
+ }
+
+ /* Did not find any usable region? Give up randomize */
+ if (index < 0)
+ kaslr_offset = 0;
+
+ return kaslr_offset;
}
/*
@@ -59,6 +375,8 @@ notrace void __init kaslr_early_init(void *dt_ptr, phys_addr_t size)
kernel_sz = (unsigned long)_end - KERNELBASE;
+ kaslr_get_cmdline(dt_ptr);
+
offset = kaslr_choose_location(dt_ptr, size, kernel_sz);
if (offset == 0)
--
2.17.2
Jason Yan <[email protected]> writes:
> Now the kernel base is a fixed value - KERNELBASE. To support KASLR, we
> need a variable to store the kernel base.
>
> Signed-off-by: Jason Yan <[email protected]>
> Cc: Diana Craciun <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: Christophe Leroy <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Nicholas Piggin <[email protected]>
> Cc: Kees Cook <[email protected]>
> Reviewed-by: Christophe Leroy <[email protected]>
> Reviewed-by: Diana Craciun <[email protected]>
> Tested-by: Diana Craciun <[email protected]>
> ---
> arch/powerpc/include/asm/page.h | 2 ++
> arch/powerpc/mm/init-common.c | 2 ++
> 2 files changed, 4 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index 0d52f57fca04..60a68d3a54b1 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -315,6 +315,8 @@ void arch_free_page(struct page *page, int order);
>
> struct vm_area_struct;
>
> +extern unsigned long kimage_vaddr;
> +
> #include <asm-generic/memory_model.h>
> #endif /* __ASSEMBLY__ */
> #include <asm/slice.h>
> diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
> index 152ae0d21435..d4801ce48dc5 100644
> --- a/arch/powerpc/mm/init-common.c
> +++ b/arch/powerpc/mm/init-common.c
> @@ -25,6 +25,8 @@ phys_addr_t memstart_addr = (phys_addr_t)~0ull;
> EXPORT_SYMBOL_GPL(memstart_addr);
> phys_addr_t kernstart_addr;
> EXPORT_SYMBOL_GPL(kernstart_addr);
> +unsigned long kimage_vaddr = KERNELBASE;
> +EXPORT_SYMBOL_GPL(kimage_vaddr);
The names of the #defines and variables we use for these values are not
very consistent already, but using kimage_vaddr makes it worse I think.
Isn't this going to have the same value as kernstart_addr, but the
virtual rather than physical address?
If so kernstart_virt_addr would seem better.
cheers
Jason Yan <[email protected]> writes:
> When kaslr is enabled, the kernel offset is different for every boot.
> This brings some difficult to debug the kernel. Dump out the kernel
> offset when panic so that we can easily debug the kernel.
Some of this is taken from the arm64 version right? Please say so when
you copy other people's code.
> diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
> index c4ed328a7b96..078fe3d76feb 100644
> --- a/arch/powerpc/kernel/machine_kexec.c
> +++ b/arch/powerpc/kernel/machine_kexec.c
> @@ -86,6 +86,7 @@ void arch_crash_save_vmcoreinfo(void)
> VMCOREINFO_STRUCT_SIZE(mmu_psize_def);
> VMCOREINFO_OFFSET(mmu_psize_def, shift);
> #endif
> + vmcoreinfo_append_str("KERNELOFFSET=%lx\n", kaslr_offset());
> }
There's no mention of that in the commit log.
Please split it into a separate patch and describe what you're doing and
why.
> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
> index 1f8db666468d..064075f02837 100644
> --- a/arch/powerpc/kernel/setup-common.c
> +++ b/arch/powerpc/kernel/setup-common.c
> @@ -715,12 +715,31 @@ static struct notifier_block ppc_panic_block = {
> .priority = INT_MIN /* may not return; must be done last */
> };
>
> +/*
> + * Dump out kernel offset information on panic.
> + */
> +static int dump_kernel_offset(struct notifier_block *self, unsigned long v,
> + void *p)
> +{
> + pr_emerg("Kernel Offset: 0x%lx from 0x%lx\n",
> + kaslr_offset(), KERNELBASE);
> +
> + return 0;
> +}
> +
> +static struct notifier_block kernel_offset_notifier = {
> + .notifier_call = dump_kernel_offset
> +};
> +
> void __init setup_panic(void)
> {
> /* PPC64 always does a hard irq disable in its panic handler */
> if (!IS_ENABLED(CONFIG_PPC64) && !ppc_md.panic)
> return;
> atomic_notifier_chain_register(&panic_notifier_list, &ppc_panic_block);
> + if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && kaslr_offset() > 0)
> + atomic_notifier_chain_register(&panic_notifier_list,
> + &kernel_offset_notifier);
Don't you want to do that before the return above?
> }
cheers
Jason Yan <[email protected]> writes:
> diff --git a/arch/powerpc/kernel/kaslr_booke.c b/arch/powerpc/kernel/kaslr_booke.c
> index c6b326424b54..436f9a03f385 100644
> --- a/arch/powerpc/kernel/kaslr_booke.c
> +++ b/arch/powerpc/kernel/kaslr_booke.c
> @@ -361,6 +361,18 @@ static unsigned long __init kaslr_choose_location(void *dt_ptr, phys_addr_t size
> return kaslr_offset;
> }
>
> +static inline __init bool kaslr_disabled(void)
> +{
> + char *str;
> +
> + str = strstr(boot_command_line, "nokaslr");
> + if (str == boot_command_line ||
> + (str > boot_command_line && *(str - 1) == ' '))
> + return true;
This extra logic doesn't work for "nokaslrfoo". Is it worth it?
cheers
Jason Yan <[email protected]> writes:
> This patch add support to boot kernel from places other than KERNELBASE.
> Since CONFIG_RELOCATABLE has already supported, what we need to do is
> map or copy kernel to a proper place and relocate. Freescale Book-E
> parts expect lowmem to be mapped by fixed TLB entries(TLB1). The TLB1
> entries are not suitable to map the kernel directly in a randomized
> region, so we chose to copy the kernel to a proper place and restart to
> relocate.
So to be 100% clear you are randomising the location of the kernel in
virtual and physical space, by the same amount, and retaining the 1:1
linear mapping.
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 77f6ebf97113..755378887912 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -548,6 +548,17 @@ config RELOCATABLE
> setting can still be useful to bootwrappers that need to know the
> load address of the kernel (eg. u-boot/mkimage).
>
> +config RANDOMIZE_BASE
> + bool "Randomize the address of the kernel image"
> + depends on (FSL_BOOKE && FLATMEM && PPC32)
> + select RELOCATABLE
I think this should depend on RELOCATABLE, rather than selecting it.
> diff --git a/arch/powerpc/kernel/kaslr_booke.c b/arch/powerpc/kernel/kaslr_booke.c
> new file mode 100644
> index 000000000000..30f84c0321b2
> --- /dev/null
> +++ b/arch/powerpc/kernel/kaslr_booke.c
> @@ -0,0 +1,84 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2019 Jason Yan <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
You don't need that paragraph now that you have the SPDX tag.
Rather than using a '//' comment followed by a single line block comment
you can format it as:
// SPDX-License-Identifier: GPL-2.0-only
//
// Copyright (C) 2019 Jason Yan <[email protected]>
> +#include <linux/signal.h>
> +#include <linux/sched.h>
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +#include <linux/ptrace.h>
> +#include <linux/mman.h>
> +#include <linux/mm.h>
> +#include <linux/swap.h>
> +#include <linux/stddef.h>
> +#include <linux/vmalloc.h>
> +#include <linux/init.h>
> +#include <linux/delay.h>
> +#include <linux/highmem.h>
> +#include <linux/memblock.h>
> +#include <asm/pgalloc.h>
> +#include <asm/prom.h>
> +#include <asm/io.h>
> +#include <asm/mmu_context.h>
> +#include <asm/pgtable.h>
> +#include <asm/mmu.h>
> +#include <linux/uaccess.h>
> +#include <asm/smp.h>
> +#include <asm/machdep.h>
> +#include <asm/setup.h>
> +#include <asm/paca.h>
> +#include <mm/mmu_decl.h>
Do you really need all those headers?
> +extern int is_second_reloc;
That should be in a header.
Any reason why it isn't a bool?
cheers
Jason Yan <[email protected]> writes:
> M_IF_NEEDED is defined too many times. Move it to a common place.
The name is not great, can you call it MAS2_M_IF_NEEDED, which at least
gives a clue what it's for?
cheers
> Signed-off-by: Jason Yan <[email protected]>
> Cc: Diana Craciun <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: Christophe Leroy <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Nicholas Piggin <[email protected]>
> Cc: Kees Cook <[email protected]>
> Reviewed-by: Christophe Leroy <[email protected]>
> Reviewed-by: Diana Craciun <[email protected]>
> Tested-by: Diana Craciun <[email protected]>
> ---
> arch/powerpc/include/asm/nohash/mmu-book3e.h | 10 ++++++++++
> arch/powerpc/kernel/exceptions-64e.S | 10 ----------
> arch/powerpc/kernel/fsl_booke_entry_mapping.S | 10 ----------
> arch/powerpc/kernel/misc_64.S | 5 -----
> 4 files changed, 10 insertions(+), 25 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/nohash/mmu-book3e.h b/arch/powerpc/include/asm/nohash/mmu-book3e.h
> index 4c9777d256fb..0877362e48fa 100644
> --- a/arch/powerpc/include/asm/nohash/mmu-book3e.h
> +++ b/arch/powerpc/include/asm/nohash/mmu-book3e.h
> @@ -221,6 +221,16 @@
> #define TLBILX_T_CLASS2 6
> #define TLBILX_T_CLASS3 7
>
> +/*
> + * The mapping only needs to be cache-coherent on SMP, except on
> + * Freescale e500mc derivatives where it's also needed for coherent DMA.
> + */
> +#if defined(CONFIG_SMP) || defined(CONFIG_PPC_E500MC)
> +#define M_IF_NEEDED MAS2_M
> +#else
> +#define M_IF_NEEDED 0
> +#endif
> +
> #ifndef __ASSEMBLY__
> #include <asm/bug.h>
>
> diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
> index 1cfb3da4a84a..fd49ec07ce4a 100644
> --- a/arch/powerpc/kernel/exceptions-64e.S
> +++ b/arch/powerpc/kernel/exceptions-64e.S
> @@ -1342,16 +1342,6 @@ skpinv: addi r6,r6,1 /* Increment */
> sync
> isync
>
> -/*
> - * The mapping only needs to be cache-coherent on SMP, except on
> - * Freescale e500mc derivatives where it's also needed for coherent DMA.
> - */
> -#if defined(CONFIG_SMP) || defined(CONFIG_PPC_E500MC)
> -#define M_IF_NEEDED MAS2_M
> -#else
> -#define M_IF_NEEDED 0
> -#endif
> -
> /* 6. Setup KERNELBASE mapping in TLB[0]
> *
> * r3 = MAS0 w/TLBSEL & ESEL for the entry we started in
> diff --git a/arch/powerpc/kernel/fsl_booke_entry_mapping.S b/arch/powerpc/kernel/fsl_booke_entry_mapping.S
> index ea065282b303..de0980945510 100644
> --- a/arch/powerpc/kernel/fsl_booke_entry_mapping.S
> +++ b/arch/powerpc/kernel/fsl_booke_entry_mapping.S
> @@ -153,16 +153,6 @@ skpinv: addi r6,r6,1 /* Increment */
> tlbivax 0,r9
> TLBSYNC
>
> -/*
> - * The mapping only needs to be cache-coherent on SMP, except on
> - * Freescale e500mc derivatives where it's also needed for coherent DMA.
> - */
> -#if defined(CONFIG_SMP) || defined(CONFIG_PPC_E500MC)
> -#define M_IF_NEEDED MAS2_M
> -#else
> -#define M_IF_NEEDED 0
> -#endif
> -
> #if defined(ENTRY_MAPPING_BOOT_SETUP)
>
> /* 6. Setup KERNELBASE mapping in TLB1[0] */
> diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
> index b55a7b4cb543..26074f92d4bc 100644
> --- a/arch/powerpc/kernel/misc_64.S
> +++ b/arch/powerpc/kernel/misc_64.S
> @@ -432,11 +432,6 @@ kexec_create_tlb:
> rlwimi r9,r10,16,4,15 /* Setup MAS0 = TLBSEL | ESEL(r9) */
>
> /* Set up a temp identity mapping v:0 to p:0 and return to it. */
> -#if defined(CONFIG_SMP) || defined(CONFIG_PPC_E500MC)
> -#define M_IF_NEEDED MAS2_M
> -#else
> -#define M_IF_NEEDED 0
> -#endif
> mtspr SPRN_MAS0,r9
>
> lis r9,(MAS1_VALID|MAS1_IPROT)@h
> --
> 2.17.2
Jason Yan <[email protected]> writes:
> These two variables are both defined in init_32.c and init_64.c. Move
> them to init-common.c.
>
> Signed-off-by: Jason Yan <[email protected]>
> Cc: Diana Craciun <[email protected]>
> Cc: Michael Ellerman <[email protected]>
> Cc: Christophe Leroy <[email protected]>
> Cc: Benjamin Herrenschmidt <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> Cc: Nicholas Piggin <[email protected]>
> Cc: Kees Cook <[email protected]>
> Reviewed-by: Christophe Leroy <[email protected]>
> Reviewed-by: Diana Craciun <[email protected]>
> Tested-by: Diana Craciun <[email protected]>
> ---
> arch/powerpc/mm/init-common.c | 5 +++++
> arch/powerpc/mm/init_32.c | 5 -----
> arch/powerpc/mm/init_64.c | 5 -----
> 3 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
> index a84da92920f7..152ae0d21435 100644
> --- a/arch/powerpc/mm/init-common.c
> +++ b/arch/powerpc/mm/init-common.c
> @@ -21,6 +21,11 @@
> #include <asm/pgtable.h>
> #include <asm/kup.h>
>
> +phys_addr_t memstart_addr = (phys_addr_t)~0ull;
> +EXPORT_SYMBOL_GPL(memstart_addr);
> +phys_addr_t kernstart_addr;
> +EXPORT_SYMBOL_GPL(kernstart_addr);
Would be nice if these can be __ro_after_init ?
cheers
Jason Yan <[email protected]> writes:
> After we have the basic support of relocate the kernel in some
> appropriate place, we can start to randomize the offset now.
>
> Entropy is derived from the banner and timer, which will change every
> build and boot. This not so much safe so additionally the bootloader may
> pass entropy via the /chosen/kaslr-seed node in device tree.
>
> We will use the first 512M of the low memory to randomize the kernel
> image. The memory will be split in 64M zones. We will use the lower 8
> bit of the entropy to decide the index of the 64M zone. Then we chose a
> 16K aligned offset inside the 64M zone to put the kernel in.
>
> KERNELBASE
>
> |--> 64M <--|
> | |
> +---------------+ +----------------+---------------+
> | |....| |kernel| | |
> +---------------+ +----------------+---------------+
> | |
> |-----> offset <-----|
>
> kimage_vaddr
Can you drop this description / diagram and any other relevant design
details in eg. Documentation/powerpc/kaslr-booke32.rst please?
See cpu_families.rst for an example of how to incorporate the ASCII
diagram.
> diff --git a/arch/powerpc/kernel/kaslr_booke.c b/arch/powerpc/kernel/kaslr_booke.c
> index 30f84c0321b2..52b59b05f906 100644
> --- a/arch/powerpc/kernel/kaslr_booke.c
> +++ b/arch/powerpc/kernel/kaslr_booke.c
> @@ -34,15 +36,329 @@
> #include <asm/machdep.h>
> #include <asm/setup.h>
> #include <asm/paca.h>
> +#include <asm/kdump.h>
> #include <mm/mmu_decl.h>
> +#include <generated/compile.h>
> +#include <generated/utsrelease.h>
> +
> +#ifdef DEBUG
> +#define DBG(fmt...) pr_info(fmt)
> +#else
> +#define DBG(fmt...)
> +#endif
Just use pr_debug()?
> +struct regions {
> + unsigned long pa_start;
> + unsigned long pa_end;
> + unsigned long kernel_size;
> + unsigned long dtb_start;
> + unsigned long dtb_end;
> + unsigned long initrd_start;
> + unsigned long initrd_end;
> + unsigned long crash_start;
> + unsigned long crash_end;
> + int reserved_mem;
> + int reserved_mem_addr_cells;
> + int reserved_mem_size_cells;
> +};
>
> extern int is_second_reloc;
>
> +/* Simplified build-specific string for starting entropy. */
> +static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
> + LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
> +
> +static __init void kaslr_get_cmdline(void *fdt)
> +{
> + int node = fdt_path_offset(fdt, "/chosen");
> +
> + early_init_dt_scan_chosen(node, "chosen", 1, boot_command_line);
> +}
> +
> +static unsigned long __init rotate_xor(unsigned long hash, const void *area,
> + size_t size)
> +{
> + size_t i;
> + const unsigned long *ptr = area;
> +
> + for (i = 0; i < size / sizeof(hash); i++) {
> + /* Rotate by odd number of bits and XOR. */
> + hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
> + hash ^= ptr[i];
> + }
> +
> + return hash;
> +}
That looks suspiciously like the version Kees wrote in 2013 in
arch/x86/boot/compressed/kaslr.c ?
You should mention that in the change log at least.
> +
> +/* Attempt to create a simple but unpredictable starting entropy. */
It's simple, but I would argue unpredictable is not really true. A local
attacker can probably fingerprint the kernel version, and also has
access to the unflattened device tree, which means they can make
educated guesses about the flattened tree size.
Be careful when copying comments :)
> +static unsigned long __init get_boot_seed(void *fdt)
> +{
> + unsigned long hash = 0;
> +
> + hash = rotate_xor(hash, build_str, sizeof(build_str));
> + hash = rotate_xor(hash, fdt, fdt_totalsize(fdt));
> +
> + return hash;
> +}
> +
> +static __init u64 get_kaslr_seed(void *fdt)
> +{
> + int node, len;
> + fdt64_t *prop;
> + u64 ret;
> +
> + node = fdt_path_offset(fdt, "/chosen");
> + if (node < 0)
> + return 0;
> +
> + prop = fdt_getprop_w(fdt, node, "kaslr-seed", &len);
> + if (!prop || len != sizeof(u64))
> + return 0;
> +
> + ret = fdt64_to_cpu(*prop);
> + *prop = 0;
> + return ret;
> +}
> +
> +static __init bool regions_overlap(u32 s1, u32 e1, u32 s2, u32 e2)
> +{
> + return e1 >= s2 && e2 >= s1;
> +}
There's a generic helper called memory_intersects(), though it takes
void*. Might not be worth using, not sure.
...
> static unsigned long __init kaslr_choose_location(void *dt_ptr, phys_addr_t size,
> unsigned long kernel_sz)
> {
> - /* return a fixed offset of 64M for now */
> - return SZ_64M;
> + unsigned long offset, random;
> + unsigned long ram, linear_sz;
> + unsigned long kaslr_offset;
> + u64 seed;
> + struct regions regions;
You pass that around to a lot of the functions, would it be simpler just
to make it static global and __initdata ?
cheers
Hi Jason,
Jason Yan <[email protected]> writes:
> This series implements KASLR for powerpc/fsl_booke/32, as a security
> feature that deters exploit attempts relying on knowledge of the location
> of kernel internals.
Thanks for doing this work.
Sorry I didn't get a chance to look at this until v5, I sent a few
comments just now. Nothing major though, I think this looks almost ready
to merge.
cheers
> Since CONFIG_RELOCATABLE has already supported, what we need to do is
> map or copy kernel to a proper place and relocate. Freescale Book-E
> parts expect lowmem to be mapped by fixed TLB entries(TLB1). The TLB1
> entries are not suitable to map the kernel directly in a randomized
> region, so we chose to copy the kernel to a proper place and restart to
> relocate.
>
> Entropy is derived from the banner and timer base, which will change every
> build and boot. This not so much safe so additionally the bootloader may
> pass entropy via the /chosen/kaslr-seed node in device tree.
>
> We will use the first 512M of the low memory to randomize the kernel
> image. The memory will be split in 64M zones. We will use the lower 8
> bit of the entropy to decide the index of the 64M zone. Then we chose a
> 16K aligned offset inside the 64M zone to put the kernel in.
>
> KERNELBASE
>
> |--> 64M <--|
> | |
> +---------------+ +----------------+---------------+
> | |....| |kernel| | |
> +---------------+ +----------------+---------------+
> | |
> |-----> offset <-----|
>
> kimage_vaddr
>
> We also check if we will overlap with some areas like the dtb area, the
> initrd area or the crashkernel area. If we cannot find a proper area,
> kaslr will be disabled and boot from the original kernel.
>
> Changes since v4:
> - Add Reviewed-by tag from Christophe
> - Remove an unnecessary cast
> - Remove unnecessary parenthesis
> - Fix checkpatch warning
>
> Changes since v3:
> - Add Reviewed-by and Tested-by tag from Diana
> - Change the comment in fsl_booke_entry_mapping.S to be consistent
> with the new code.
>
> Changes since v2:
> - Remove unnecessary #ifdef
> - Use SZ_64M instead of0x4000000
> - Call early_init_dt_scan_chosen() to init boot_command_line
> - Rename kaslr_second_init() to kaslr_late_init()
>
> Changes since v1:
> - Remove some useless 'extern' keyword.
> - Replace EXPORT_SYMBOL with EXPORT_SYMBOL_GPL
> - Improve some assembly code
> - Use memzero_explicit instead of memset
> - Use boot_command_line and remove early_command_line
> - Do not print kaslr offset if kaslr is disabled
>
> Jason Yan (10):
> powerpc: unify definition of M_IF_NEEDED
> powerpc: move memstart_addr and kernstart_addr to init-common.c
> powerpc: introduce kimage_vaddr to store the kernel base
> powerpc/fsl_booke/32: introduce create_tlb_entry() helper
> powerpc/fsl_booke/32: introduce reloc_kernel_entry() helper
> powerpc/fsl_booke/32: implement KASLR infrastructure
> powerpc/fsl_booke/32: randomize the kernel image offset
> powerpc/fsl_booke/kaslr: clear the original kernel if randomized
> powerpc/fsl_booke/kaslr: support nokaslr cmdline parameter
> powerpc/fsl_booke/kaslr: dump out kernel offset information on panic
>
> arch/powerpc/Kconfig | 11 +
> arch/powerpc/include/asm/nohash/mmu-book3e.h | 10 +
> arch/powerpc/include/asm/page.h | 7 +
> arch/powerpc/kernel/Makefile | 1 +
> arch/powerpc/kernel/early_32.c | 2 +-
> arch/powerpc/kernel/exceptions-64e.S | 10 -
> arch/powerpc/kernel/fsl_booke_entry_mapping.S | 27 +-
> arch/powerpc/kernel/head_fsl_booke.S | 55 ++-
> arch/powerpc/kernel/kaslr_booke.c | 427 ++++++++++++++++++
> arch/powerpc/kernel/machine_kexec.c | 1 +
> arch/powerpc/kernel/misc_64.S | 5 -
> arch/powerpc/kernel/setup-common.c | 19 +
> arch/powerpc/mm/init-common.c | 7 +
> arch/powerpc/mm/init_32.c | 5 -
> arch/powerpc/mm/init_64.c | 5 -
> arch/powerpc/mm/mmu_decl.h | 10 +
> arch/powerpc/mm/nohash/fsl_booke.c | 8 +-
> 17 files changed, 560 insertions(+), 50 deletions(-)
> create mode 100644 arch/powerpc/kernel/kaslr_booke.c
>
> --
> 2.17.2
On 2019/8/7 21:12, Michael Ellerman wrote:
> Hi Jason,
>
> Jason Yan <[email protected]> writes:
>> This series implements KASLR for powerpc/fsl_booke/32, as a security
>> feature that deters exploit attempts relying on knowledge of the location
>> of kernel internals.
>
> Thanks for doing this work.
>
> Sorry I didn't get a chance to look at this until v5, I sent a few
> comments just now. Nothing major though, I think this looks almost ready
> to merge.
>
Thank you. I will try my best to improve the code.
> cheers
>
>> Since CONFIG_RELOCATABLE has already supported, what we need to do is
>> map or copy kernel to a proper place and relocate. Freescale Book-E
>> parts expect lowmem to be mapped by fixed TLB entries(TLB1). The TLB1
>> entries are not suitable to map the kernel directly in a randomized
>> region, so we chose to copy the kernel to a proper place and restart to
>> relocate.
>>
>> Entropy is derived from the banner and timer base, which will change every
>> build and boot. This not so much safe so additionally the bootloader may
>> pass entropy via the /chosen/kaslr-seed node in device tree.
>>
>> We will use the first 512M of the low memory to randomize the kernel
>> image. The memory will be split in 64M zones. We will use the lower 8
>> bit of the entropy to decide the index of the 64M zone. Then we chose a
>> 16K aligned offset inside the 64M zone to put the kernel in.
>>
>> KERNELBASE
>>
>> |--> 64M <--|
>> | |
>> +---------------+ +----------------+---------------+
>> | |....| |kernel| | |
>> +---------------+ +----------------+---------------+
>> | |
>> |-----> offset <-----|
>>
>> kimage_vaddr
>>
>> We also check if we will overlap with some areas like the dtb area, the
>> initrd area or the crashkernel area. If we cannot find a proper area,
>> kaslr will be disabled and boot from the original kernel.
>>
>> Changes since v4:
>> - Add Reviewed-by tag from Christophe
>> - Remove an unnecessary cast
>> - Remove unnecessary parenthesis
>> - Fix checkpatch warning
>>
>> Changes since v3:
>> - Add Reviewed-by and Tested-by tag from Diana
>> - Change the comment in fsl_booke_entry_mapping.S to be consistent
>> with the new code.
>>
>> Changes since v2:
>> - Remove unnecessary #ifdef
>> - Use SZ_64M instead of0x4000000
>> - Call early_init_dt_scan_chosen() to init boot_command_line
>> - Rename kaslr_second_init() to kaslr_late_init()
>>
>> Changes since v1:
>> - Remove some useless 'extern' keyword.
>> - Replace EXPORT_SYMBOL with EXPORT_SYMBOL_GPL
>> - Improve some assembly code
>> - Use memzero_explicit instead of memset
>> - Use boot_command_line and remove early_command_line
>> - Do not print kaslr offset if kaslr is disabled
>>
>> Jason Yan (10):
>> powerpc: unify definition of M_IF_NEEDED
>> powerpc: move memstart_addr and kernstart_addr to init-common.c
>> powerpc: introduce kimage_vaddr to store the kernel base
>> powerpc/fsl_booke/32: introduce create_tlb_entry() helper
>> powerpc/fsl_booke/32: introduce reloc_kernel_entry() helper
>> powerpc/fsl_booke/32: implement KASLR infrastructure
>> powerpc/fsl_booke/32: randomize the kernel image offset
>> powerpc/fsl_booke/kaslr: clear the original kernel if randomized
>> powerpc/fsl_booke/kaslr: support nokaslr cmdline parameter
>> powerpc/fsl_booke/kaslr: dump out kernel offset information on panic
>>
>> arch/powerpc/Kconfig | 11 +
>> arch/powerpc/include/asm/nohash/mmu-book3e.h | 10 +
>> arch/powerpc/include/asm/page.h | 7 +
>> arch/powerpc/kernel/Makefile | 1 +
>> arch/powerpc/kernel/early_32.c | 2 +-
>> arch/powerpc/kernel/exceptions-64e.S | 10 -
>> arch/powerpc/kernel/fsl_booke_entry_mapping.S | 27 +-
>> arch/powerpc/kernel/head_fsl_booke.S | 55 ++-
>> arch/powerpc/kernel/kaslr_booke.c | 427 ++++++++++++++++++
>> arch/powerpc/kernel/machine_kexec.c | 1 +
>> arch/powerpc/kernel/misc_64.S | 5 -
>> arch/powerpc/kernel/setup-common.c | 19 +
>> arch/powerpc/mm/init-common.c | 7 +
>> arch/powerpc/mm/init_32.c | 5 -
>> arch/powerpc/mm/init_64.c | 5 -
>> arch/powerpc/mm/mmu_decl.h | 10 +
>> arch/powerpc/mm/nohash/fsl_booke.c | 8 +-
>> 17 files changed, 560 insertions(+), 50 deletions(-)
>> create mode 100644 arch/powerpc/kernel/kaslr_booke.c
>>
>> --
>> 2.17.2
>
> .
>
On 2019/8/7 21:13, Michael Ellerman wrote:
> Jason Yan <[email protected]> writes:
>> M_IF_NEEDED is defined too many times. Move it to a common place.
>
> The name is not great, can you call it MAS2_M_IF_NEEDED, which at least
> gives a clue what it's for?
>
OK.
> cheers
>
>> Signed-off-by: Jason Yan <[email protected]>
>> Cc: Diana Craciun <[email protected]>
>> Cc: Michael Ellerman <[email protected]>
>> Cc: Christophe Leroy <[email protected]>
>> Cc: Benjamin Herrenschmidt <[email protected]>
>> Cc: Paul Mackerras <[email protected]>
>> Cc: Nicholas Piggin <[email protected]>
>> Cc: Kees Cook <[email protected]>
>> Reviewed-by: Christophe Leroy <[email protected]>
>> Reviewed-by: Diana Craciun <[email protected]>
>> Tested-by: Diana Craciun <[email protected]>
>> ---
>> arch/powerpc/include/asm/nohash/mmu-book3e.h | 10 ++++++++++
>> arch/powerpc/kernel/exceptions-64e.S | 10 ----------
>> arch/powerpc/kernel/fsl_booke_entry_mapping.S | 10 ----------
>> arch/powerpc/kernel/misc_64.S | 5 -----
>> 4 files changed, 10 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/nohash/mmu-book3e.h b/arch/powerpc/include/asm/nohash/mmu-book3e.h
>> index 4c9777d256fb..0877362e48fa 100644
>> --- a/arch/powerpc/include/asm/nohash/mmu-book3e.h
>> +++ b/arch/powerpc/include/asm/nohash/mmu-book3e.h
>> @@ -221,6 +221,16 @@
>> #define TLBILX_T_CLASS2 6
>> #define TLBILX_T_CLASS3 7
>>
>> +/*
>> + * The mapping only needs to be cache-coherent on SMP, except on
>> + * Freescale e500mc derivatives where it's also needed for coherent DMA.
>> + */
>> +#if defined(CONFIG_SMP) || defined(CONFIG_PPC_E500MC)
>> +#define M_IF_NEEDED MAS2_M
>> +#else
>> +#define M_IF_NEEDED 0
>> +#endif
>> +
>> #ifndef __ASSEMBLY__
>> #include <asm/bug.h>
>>
>> diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
>> index 1cfb3da4a84a..fd49ec07ce4a 100644
>> --- a/arch/powerpc/kernel/exceptions-64e.S
>> +++ b/arch/powerpc/kernel/exceptions-64e.S
>> @@ -1342,16 +1342,6 @@ skpinv: addi r6,r6,1 /* Increment */
>> sync
>> isync
>>
>> -/*
>> - * The mapping only needs to be cache-coherent on SMP, except on
>> - * Freescale e500mc derivatives where it's also needed for coherent DMA.
>> - */
>> -#if defined(CONFIG_SMP) || defined(CONFIG_PPC_E500MC)
>> -#define M_IF_NEEDED MAS2_M
>> -#else
>> -#define M_IF_NEEDED 0
>> -#endif
>> -
>> /* 6. Setup KERNELBASE mapping in TLB[0]
>> *
>> * r3 = MAS0 w/TLBSEL & ESEL for the entry we started in
>> diff --git a/arch/powerpc/kernel/fsl_booke_entry_mapping.S b/arch/powerpc/kernel/fsl_booke_entry_mapping.S
>> index ea065282b303..de0980945510 100644
>> --- a/arch/powerpc/kernel/fsl_booke_entry_mapping.S
>> +++ b/arch/powerpc/kernel/fsl_booke_entry_mapping.S
>> @@ -153,16 +153,6 @@ skpinv: addi r6,r6,1 /* Increment */
>> tlbivax 0,r9
>> TLBSYNC
>>
>> -/*
>> - * The mapping only needs to be cache-coherent on SMP, except on
>> - * Freescale e500mc derivatives where it's also needed for coherent DMA.
>> - */
>> -#if defined(CONFIG_SMP) || defined(CONFIG_PPC_E500MC)
>> -#define M_IF_NEEDED MAS2_M
>> -#else
>> -#define M_IF_NEEDED 0
>> -#endif
>> -
>> #if defined(ENTRY_MAPPING_BOOT_SETUP)
>>
>> /* 6. Setup KERNELBASE mapping in TLB1[0] */
>> diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S
>> index b55a7b4cb543..26074f92d4bc 100644
>> --- a/arch/powerpc/kernel/misc_64.S
>> +++ b/arch/powerpc/kernel/misc_64.S
>> @@ -432,11 +432,6 @@ kexec_create_tlb:
>> rlwimi r9,r10,16,4,15 /* Setup MAS0 = TLBSEL | ESEL(r9) */
>>
>> /* Set up a temp identity mapping v:0 to p:0 and return to it. */
>> -#if defined(CONFIG_SMP) || defined(CONFIG_PPC_E500MC)
>> -#define M_IF_NEEDED MAS2_M
>> -#else
>> -#define M_IF_NEEDED 0
>> -#endif
>> mtspr SPRN_MAS0,r9
>>
>> lis r9,(MAS1_VALID|MAS1_IPROT)@h
>> --
>> 2.17.2
>
> .
>
On 2019/8/7 21:02, Michael Ellerman wrote:
> Jason Yan <[email protected]> writes:
>> These two variables are both defined in init_32.c and init_64.c. Move
>> them to init-common.c.
>>
>> Signed-off-by: Jason Yan <[email protected]>
>> Cc: Diana Craciun <[email protected]>
>> Cc: Michael Ellerman <[email protected]>
>> Cc: Christophe Leroy <[email protected]>
>> Cc: Benjamin Herrenschmidt <[email protected]>
>> Cc: Paul Mackerras <[email protected]>
>> Cc: Nicholas Piggin <[email protected]>
>> Cc: Kees Cook <[email protected]>
>> Reviewed-by: Christophe Leroy <[email protected]>
>> Reviewed-by: Diana Craciun <[email protected]>
>> Tested-by: Diana Craciun <[email protected]>
>> ---
>> arch/powerpc/mm/init-common.c | 5 +++++
>> arch/powerpc/mm/init_32.c | 5 -----
>> arch/powerpc/mm/init_64.c | 5 -----
>> 3 files changed, 5 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
>> index a84da92920f7..152ae0d21435 100644
>> --- a/arch/powerpc/mm/init-common.c
>> +++ b/arch/powerpc/mm/init-common.c
>> @@ -21,6 +21,11 @@
>> #include <asm/pgtable.h>
>> #include <asm/kup.h>
>>
>> +phys_addr_t memstart_addr = (phys_addr_t)~0ull;
>> +EXPORT_SYMBOL_GPL(memstart_addr);
>> +phys_addr_t kernstart_addr;
>> +EXPORT_SYMBOL_GPL(kernstart_addr);
>
> Would be nice if these can be __ro_after_init ?
>
Good idea.
> cheers
>
> .
>
On 2019/8/7 21:03, Michael Ellerman wrote:
> Jason Yan <[email protected]> writes:
>> Now the kernel base is a fixed value - KERNELBASE. To support KASLR, we
>> need a variable to store the kernel base.
>>
>> Signed-off-by: Jason Yan <[email protected]>
>> Cc: Diana Craciun <[email protected]>
>> Cc: Michael Ellerman <[email protected]>
>> Cc: Christophe Leroy <[email protected]>
>> Cc: Benjamin Herrenschmidt <[email protected]>
>> Cc: Paul Mackerras <[email protected]>
>> Cc: Nicholas Piggin <[email protected]>
>> Cc: Kees Cook <[email protected]>
>> Reviewed-by: Christophe Leroy <[email protected]>
>> Reviewed-by: Diana Craciun <[email protected]>
>> Tested-by: Diana Craciun <[email protected]>
>> ---
>> arch/powerpc/include/asm/page.h | 2 ++
>> arch/powerpc/mm/init-common.c | 2 ++
>> 2 files changed, 4 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
>> index 0d52f57fca04..60a68d3a54b1 100644
>> --- a/arch/powerpc/include/asm/page.h
>> +++ b/arch/powerpc/include/asm/page.h
>> @@ -315,6 +315,8 @@ void arch_free_page(struct page *page, int order);
>>
>> struct vm_area_struct;
>>
>> +extern unsigned long kimage_vaddr;
>> +
>> #include <asm-generic/memory_model.h>
>> #endif /* __ASSEMBLY__ */
>> #include <asm/slice.h>
>> diff --git a/arch/powerpc/mm/init-common.c b/arch/powerpc/mm/init-common.c
>> index 152ae0d21435..d4801ce48dc5 100644
>> --- a/arch/powerpc/mm/init-common.c
>> +++ b/arch/powerpc/mm/init-common.c
>> @@ -25,6 +25,8 @@ phys_addr_t memstart_addr = (phys_addr_t)~0ull;
>> EXPORT_SYMBOL_GPL(memstart_addr);
>> phys_addr_t kernstart_addr;
>> EXPORT_SYMBOL_GPL(kernstart_addr);
>> +unsigned long kimage_vaddr = KERNELBASE;
>> +EXPORT_SYMBOL_GPL(kimage_vaddr);
>
> The names of the #defines and variables we use for these values are not
> very consistent already, but using kimage_vaddr makes it worse I think.
>
> Isn't this going to have the same value as kernstart_addr, but the
> virtual rather than physical address?
>
Yes, that's true.
> If so kernstart_virt_addr would seem better.
>
OK, I will take kernstart_virt_addr if no better name appears.
> cheers
>
> .
>
On 2019/8/7 21:04, Michael Ellerman wrote:
> Jason Yan <[email protected]> writes:
>> This patch add support to boot kernel from places other than KERNELBASE.
>> Since CONFIG_RELOCATABLE has already supported, what we need to do is
>> map or copy kernel to a proper place and relocate. Freescale Book-E
>> parts expect lowmem to be mapped by fixed TLB entries(TLB1). The TLB1
>> entries are not suitable to map the kernel directly in a randomized
>> region, so we chose to copy the kernel to a proper place and restart to
>> relocate.
>
> So to be 100% clear you are randomising the location of the kernel in
> virtual and physical space, by the same amount, and retaining the 1:1
> linear mapping.
>
100% right :)
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 77f6ebf97113..755378887912 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -548,6 +548,17 @@ config RELOCATABLE
>> setting can still be useful to bootwrappers that need to know the
>> load address of the kernel (eg. u-boot/mkimage).
>>
>> +config RANDOMIZE_BASE
>> + bool "Randomize the address of the kernel image"
>> + depends on (FSL_BOOKE && FLATMEM && PPC32)
>> + select RELOCATABLE
>
> I think this should depend on RELOCATABLE, rather than selecting it.
>
>> diff --git a/arch/powerpc/kernel/kaslr_booke.c b/arch/powerpc/kernel/kaslr_booke.c
>> new file mode 100644
>> index 000000000000..30f84c0321b2
>> --- /dev/null
>> +++ b/arch/powerpc/kernel/kaslr_booke.c
>> @@ -0,0 +1,84 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (C) 2019 Jason Yan <[email protected]>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>
> You don't need that paragraph now that you have the SPDX tag.
>
> Rather than using a '//' comment followed by a single line block comment
> you can format it as:
>
> // SPDX-License-Identifier: GPL-2.0-only
> //
> // Copyright (C) 2019 Jason Yan <[email protected]>
> >
>> +#include <linux/signal.h>
>> +#include <linux/sched.h>
>> +#include <linux/kernel.h>
>> +#include <linux/errno.h>
>> +#include <linux/string.h>
>> +#include <linux/types.h>
>> +#include <linux/ptrace.h>
>> +#include <linux/mman.h>
>> +#include <linux/mm.h>
>> +#include <linux/swap.h>
>> +#include <linux/stddef.h>
>> +#include <linux/vmalloc.h>
>> +#include <linux/init.h>
>> +#include <linux/delay.h>
>> +#include <linux/highmem.h>
>> +#include <linux/memblock.h>
>> +#include <asm/pgalloc.h>
>> +#include <asm/prom.h>
>> +#include <asm/io.h>
>> +#include <asm/mmu_context.h>
>> +#include <asm/pgtable.h>
>> +#include <asm/mmu.h>
>> +#include <linux/uaccess.h>
>> +#include <asm/smp.h>
>> +#include <asm/machdep.h>
>> +#include <asm/setup.h>
>> +#include <asm/paca.h>
>> +#include <mm/mmu_decl.h>
>
> Do you really need all those headers?
>
I will remove useless headers.
>> +extern int is_second_reloc;
>
> That should be in a header.
>
> Any reason why it isn't a bool?
>
Oh yes, it should be in a header. This variable is already defined
before and also used in assembly code. I think it was not defined as a
bool just because there is no 'bool' in assembly code.
> cheers
>
>
> .
>
On 2019/8/7 21:03, Michael Ellerman wrote:
> Jason Yan <[email protected]> writes:
>> After we have the basic support of relocate the kernel in some
>> appropriate place, we can start to randomize the offset now.
>>
>> Entropy is derived from the banner and timer, which will change every
>> build and boot. This not so much safe so additionally the bootloader may
>> pass entropy via the /chosen/kaslr-seed node in device tree.
>>
>> We will use the first 512M of the low memory to randomize the kernel
>> image. The memory will be split in 64M zones. We will use the lower 8
>> bit of the entropy to decide the index of the 64M zone. Then we chose a
>> 16K aligned offset inside the 64M zone to put the kernel in.
>>
>> KERNELBASE
>>
>> |--> 64M <--|
>> | |
>> +---------------+ +----------------+---------------+
>> | |....| |kernel| | |
>> +---------------+ +----------------+---------------+
>> | |
>> |-----> offset <-----|
>>
>> kimage_vaddr
>
> Can you drop this description / diagram and any other relevant design
> details in eg. Documentation/powerpc/kaslr-booke32.rst please?
>
No problem.
> See cpu_families.rst for an example of how to incorporate the ASCII
> diagram.
> >> diff --git a/arch/powerpc/kernel/kaslr_booke.c
b/arch/powerpc/kernel/kaslr_booke.c
>> index 30f84c0321b2..52b59b05f906 100644
>> --- a/arch/powerpc/kernel/kaslr_booke.c
>> +++ b/arch/powerpc/kernel/kaslr_booke.c
>> @@ -34,15 +36,329 @@
>> #include <asm/machdep.h>
>> #include <asm/setup.h>
>> #include <asm/paca.h>
>> +#include <asm/kdump.h>
>> #include <mm/mmu_decl.h>
>> +#include <generated/compile.h>
>> +#include <generated/utsrelease.h>
>> +
>> +#ifdef DEBUG
>> +#define DBG(fmt...) pr_info(fmt)
>> +#else
>> +#define DBG(fmt...)
>> +#endif
>
> Just use pr_debug()?
>
Sounds better.
>> +struct regions {
>> + unsigned long pa_start;
>> + unsigned long pa_end;
>> + unsigned long kernel_size;
>> + unsigned long dtb_start;
>> + unsigned long dtb_end;
>> + unsigned long initrd_start;
>> + unsigned long initrd_end;
>> + unsigned long crash_start;
>> + unsigned long crash_end;
>> + int reserved_mem;
>> + int reserved_mem_addr_cells;
>> + int reserved_mem_size_cells;
>> +};
>>
>> extern int is_second_reloc;
>>
>> +/* Simplified build-specific string for starting entropy. */
>> +static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
>> + LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
>> +
>> +static __init void kaslr_get_cmdline(void *fdt)
>> +{
>> + int node = fdt_path_offset(fdt, "/chosen");
>> +
>> + early_init_dt_scan_chosen(node, "chosen", 1, boot_command_line);
>> +}
>> +
>> +static unsigned long __init rotate_xor(unsigned long hash, const void *area,
>> + size_t size)
>> +{
>> + size_t i;
>> + const unsigned long *ptr = area;
>> +
>> + for (i = 0; i < size / sizeof(hash); i++) {
>> + /* Rotate by odd number of bits and XOR. */
>> + hash = (hash << ((sizeof(hash) * 8) - 7)) | (hash >> 7);
>> + hash ^= ptr[i];
>> + }
>> +
>> + return hash;
>> +}
>
> That looks suspiciously like the version Kees wrote in 2013 in
> arch/x86/boot/compressed/kaslr.c ?
>
> You should mention that in the change log at least.
>
Oh yes, I should have do that. Thanks for reminding me.
>> +
>> +/* Attempt to create a simple but unpredictable starting entropy. */
>
> It's simple, but I would argue unpredictable is not really true. A local
> attacker can probably fingerprint the kernel version, and also has
> access to the unflattened device tree, which means they can make
> educated guesses about the flattened tree size.
>
> Be careful when copying comments :)
>
It's true that the comment is not so precise. It's an 'attempt' to
create unpredictable entropy. And apparently the 'attempt' was failed.
I will try to rewrite the comment to reflect the code more precisely.
>> +static unsigned long __init get_boot_seed(void *fdt)
>> +{
>> + unsigned long hash = 0;
>> +
>> + hash = rotate_xor(hash, build_str, sizeof(build_str));
>> + hash = rotate_xor(hash, fdt, fdt_totalsize(fdt));
>> +
>> + return hash;
>> +}
>> +
>> +static __init u64 get_kaslr_seed(void *fdt)
>> +{
>> + int node, len;
>> + fdt64_t *prop;
>> + u64 ret;
>> +
>> + node = fdt_path_offset(fdt, "/chosen");
>> + if (node < 0)
>> + return 0;
>> +
>> + prop = fdt_getprop_w(fdt, node, "kaslr-seed", &len);
>> + if (!prop || len != sizeof(u64))
>> + return 0;
>> +
>> + ret = fdt64_to_cpu(*prop);
>> + *prop = 0;
>> + return ret;
>> +}
>> +
>> +static __init bool regions_overlap(u32 s1, u32 e1, u32 s2, u32 e2)
>> +{
>> + return e1 >= s2 && e2 >= s1;
>> +}
>
> There's a generic helper called memory_intersects(), though it takes
> void*. Might not be worth using, not sure.
>
I will have a try to see if this can save some codes or not.
> ...
>> static unsigned long __init kaslr_choose_location(void *dt_ptr, phys_addr_t size,
>> unsigned long kernel_sz)
>> {
>> - /* return a fixed offset of 64M for now */
>> - return SZ_64M;
>> + unsigned long offset, random;
>> + unsigned long ram, linear_sz;
>> + unsigned long kaslr_offset;
>> + u64 seed;
>> + struct regions regions;
>
> You pass that around to a lot of the functions, would it be simpler just
> to make it static global and __initdata ?
>
Not sure if it's simpler. Let me have a try.
> cheers
>
>
> .
>
On 2019/8/7 21:03, Michael Ellerman wrote:
> Jason Yan <[email protected]> writes:
>> diff --git a/arch/powerpc/kernel/kaslr_booke.c b/arch/powerpc/kernel/kaslr_booke.c
>> index c6b326424b54..436f9a03f385 100644
>> --- a/arch/powerpc/kernel/kaslr_booke.c
>> +++ b/arch/powerpc/kernel/kaslr_booke.c
>> @@ -361,6 +361,18 @@ static unsigned long __init kaslr_choose_location(void *dt_ptr, phys_addr_t size
>> return kaslr_offset;
>> }
>>
>> +static inline __init bool kaslr_disabled(void)
>> +{
>> + char *str;
>> +
>> + str = strstr(boot_command_line, "nokaslr");
>> + if (str == boot_command_line ||
>> + (str > boot_command_line && *(str - 1) == ' '))
>> + return true;
>
> This extra logic doesn't work for "nokaslrfoo". Is it worth it?
>
Seems nobody likes this logic. Maybe I can delete this logic for now and
see if anyone has any objections.
> cheers
>
> .
>
On 2019/8/7 21:03, Michael Ellerman wrote:
> Jason Yan <[email protected]> writes:
>> When kaslr is enabled, the kernel offset is different for every boot.
>> This brings some difficult to debug the kernel. Dump out the kernel
>> offset when panic so that we can easily debug the kernel.
>
> Some of this is taken from the arm64 version right? Please say so when
> you copy other people's code.
>
No problem. Architectures like x86 or arm64 or s390 both have this
similar code. I guess x86 is the first one.
>> diff --git a/arch/powerpc/kernel/machine_kexec.c b/arch/powerpc/kernel/machine_kexec.c
>> index c4ed328a7b96..078fe3d76feb 100644
>> --- a/arch/powerpc/kernel/machine_kexec.c
>> +++ b/arch/powerpc/kernel/machine_kexec.c
>> @@ -86,6 +86,7 @@ void arch_crash_save_vmcoreinfo(void)
>> VMCOREINFO_STRUCT_SIZE(mmu_psize_def);
>> VMCOREINFO_OFFSET(mmu_psize_def, shift);
>> #endif
>> + vmcoreinfo_append_str("KERNELOFFSET=%lx\n", kaslr_offset());
>> }
>
> There's no mention of that in the commit log.
>
> Please split it into a separate patch and describe what you're doing and
> why.
OK
>
>> diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
>> index 1f8db666468d..064075f02837 100644
>> --- a/arch/powerpc/kernel/setup-common.c
>> +++ b/arch/powerpc/kernel/setup-common.c
>> @@ -715,12 +715,31 @@ static struct notifier_block ppc_panic_block = {
>> .priority = INT_MIN /* may not return; must be done last */
>> };
>>
>> +/*
>> + * Dump out kernel offset information on panic.
>> + */
>> +static int dump_kernel_offset(struct notifier_block *self, unsigned long v,
>> + void *p)
>> +{
>> + pr_emerg("Kernel Offset: 0x%lx from 0x%lx\n",
>> + kaslr_offset(), KERNELBASE);
>> +
>> + return 0;
>> +}
>> +
>> +static struct notifier_block kernel_offset_notifier = {
>> + .notifier_call = dump_kernel_offset
>> +};
>> +
>> void __init setup_panic(void)
>> {
>> /* PPC64 always does a hard irq disable in its panic handler */
>> if (!IS_ENABLED(CONFIG_PPC64) && !ppc_md.panic)
>> return;
>> atomic_notifier_chain_register(&panic_notifier_list, &ppc_panic_block);
>
>> + if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && kaslr_offset() > 0)
>> + atomic_notifier_chain_register(&panic_notifier_list,
>> + &kernel_offset_notifier);
>
> Don't you want to do that before the return above?
>
Eagle eye. This should not affected by the conditions above.
>> }
>
> cheers
>
> .
>