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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 8 Sep 2021 10:47:15 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Tiezhu Yang <yangtiezhu@...ngson.cn>,
        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

[ 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. Tiezhu, do you still see any arch that is not on 33
from your testing? 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