linux-kernel - Re: [PATCH] powerpc/bpf: Only update ldimm64 during extra pass when it is an address

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 24 Nov 2022 19:46:08 +0000
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     "Naveen N. Rao" <naveen.n.rao@...ux.ibm.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        Nicholas Piggin <npiggin@...il.com>
CC:     Andrii Nakryiko <andrii@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Hao Luo <haoluo@...gle.com>,
        John Fastabend <john.fastabend@...il.com>,
        Jiri Olsa <jolsa@...nel.org>, KP Singh <kpsingh@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Stanislav Fomichev <sdf@...gle.com>,
        Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>
Subject: Re: [PATCH] powerpc/bpf: Only update ldimm64 during extra pass when
 it is an address



Le 24/11/2022 à 14:49, Naveen N. Rao a écrit :
> Christophe Leroy wrote:
>>
>>
>> Le 24/11/2022 à 11:13, Naveen N. Rao a écrit :
>>> Christophe Leroy wrote:
>>>> ldimm64 is not only used for loading function addresses, and
>>>
>>> That's probably true today, but I worry that that can change upstream 
>>> and we may not notice at all.
>>
>> Not sure what you mean.
>>
>> Today POWERPC considers that ldimm64 is _always_ loading a function 
>> address whereas upstream BPF considers that ldimm64 is a function only 
>> when it is flagged BPF_PSEUDO_FUNC.
> 
> Not sure why you think we consider ldimm64 to always be loading a 
> function address. Perhaps it is due to the poorly chosen variable name 
> func_addr in bpf_jit_fixup_addresses(), or due to the fact that we 
> always update the JIT code for ldimm64. In any case, we simply overwrite 
> imm64 load instructions to ensure we are using the updated address.

Well that's the padding which make me think that. When ldimm64 is used 
with immediate value, it won't change from one pass to the other. We 
have the need for the padding only because it may contain addresses that 
will change from one pass to another.

> 
>>
>> In what direction could that change in the future ?
>>
>> For me if they change that it becomes an API change.
> 
> More of an extension, which is exactly what we had when BPF_PSEUDO_FUNC 
> was introduced. Took us nearly a year before we noticed.
> 
> Because we do not do a full JIT during the extra pass today like other 
> architectures, we are the exception - there is always the risk of bpf 
> core changes breaking our JIT. So, I still think it is better if we do a 
> full JIT during extra pass.
> 

I like the idea of a full JIT during extra passes and will start looking 
at it.

Will it also allow us to revert your commit fab07611fb2e 
("powerpc32/bpf: Fix codegen for bpf-to-bpf calls") ?

Thanks
Christophe