| 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
| ||
|
Message-ID: <b74fc5436d0b21ccd72917cf94b8bbc7eb9e5179.camel@linux.ibm.com>
Date: Thu, 04 Nov 2021 14:36:22 +0100
From: Ilya Leoshkevich <iii@...ux.ibm.com>
To: Tiezhu Yang <yangtiezhu@...ngson.cn>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>
Cc: netdev@...r.kernel.org, bpf@...r.kernel.org,
linux-kernel@...r.kernel.org, Xuefeng Li <lixuefeng@...ngson.cn>,
Johan Almbladh <johan.almbladh@...finetworks.com>,
Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>, illusionist.neo@...il.com,
zlim.lnx@...il.com, naveen.n.rao@...ux.ibm.com,
luke.r.nels@...il.com, xi.wang@...il.com, bjorn@...nel.org,
hca@...ux.ibm.com, gor@...ux.ibm.com, udknight@...il.com,
davem@...emloft.net
Subject: Re: [PATCH bpf-next v5] bpf: Change value of MAX_TAIL_CALL_CNT from
32 to 33
On Thu, 2021-11-04 at 09:50 +0800, Tiezhu Yang wrote:
> In the current code, the actual max tail call count is 33 which is
> greater
> than MAX_TAIL_CALL_CNT (defined as 32), the actual limit is not
> consistent
> with the meaning of MAX_TAIL_CALL_CNT, there is some confusion and
> need to
> spend some time to think about the reason at the first glance.
>
> We can see the historical evolution from commit 04fd61ab36ec ("bpf:
> allow
> bpf programs to tail-call other bpf programs") and commit
> f9dabe016b63
> ("bpf: Undo off-by-one in interpreter tail call count limit").
>
> In order to avoid changing existing behavior, the actual limit is 33
> now,
> this is reasonable.
>
> 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
>
> Although the above failed testcase has been fixed in commit
> 18935a72eb25
> ("bpf/tests: Fix error in tail call limit tests"), it is still
> necessary
> to change the value of MAX_TAIL_CALL_CNT from 32 to 33 to make the
> code
> more readable, then do some small changes of the related code.
>
> With this patch, it does not change the current limit 33,
> MAX_TAIL_CALL_CNT
> can reflect the actual max tail call count, the related tailcall
> testcases
> in test_bpf and selftests can work well for the interpreter and the
> JIT.
>
> Here are the test results on x86_64:
>
> # uname -m
> x86_64
> # echo 0 > /proc/sys/net/core/bpf_jit_enable
> # modprobe test_bpf test_suite=test_tail_calls
> # dmesg | tail -1
> test_bpf: test_tail_calls: Summary: 8 PASSED, 0 FAILED, [0/8 JIT'ed]
> # rmmod test_bpf
> # echo 1 > /proc/sys/net/core/bpf_jit_enable
> # modprobe test_bpf test_suite=test_tail_calls
> # dmesg | tail -1
> test_bpf: test_tail_calls: Summary: 8 PASSED, 0 FAILED, [8/8 JIT'ed]
> # rmmod test_bpf
> # ./test_progs -t tailcalls
> #142 tailcalls:OK
> Summary: 1/11 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Tiezhu Yang <yangtiezhu@...ngson.cn>
> Acked-by: Björn Töpel <bjorn@...nel.org>
> ---
>
> v5: Use RV_REG_TCC directly to save one move for RISC-V,
> suggested by Björn Töpel, thank you.
>
> v4: Use >= as check condition,
> suggested by Daniel Borkmann, thank you.
>
> arch/arm/net/bpf_jit_32.c | 5 +++--
> arch/arm64/net/bpf_jit_comp.c | 5 +++--
> arch/mips/net/bpf_jit_comp32.c | 2 +-
> arch/mips/net/bpf_jit_comp64.c | 2 +-
> arch/powerpc/net/bpf_jit_comp32.c | 4 ++--
> arch/powerpc/net/bpf_jit_comp64.c | 4 ++--
> arch/riscv/net/bpf_jit_comp32.c | 6 ++----
> arch/riscv/net/bpf_jit_comp64.c | 7 +++----
> arch/s390/net/bpf_jit_comp.c | 6 +++---
> arch/sparc/net/bpf_jit_comp_64.c | 2 +-
> arch/x86/net/bpf_jit_comp.c | 10 +++++-----
> arch/x86/net/bpf_jit_comp32.c | 4 ++--
> include/linux/bpf.h | 2 +-
> include/uapi/linux/bpf.h | 2 +-
> kernel/bpf/core.c | 3 ++-
> lib/test_bpf.c | 4 ++--
> tools/include/uapi/linux/bpf.h | 2 +-
> 17 files changed, 35 insertions(+), 35 deletions(-)
[...]
> diff --git a/arch/s390/net/bpf_jit_comp.c
> b/arch/s390/net/bpf_jit_comp.c
> index 1a374d0..3553cfc 100644
> --- a/arch/s390/net/bpf_jit_comp.c
> +++ b/arch/s390/net/bpf_jit_comp.c
> @@ -1369,7 +1369,7 @@ static noinline int bpf_jit_insn(struct bpf_jit
> *jit, struct bpf_prog *fp,
> jit->prg);
>
> /*
> - * if (tail_call_cnt++ > MAX_TAIL_CALL_CNT)
> + * if (tail_call_cnt++ >= MAX_TAIL_CALL_CNT)
> * goto out;
> */
>
> @@ -1381,9 +1381,9 @@ static noinline int bpf_jit_insn(struct bpf_jit
> *jit, struct bpf_prog *fp,
> EMIT4_IMM(0xa7080000, REG_W0, 1);
> /* laal %w1,%w0,off(%r15) */
> EMIT6_DISP_LH(0xeb000000, 0x00fa, REG_W1, REG_W0,
> REG_15, off);
> - /* clij %w1,MAX_TAIL_CALL_CNT,0x2,out */
> + /* clij %w1,MAX_TAIL_CALL_CNT-1,0x2,out */
> patch_2_clij = jit->prg;
> - EMIT6_PCREL_RIEC(0xec000000, 0x007f, REG_W1,
> MAX_TAIL_CALL_CNT,
> + EMIT6_PCREL_RIEC(0xec000000, 0x007f, REG_W1,
> MAX_TAIL_CALL_CNT - 1,
> 2, jit->prg);
>
> /*
For the s390 part:
Tested-by: Ilya Leoshkevich <iii@...ux.ibm.com>
Acked-by: Ilya Leoshkevich <iii@...ux.ibm.com>
[...]
Powered by blists - more mailing lists