lkml.org 
[lkml]   [2020]   [Apr]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH RFC 6/8] riscv/kaslr: clear the original kernel image
From
Date
On 4/9/20 1:53 AM, Alex Ghiti wrote:
>
>
> On 4/7/20 7:18 AM, Zong Li wrote:
>> On Tue, Apr 7, 2020 at 1:11 PM Alex Ghiti <alex@ghiti.fr> 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@sifive.com>
>>>> ---
>>>>    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@sifive.com>
>>>> + */
>>>> +
>>>> +#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
>

I had a doubt so I read set_memory_nx implementation again and I was
indeed completely wrong: set_memory_nx tackles init_mm and then
swapper_pg_dir. So you just remove executability for the old kernel
zone, that's ok I think.

Sorry for the noise !

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

\
 
 \ /
  Last update: 2020-04-09 10:14    [W:0.073 / U:2.124 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site