netdev - Re: [RFC PATCH bpf-next] bpf: Make actual max tail call count as MAX_TAIL_CALL

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]

Message-ID: <9d0ca1ba-b8e1-dc99-17f4-189571f33c97@loongson.cn>
Date:   Wed, 8 Sep 2021 18:56:28 +0800
From:   Tiezhu Yang <yangtiezhu@...ngson.cn>
To:     Daniel Borkmann <daniel@...earbox.net>,
        Shubham Bansal <illusionist.neo@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Alexei Starovoitov <ast@...nel.org>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Zi Shen Lim <zlim.lnx@...il.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Paul Burton <paulburton@...nel.org>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        naveen.n.rao@...ux.ibm.com, Michael Ellerman <mpe@...erman.id.au>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Luke Nelson <luke.r.nels@...il.com>,
        Xi Wang <xi.wang@...il.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>, bjorn@...nel.org,
        davem@...emloft.net
Cc:     netdev@...r.kernel.org, bpf@...r.kernel.org,
        Paul Chaignon <paul@...ium.io>,
        Johan Almbladh <johan.almbladh@...finetworks.com>
Subject: Re: [RFC PATCH bpf-next] bpf: Make actual max tail call count as
 MAX_TAIL_CALL_CNT

On 09/08/2021 04:47 PM, Daniel Borkmann wrote:
> [ You have a huge Cc list, but forgot to add Paul and Johan who recently
>   looked into this. Added here. ]
>
> On 9/8/21 10:20 AM, Tiezhu Yang wrote:
>> In the current code, the actual max tail call count is 33 which is 
>> greater
>> than MAX_TAIL_CALL_CNT, this is not consistent with the intended meaning
>> in the commit 04fd61ab36ec ("bpf: allow bpf programs to tail-call other
>> bpf programs"):
>>
>> "The chain of tail calls can form unpredictable dynamic loops therefore
>> tail_call_cnt is used to limit the number of calls and currently is set
>> to 32."
>>
>> Additionally, after commit 874be05f525e ("bpf, tests: Add tail call test
>> suite"), we can see there exists failed testcase.
>>
>> On all archs when CONFIG_BPF_JIT_ALWAYS_ON is not set:
>>   # echo 0 > /proc/sys/net/core/bpf_jit_enable
>>   # modprobe test_bpf
>>   # dmesg | grep -w FAIL
>>   Tail call error path, max count reached jited:0 ret 34 != 33 FAIL
>>
>> On some archs:
>>   # echo 1 > /proc/sys/net/core/bpf_jit_enable
>>   # modprobe test_bpf
>>   # dmesg | grep -w FAIL
>>   Tail call error path, max count reached jited:1 ret 34 != 33 FAIL
>>
>> with this patch, make the actual max tail call count as 
>> MAX_TAIL_CALL_CNT,
>> at the same time, the above failed testcase can be fixed.
>>
>> Signed-off-by: Tiezhu Yang <yangtiezhu@...ngson.cn>
>> ---
>>
>> Hi all,
>>
>> This is a RFC patch, if I am wrong or I missed something,
>> please let me know, thank you!
>
> Yes, the original commit from 04fd61ab36ec ("bpf: allow bpf programs 
> to tail-call
> other bpf programs") got the counting wrong, but please also check 
> f9dabe016b63
> ("bpf: Undo off-by-one in interpreter tail call count limit") where we 
> agreed to
> align everything to 33 in order to avoid changing existing behavior, 
> and if we
> intend to ever change the count, then only in terms of increasing but 
> not decreasing
> since that ship has sailed.

Thank you, understood.

But I still think there is some confusion about the macro MAX_TAIL_CALL_CNT
which is 32 and the actual value 33, I spent some time to understand it
at the first glance.

Is it impossible to keep the actual max tail call count consistent with
the value 32 of MAX_TAIL_CALL_CNT now?

At least, maybe we need to modify the testcase?

> Tiezhu, do you still see any arch that is not on 33
> from your testing?

If the testcase "Tail call error path, max count reached" in test_bpf is 
right,
it seems that the tail call count limit is 32 on x86, because the testcase
passed on x86 jited.

> Last time Paul fixed the remaining ones in 96bc4432f5ad ("bpf,
> riscv: Limit to 33 tail calls") and e49e6f6db04e ("bpf, mips: Limit to 
> 33 tail calls").
>
> Thanks,
> Daniel