[RFC PATCH 0/3] enable bpf_prog_pack allocator for powerpc

[Christophe Leroy]

Le 18/11/2022 à 18:28, Song Liu a écrit :
> On Fri, Nov 18, 2022 at 3:47 AM Christophe Leroy
> <christophe.leroy at csgroup.eu> wrote:
>>
>>
>>
>> Le 18/11/2022 à 10:39, Hari Bathini a écrit :
>>>
>>>
>>> On 18/11/22 2:21 pm, Christophe Leroy wrote: >>>>>
>>>>>>> I had the same config but hit this problem:
>>>>>>>
>>>>>>>         # echo 1 > /proc/sys/net/core/bpf_jit_enable; modprobe test_bpf
>>>>>>>         test_bpf: #0 TAX
>>>>>>>         ------------[ cut here ]------------
>>>>>>>         WARNING: CPU: 0 PID: 96 at arch/powerpc/net/bpf_jit_comp.c:367
>>>>>>> bpf_int_jit_compile+0x8a0/0x9f8
>>>>>>
>>>>>> I get no such problem, on QEMU, and I checked the .config has:
>>>>>
>>>>>> CONFIG_STRICT_KERNEL_RWX=y
>>>>>> CONFIG_STRICT_MODULE_RWX=y
>>>>>
>>>>> Yeah. That did the trick.
>>>>
>>>> Interesting. I guess we have to find out why it fails when those config
>>>> are missing.
>>>>
>>>> Maybe module code plays with RO and NX flags even if
>>>> CONFIG_STRICT_MODULE_RWX is not selected ?
>>>
>>> Need to look at the code closely but fwiw, observing same failure on
>>> 64-bit as well with !STRICT_RWX...
>>
>> The problem is in bpf_prog_pack_alloc() and in alloc_new_pack() : They
>> do set_memory_ro() and set_memory_x() without taking into account
>> CONFIG_STRICT_MODULE_RWX.
>>
>> When CONFIG_STRICT_MODULE_RWX is selected, powerpc module_alloc()
>> allocates PAGE_KERNEL memory, that is RW memory, and expects the user to
>> call do set_memory_ro() and set_memory_x().
>>
>> But when CONFIG_STRICT_MODULE_RWX is not selected, powerpc
>> module_alloc() allocates PAGE_KERNEL_TEXT memory, that is RWX memory,
>> and expects to be able to always write into it.
> 
> Ah, I see. x86_64 requires CONFIG_STRICT_MODULE_RWX, so this hasn't
> been a problem yet.
> 

In fact it shouldn't be a problem for BPF on powerpc either. Because 
powerpc BPF expects RO at all time and today uses bpf_jit_binary_lock_ro().

It just means that we can't use patch_instruction() for that. Anyway, 
using patch_instruction() was sub-optimal.

All we have to do I think is set a mirror of the page using vmap() then 
perform a memcpy() of the code then vunmap() it. Maybe a call to 
flush_tlb_kernel_range() will be also needed, unless BPF already does it.

Christophe