linux-kernel - Re: [PATCH] riscv: xip: support runtime trap patching

Open Source and information security mailing list archives

Message-ID: <2fb64dce-779a-c6f2-53b7-088cd38813a4@ghiti.fr>
Date:   Fri, 4 Jun 2021 11:35:17 +0200
From:   Alex Ghiti <alex@...ti.fr>
To:     Vitaly Wool <vitaly.wool@...sulko.com>,
        linux-riscv@...ts.infradead.org
Cc:     linux-kernel@...r.kernel.org,
        Palmer Dabbelt <palmerdabbelt@...gle.com>,
        Nicolas Pitre <nico@...xnic.net>
Subject: Re: [PATCH] riscv: xip: support runtime trap patching

Le 31/05/2021 à 17:17, Alex Ghiti a écrit :
> Hi Vitaly,
> 
> Le 31/05/2021 à 10:53, Vitaly Wool a écrit :
>> RISCV_ERRATA_ALTERNATIVE patches text at runtime which is currently
>> not possible when the kernel is executed from the flash in XIP mode.
>> Since runtime patching concerns only traps at the moment, let's just
>> have all the traps reside in RAM anyway if RISCV_ERRATA_ALTERNATIVE
>> is set. Thus, these functions will be patch-able even when the .text
>> section is in flash.
>>
> 
> This sounds like a good fix for sifive platforms to work with XIP kernel 
> in 5.13: did you test that it actually works on HW?
> 
>> Signed-off-by: Vitaly Wool <vitaly.wool@...sulko.com>
>> ---
>>   arch/riscv/kernel/traps.c           | 13 +++++++++----
>>   arch/riscv/kernel/vmlinux-xip.lds.S | 15 ++++++++++++++-
>>   2 files changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
>> index 0721b9798595..7bc88d8aab97 100644
>> --- a/arch/riscv/kernel/traps.c
>> +++ b/arch/riscv/kernel/traps.c
>> @@ -86,8 +86,13 @@ static void do_trap_error(struct pt_regs *regs, int 
>> signo, int code,
>>       }
>>   }
>> +#if defined (CONFIG_XIP_KERNEL) && defined 
>> (CONFIG_RISCV_ERRATA_ALTERNATIVE)
>> +#define __trap_section        __section(".xip.traps")
>> +#else
>> +#define __trap_section
>> +#endif
> 
> Maybe we could do something more generic. At the moment, only traps are 
> subject to alternatives but that will likely expand: what about rather 
> defining a section called __alternative_section?

Any thoughts about that?

Thanks,

Alex

> 
>>   #define DO_ERROR_INFO(name, signo, code, str)                \
>> -asmlinkage __visible void name(struct pt_regs *regs)            \
>> +asmlinkage __visible __trap_section void name(struct pt_regs *regs)    \
>>   {                                    \
>>       do_trap_error(regs, signo, code, regs->epc, "Oops - " str);    \
>>   }
>> @@ -111,7 +116,7 @@ DO_ERROR_INFO(do_trap_store_misaligned,
>>   int handle_misaligned_load(struct pt_regs *regs);
>>   int handle_misaligned_store(struct pt_regs *regs);
>> -asmlinkage void do_trap_load_misaligned(struct pt_regs *regs)
>> +asmlinkage void __trap_section do_trap_load_misaligned(struct pt_regs 
>> *regs)
>>   {
>>       if (!handle_misaligned_load(regs))
>>           return;
>> @@ -119,7 +124,7 @@ asmlinkage void do_trap_load_misaligned(struct 
>> pt_regs *regs)
>>                 "Oops - load address misaligned");
>>   }
>> -asmlinkage void do_trap_store_misaligned(struct pt_regs *regs)
>> +asmlinkage void __trap_section do_trap_store_misaligned(struct 
>> pt_regs *regs)
>>   {
>>       if (!handle_misaligned_store(regs))
>>           return;
>> @@ -146,7 +151,7 @@ static inline unsigned long 
>> get_break_insn_length(unsigned long pc)
>>       return GET_INSN_LENGTH(insn);
>>   }
>> -asmlinkage __visible void do_trap_break(struct pt_regs *regs)
>> +asmlinkage __visible __trap_section void do_trap_break(struct pt_regs 
>> *regs)
>>   {
>>   #ifdef CONFIG_KPROBES
>>       if (kprobe_single_step_handler(regs))
>> diff --git a/arch/riscv/kernel/vmlinux-xip.lds.S 
>> b/arch/riscv/kernel/vmlinux-xip.lds.S
>> index 4b29b9917f99..a3ff09c4c3f9 100644
>> --- a/arch/riscv/kernel/vmlinux-xip.lds.S
>> +++ b/arch/riscv/kernel/vmlinux-xip.lds.S
>> @@ -99,9 +99,22 @@ SECTIONS
>>       }
>>       PERCPU_SECTION(L1_CACHE_BYTES)
>> -    . = ALIGN(PAGE_SIZE);
>> +    . = ALIGN(8);
>> +    .alternative : {
>> +        __alt_start = .;
>> +        *(.alternative)
>> +        __alt_end = .;
>> +    }
>>       __init_end = .;
>> +    . = ALIGN(16);
> 
> Why 16 here?
> 
>> +    .xip.traps : {
>> +        __xip_traps_start = .;
>> +        *(.xip.traps)
>> +        __xip_traps_end = .;
>> +    }
>> +
>> +    . = ALIGN(PAGE_SIZE);
>>       .sdata : {
>>           __global_pointer$ = . + 0x800;
>>           *(.sdata*)
>>
> 
> Thanks,
> 
> Alex
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives