netdev - Re: [PATCH bpf] bpf, arm64: calculate offset as byte-offset for bpf line info

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] [day] [month] [year] [list]

Date:   Wed, 19 Jan 2022 22:45:37 +0800
From:   Hou Tao <houtao1@...wei.com>
To:     Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>
CC:     Martin KaFai Lau <kafai@...com>, Yonghong Song <yhs@...com>,
        Andrii Nakryiko <andrii@...nel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, <netdev@...r.kernel.org>,
        <bpf@...r.kernel.org>, Zi Shen Lim <zlim.lnx@...il.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH bpf] bpf, arm64: calculate offset as byte-offset for bpf
 line info

Hi,

On 1/7/2022 6:00 AM, Daniel Borkmann wrote:
> On 1/4/22 2:42 AM, Hou Tao wrote:
>> The bpf line info for arm64 is broken due to two reasons:
>> (1) insn_to_jit_off passed to bpf_prog_fill_jited_linfo() is
>>      calculated in instruction granularity instead of bytes
>>      granularity.
>> (2) insn_to_jit_off only considers the body itself and ignores
>>      prologue before the body.
>>
>> So fix it by calculating offset as byte-offset and do build_prologue()
>> first in the first JIT pass.
>>
[snip]
>> -    /* Fake pass to fill in ctx->offset. */
>> -    if (build_body(&ctx, extra_pass)) {
>> +    /*
>> +     * 1. Initial fake pass to compute ctx->idx and ctx->offset.
>> +     *
>> +     * BPF line info needs ctx->offset[i] to be the byte offset
>> +     * of instruction[i] in jited image, so build prologue first.
>> +     */
>> +    if (build_prologue(&ctx, was_classic)) {
>>           prog = orig_prog;
>>           goto out_off;
>>       }
>>   -    if (build_prologue(&ctx, was_classic)) {
>> +    if (build_body(&ctx, extra_pass)) {
>>           prog = orig_prog;
>>           goto out_off;
>
> Could you split this into two logical patches? Both 1/2 seem independent
> of each other and should have been rather 2 patches instead of 1.
>
Sorry for the later reply. Splitting into two patches make sense for me. Will do
it in v2.
> Did you check if also other JITs could be affected?
It seems sparc also doesn't represent offset by bytes and I can check other
arches as well,
but it is sad that I don't have the environments for these arches.

> Thanks,
> Daniel
> .