Re: [PATCH] riscv/kprobes: allocate detour buffer from module area

[liaochang (A)]

在 2022/8/12 1:09, Palmer Dabbelt 写道:
> On Tue, 28 Jun 2022 18:13:17 PDT (-0700), liaochang1@xxxxxxxxxx wrote:
>> To address the limitation of PC-relative branch instruction
>> on riscv architecture, detour buffer slot is allocated from
>> a area, the distance of which from kernel should be less than 4GB.
>>
>> For the time being, Modules region always live before the kernel.
>> But Vmalloc region reside far away from kernel, the distance is
>> half of the kernel address space.
>>
>> Signed-off-by: Liao Chang <liaochang1@xxxxxxxxxx>
>> ---
>>  arch/riscv/kernel/probes/kprobes.c | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c
>> index e6e950b7cf32..bc027a663b17 100644
>> --- a/arch/riscv/kernel/probes/kprobes.c
>> +++ b/arch/riscv/kernel/probes/kprobes.c
>> @@ -6,12 +6,14 @@
>>  #include <linux/extable.h>
>>  #include <linux/slab.h>
>>  #include <linux/stop_machine.h>
>> +#include <linux/moduleloader.h>
>>  #include <asm/ptrace.h>
>>  #include <linux/uaccess.h>
>>  #include <asm/sections.h>
>>  #include <asm/cacheflush.h>
>>  #include <asm/bug.h>
>>  #include <asm/patch.h>
>> +#include <asm/set_memory.h>
>>
>>  #include "decode-insn.h"
>>
>> @@ -86,10 +88,28 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
>>  #ifdef CONFIG_MMU
>>  void *alloc_insn_page(void)
>>  {
>> +#if defined(CONFIG_MODULES) && defined(CONFIG_64BIT)
>> +    void *page;
>> +
>> +    page = module_alloc(PAGE_SIZE);
>> +    if (!page)
>> +        return NULL;
>> +
>> +    set_vm_flush_reset_perms(page);
>> +    /*
>> +     * First make the page read-only, and only then make it executable to
>> +     * prevent it from being W+X in between.
>> +     */
>> +    set_memory_ro((unsigned long)page, 1);
>> +    set_memory_x((unsigned long)page, 1);
>> +
>> +    return page;
>> +#else
>>      return  __vmalloc_node_range(PAGE_SIZE, 1, VMALLOC_START, VMALLOC_END,
>>                       GFP_KERNEL, PAGE_KERNEL_READ_EXEC,
>>                       VM_FLUSH_RESET_PERMS, NUMA_NO_NODE,
>>                       __builtin_return_address(0));
>> +#endif
>>  }
>>  #endif
> 
> This probably shouldn't depend on CONFIG_MODULES, it's just as broken to assume we can jump to the vmalloc region regardless of whether modules are enabled or disabled.  We should be able to just allocate from the jump-safe region either way.
> .

Yes, there is no need to invoke module_alloc to allocate buffer from jump-safe region,
just use __vmalloc_node_range directly, thanks for reminding me.

-- 
BR,
Liao, Chang