Re: [PATCH RFC 6/8] riscv/kaslr: clear the original kernel image

From: Alex Ghiti
Date: Thu Apr 09 2020 - 01:53:42 EST



On 4/7/20 7:18 AM, Zong Li wrote:
On Tue, Apr 7, 2020 at 1:11 PM Alex Ghiti <alex@xxxxxxxx> wrote:

On 3/24/20 3:30 AM, Zong Li wrote:
After completing final page table, we can clear original kernel image
and remove executable permission.

Signed-off-by: Zong Li <zong.li@xxxxxxxxxx>
---
arch/riscv/include/asm/kaslr.h | 12 ++++++++++++
arch/riscv/kernel/kaslr.c | 12 ++++++++++++
arch/riscv/mm/init.c | 6 ++++++
3 files changed, 30 insertions(+)
create mode 100644 arch/riscv/include/asm/kaslr.h

diff --git a/arch/riscv/include/asm/kaslr.h b/arch/riscv/include/asm/kaslr.h
new file mode 100644
index 000000000000..b165fe71dd4a
--- /dev/null
+++ b/arch/riscv/include/asm/kaslr.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 SiFive
+ * Copyright (C) 2020 Zong Li <zong.li@xxxxxxxxxx>
+ */
+
+#ifndef _ASM_RISCV_KASLR_H
+#define _ASM_RISCV_KASLR_H
+
+void __init kaslr_late_init(void);
+
+#endif /* _ASM_RISCV_KASLR_H */
diff --git a/arch/riscv/kernel/kaslr.c b/arch/riscv/kernel/kaslr.c
index 59001d6fdfc3..0bd30831c455 100644
--- a/arch/riscv/kernel/kaslr.c
+++ b/arch/riscv/kernel/kaslr.c
@@ -356,6 +356,18 @@ static __init uintptr_t get_random_offset(u64 seed, uintptr_t kernel_size)
return get_legal_offset(random, kernel_size_align);
}

+void __init kaslr_late_init(void)
+{
+ uintptr_t kernel_size;
+
+ /* Clear original kernel image. */
+ if (kaslr_offset) {
+ kernel_size = (uintptr_t) _end - (uintptr_t) _start;

kernel_size = (uintptr_t) _end - (uintptr_t) _start + 1;

OK, change it in the next version. Thanks.


+ memset((void *)PAGE_OFFSET, 0, kernel_size);

I have been thinking again about our discussion regarding PAGE_OFFSET:
PAGE_OFFSET actually points to the address where the kernel was loaded,
not the beginning of memory, that's a bit weird.

Just saying that here, because it took me a few seconds to remember that
and understand what you were doing here.

In non-kaslr case, we load the kernel to PAGE_OFFSET which points to,
so we clear the old kernel image through PAGE_OFFSET here. Certainly,
we could use a symbol to record the start address of the old kernel
image instead of PAGE_OFFSET here. I don't see other architectures
changing PAGE_OFFSET after copying the kernel to the new location in
kaslr. If you think the PAGE_OFFSET needs to be changed, we need to
give another way to make the page table could create the mappings for
the whole memory and memblock/buddy system could see the whole memory
after the kernel moves.
>>
+ set_memory_nx(PAGE_OFFSET, kaslr_offset >> PAGE_SHIFT);

Again, I certainly missed something but when do you use old kernel
mappings ?

We use old kernel mappings when KASLR calculates the random offset, at
that moment, kernel is running on old kernel location.

Yes but haven't you already cleared the page table from the mappings for the old kernel in clear_page_tables called in setup_vm of the new kernel ?

Alex



+ }
+}
+
uintptr_t __init kaslr_early_init(void)
{
u64 seed;
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 34c6ecf2c599..08e2ce170533 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -15,6 +15,7 @@
#include <linux/set_memory.h>
#ifdef CONFIG_RELOCATABLE
#include <linux/elf.h>
+#include <asm/kaslr.h>
#endif

#include <asm/fixmap.h>
@@ -649,6 +650,11 @@ static void __init setup_vm_final(void)
/* Move to swapper page table */
csr_write(CSR_SATP, PFN_DOWN(__pa_symbol(swapper_pg_dir)) | SATP_MODE);
local_flush_tlb_all();
+
+#ifdef CONFIG_RANDOMIZE_BASE
+ /* Clear orignial kernel image and set the right permission. */
+ kaslr_late_init();
+#endif
}

void free_initmem(void)


Alex