From patchwork Thu Jan 16 05:51:29 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yonghong Song X-Patchwork-Id: 13941302 X-Patchwork-Delegate: bpf@iogearbox.net Received: from 69-171-232-181.mail-mxout.facebook.com (69-171-232-181.mail-mxout.facebook.com [69.171.232.181]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0579B149C7B for ; Thu, 16 Jan 2025 05:51:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=69.171.232.181 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737006702; cv=none; b=IlH8TGaebVJlZmpgslQI3ZyuRCmitFHxOXqgiHDt/ERiF3I/4i/VT8g8CClpUtc2z00BRKFgxLGTaH4qe+V2PoWjiJLsz3wgRY1xDkAG3tk6aX3+ktWIjrMoRZuh3aRXsfTkr8Gc+mziJ+Ik3yHh4o4yjiVbxq8cgk3wrU3VIrY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737006702; c=relaxed/simple; bh=fB+dTtDTMiWL56JUQCUrU+gGWBJW//uqaARQD0IzHWQ=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=dn3m5Rkzg3VVaIgQ8SPNVAA/VOUmdvfMKTrTE7oktF3L5pQVJDLpy9YOOfnCn/vLZfj898Tn9xn/ZUKaxzaHLo2IdiHId9oNpnXTryayfRWtXsXFR19abn+9krX//pSa/sL9+IRm5aQ+u19ZBLINyWStDsJ8hS032ZOG1X1+8b4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.dev; spf=fail smtp.mailfrom=linux.dev; arc=none smtp.client-ip=69.171.232.181 Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=fail smtp.mailfrom=linux.dev Received: by devbig309.ftw3.facebook.com (Postfix, from userid 128203) id 1E3C2D032440; Wed, 15 Jan 2025 21:51:29 -0800 (PST) From: Yonghong Song To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , kernel-team@fb.com, Martin KaFai Lau , Emil Tsalapatis Subject: [PATCH bpf-next 1/3] bpf: Allow 'may_goto 0' instruction Date: Wed, 15 Jan 2025 21:51:29 -0800 Message-ID: <20250116055129.604354-1-yonghong.song@linux.dev> X-Mailer: git-send-email 2.43.5 In-Reply-To: <20250116055123.603790-1-yonghong.song@linux.dev> References: <20250116055123.603790-1-yonghong.song@linux.dev> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net Commit 011832b97b31 ("bpf: Introduce may_goto instruction") added support for may_goto insn. The 'may_goto 0' insn is disallowed since the insn is equivalent to a nop as both branch will go to the next insn. But it is possible that compiler transformation may generate 'may_goto 0' insn. Emil Tsalapatis from Meta reported such a case which caused verification failure. For example, for the following code, int i, tmp[3]; for (i = 0; i < 3 && can_loop; i++) tmp[i] = 0; ... clang 20 may generate code like may_goto 2; may_goto 1; may_goto 0; r1 = 0; /* tmp[0] = 0; */ r2 = 0; /* tmp[1] = 0; */ r3 = 0; /* tmp[2] = 0; */ Let us permit 'may_goto 0' insn to avoid verification failure for codes like the above. Reported-by: Emil Tsalapatis Signed-off-by: Yonghong Song --- kernel/bpf/verifier.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index b8ca227c78af..edf3cc42a220 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -15899,9 +15899,8 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, if (insn->code != (BPF_JMP | BPF_JCOND) || insn->src_reg != BPF_MAY_GOTO || - insn->dst_reg || insn->imm || insn->off == 0) { - verbose(env, "invalid may_goto off %d imm %d\n", - insn->off, insn->imm); + insn->dst_reg || insn->imm) { + verbose(env, "invalid may_goto imm %d\n", insn->imm); return -EINVAL; } prev_st = find_prev_entry(env, cur_st->parent, idx); From patchwork Thu Jan 16 05:51:34 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yonghong Song X-Patchwork-Id: 13941303 X-Patchwork-Delegate: bpf@iogearbox.net Received: from 66-220-155-178.mail-mxout.facebook.com (66-220-155-178.mail-mxout.facebook.com [66.220.155.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E29D3141987 for ; Thu, 16 Jan 2025 05:51:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=66.220.155.178 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737006709; cv=none; b=tUEGKbHaF/BWxk9UoWgaCS2hJtBmn/BpIDY/p5oMbJD0pRVW0OBOBb9OrtZyE7waibGGkgQU+0zVOmBGfYFyEjioyPUopAfIrQzBwLnx29h0tKxHsfreR57QHL2GDl+QkANL/A/SrXCJZpdc1c+3Kxzs9P3+U/6ZDQbDiHvopi0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737006709; c=relaxed/simple; bh=M0MRPC4InUgpTCkcLNGTZh51OD1kaASqNwm7rdAmrAs=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=tsD8hrJp3+p/PSZeTV7UQ029qHEGUhXm5ogSQiqjIr87lndYzwp2D1lgNiMshRgrrIPuv9wLHjyXR1++2Vp+jYA9a+RHOyP64jQOOlfJS9PyWEPNiI2Y6mN3X8ujwtLSW/hzOmTBWWEh6eqp8K9Vl30vpI8sloJEYBZeXAqvnWY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.dev; spf=fail smtp.mailfrom=linux.dev; arc=none smtp.client-ip=66.220.155.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=fail smtp.mailfrom=linux.dev Received: by devbig309.ftw3.facebook.com (Postfix, from userid 128203) id 394F3D032467; Wed, 15 Jan 2025 21:51:34 -0800 (PST) From: Yonghong Song To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , kernel-team@fb.com, Martin KaFai Lau Subject: [PATCH bpf-next 2/3] bpf: Remove 'may_goto 0' instruction Date: Wed, 15 Jan 2025 21:51:34 -0800 Message-ID: <20250116055134.604867-1-yonghong.song@linux.dev> X-Mailer: git-send-email 2.43.5 In-Reply-To: <20250116055123.603790-1-yonghong.song@linux.dev> References: <20250116055123.603790-1-yonghong.song@linux.dev> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net Since 'may_goto 0' insns are actually no-op, let us remove them. Otherwise, verifier will generate code like /* r10 - 8 stores the implicit loop count */ r11 = *(u64 *)(r10 -8) if r11 == 0x0 goto pc+2 r11 -= 1 *(u64 *)(r10 -8) = r11 which is the pure overhead. The following code patterns (from the previous commit) are also handled: may_goto 2 may_goto 1 may_goto 0 With this commit, the above three 'may_goto' insns are all eliminated. Signed-off-by: Yonghong Song --- kernel/bpf/verifier.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index edf3cc42a220..72b474bfba2d 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -20133,6 +20133,40 @@ static int opt_remove_nops(struct bpf_verifier_env *env) return 0; } +static int opt_remove_useless_may_gotos(struct bpf_verifier_env *env) +{ + struct bpf_insn *insn = env->prog->insnsi; + int i, j, err, last_may_goto, removed_cnt; + int insn_cnt = env->prog->len; + + for (i = 0; i < insn_cnt; i++) { + if (!is_may_goto_insn(&insn[i])) + continue; + + for (j = i + 1; j < insn_cnt; j++) { + if (!is_may_goto_insn(&insn[j])) + break; + } + + last_may_goto = --j; + removed_cnt = 0; + while (j >= i) { + if (insn[j].off == 0) { + err = verifier_remove_insns(env, j, 1); + if (err) + return err; + removed_cnt++; + } + j--; + } + + insn_cnt -= removed_cnt; + i = last_may_goto - removed_cnt; + } + + return 0; +} + static int opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env, const union bpf_attr *attr) { @@ -23089,6 +23123,8 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3 ret = opt_remove_dead_code(env); if (ret == 0) ret = opt_remove_nops(env); + if (ret == 0) + ret = opt_remove_useless_may_gotos(env); } else { if (ret == 0) sanitize_dead_code(env); From patchwork Thu Jan 16 05:51:39 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yonghong Song X-Patchwork-Id: 13941304 X-Patchwork-Delegate: bpf@iogearbox.net Received: from 66-220-155-178.mail-mxout.facebook.com (66-220-155-178.mail-mxout.facebook.com [66.220.155.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7516C18A943 for ; Thu, 16 Jan 2025 05:51:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=66.220.155.178 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737006714; cv=none; b=JiMmrMPOyNGm3gCb/FIj6MrYk18nn0a7dCJFSYurnNLBueccDcRVZ3rGLsxB2q0eCfj1X+fv9Rz73FxvN3VFlc+4jTcB/PBw9W2x8os2YAhEzi3sa+/78E5A/W2RwVfwmHAJWAeLfdKxbDr/1eQagkKu77bP7Oh2mYw87AbE6Yk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737006714; c=relaxed/simple; bh=T07ArxD+pvnzcrwkB5L/sl+WXQqXdxRo/8QQnFS7Vqs=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=oJolXX1z/49MFaQK2dnW7TP59ULprvgrnCulUiPgsrYtRqZ5yY5AbfWiN9r1nspvp2Pb19HLK/N3L7yk1F31qUl9f/QCwHw40DE/V+ltID+IjdsxLmpFyWEvjjP3768xBZAbcaRYvB3QRE3zDGnZTBiIx2/HpGH/ASP34GQ5HB0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.dev; spf=fail smtp.mailfrom=linux.dev; arc=none smtp.client-ip=66.220.155.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=fail smtp.mailfrom=linux.dev Received: by devbig309.ftw3.facebook.com (Postfix, from userid 128203) id 5620CD032494; Wed, 15 Jan 2025 21:51:39 -0800 (PST) From: Yonghong Song To: bpf@vger.kernel.org Cc: Alexei Starovoitov , Andrii Nakryiko , Daniel Borkmann , kernel-team@fb.com, Martin KaFai Lau Subject: [PATCH bpf-next 3/3] selftests/bpf: Add some tests related to 'may_goto 0' insns Date: Wed, 15 Jan 2025 21:51:39 -0800 Message-ID: <20250116055139.605195-1-yonghong.song@linux.dev> X-Mailer: git-send-email 2.43.5 In-Reply-To: <20250116055123.603790-1-yonghong.song@linux.dev> References: <20250116055123.603790-1-yonghong.song@linux.dev> Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: bpf@iogearbox.net Add both asm-based and C-based tests which have 'may_goto 0' insns. For the following code in C-based test, int i, tmp[3]; for (i = 0; i < 3 && can_loop; i++) tmp[i] = 0; The clang compiler (clang 19 and 20) generates may_goto 2 may_goto 1 may_goto 0 r1 = 0 r2 = 0 r3 = 0 The above asm codes are due to llvm pass SROAPass. This ensures the successful verification since tmp[0-2] are initialized. Otherwise, the code without SROAPass like may_goto 5 r1 = 0 may_goto 3 r2 = 0 may_goto 1 r3 = 0 will have verification failure. Although from the source code C-based test should have verification failure, clang compiler optimization generates code with successful verification. If gcc generates different asm codes than clang, the following code can be used for gcc: int i, tmp[3]; for (i = 0; i < 3; i++) tmp[i] = 0; Signed-off-by: Yonghong Song --- .../selftests/bpf/prog_tests/verifier.c | 4 + .../selftests/bpf/progs/verifier_may_goto_1.c | 97 +++++++++++++++++++ .../selftests/bpf/progs/verifier_may_goto_2.c | 28 ++++++ 3 files changed, 129 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/verifier_may_goto_1.c create mode 100644 tools/testing/selftests/bpf/progs/verifier_may_goto_2.c diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c index 33cd3e035071..8a0e1ff8a2dc 100644 --- a/tools/testing/selftests/bpf/prog_tests/verifier.c +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c @@ -52,6 +52,8 @@ #include "verifier_map_ptr_mixing.skel.h" #include "verifier_map_ret_val.skel.h" #include "verifier_masking.skel.h" +#include "verifier_may_goto_1.skel.h" +#include "verifier_may_goto_2.skel.h" #include "verifier_meta_access.skel.h" #include "verifier_movsx.skel.h" #include "verifier_mtu.skel.h" @@ -182,6 +184,8 @@ void test_verifier_map_ptr(void) { RUN(verifier_map_ptr); } void test_verifier_map_ptr_mixing(void) { RUN(verifier_map_ptr_mixing); } void test_verifier_map_ret_val(void) { RUN(verifier_map_ret_val); } void test_verifier_masking(void) { RUN(verifier_masking); } +void test_verifier_may_goto_1(void) { RUN(verifier_may_goto_1); } +void test_verifier_may_goto_2(void) { RUN(verifier_may_goto_2); } void test_verifier_meta_access(void) { RUN(verifier_meta_access); } void test_verifier_movsx(void) { RUN(verifier_movsx); } void test_verifier_netfilter_ctx(void) { RUN(verifier_netfilter_ctx); } diff --git a/tools/testing/selftests/bpf/progs/verifier_may_goto_1.c b/tools/testing/selftests/bpf/progs/verifier_may_goto_1.c new file mode 100644 index 000000000000..e81097c96fe2 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/verifier_may_goto_1.c @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */ + +#include +#include +#include "../../../include/linux/filter.h" +#include "bpf_misc.h" + +SEC("raw_tp") +__description("may_goto 0") +__arch_x86_64 +__xlated("0: r0 = 1") +__xlated("1: exit") +__success +__naked void may_goto_simple(void) +{ + asm volatile ( + ".8byte %[may_goto];" + "r0 = 1;" + ".8byte %[may_goto];" + "exit;" + : + : __imm_insn(may_goto, BPF_RAW_INSN(BPF_JMP | BPF_JCOND, 0, 0, 0 /* offset */, 0)) + : __clobber_all); +} + +SEC("raw_tp") +__description("batch 2 of may_goto 0") +__arch_x86_64 +__xlated("0: r0 = 1") +__xlated("1: exit") +__success +__naked void may_goto_batch_0(void) +{ + asm volatile ( + ".8byte %[may_goto1];" + ".8byte %[may_goto1];" + "r0 = 1;" + ".8byte %[may_goto1];" + ".8byte %[may_goto1];" + "exit;" + : + : __imm_insn(may_goto1, BPF_RAW_INSN(BPF_JMP | BPF_JCOND, 0, 0, 0 /* offset */, 0)) + : __clobber_all); +} + +SEC("raw_tp") +__description("may_goto batch with offsets 2/1/0") +__arch_x86_64 +__xlated("0: r0 = 1") +__xlated("1: exit") +__success +__naked void may_goto_batch_1(void) +{ + asm volatile ( + ".8byte %[may_goto1];" + ".8byte %[may_goto2];" + ".8byte %[may_goto3];" + "r0 = 1;" + ".8byte %[may_goto1];" + ".8byte %[may_goto2];" + ".8byte %[may_goto3];" + "exit;" + : + : __imm_insn(may_goto1, BPF_RAW_INSN(BPF_JMP | BPF_JCOND, 0, 0, 2 /* offset */, 0)), + __imm_insn(may_goto2, BPF_RAW_INSN(BPF_JMP | BPF_JCOND, 0, 0, 1 /* offset */, 0)), + __imm_insn(may_goto3, BPF_RAW_INSN(BPF_JMP | BPF_JCOND, 0, 0, 0 /* offset */, 0)) + : __clobber_all); +} + +SEC("raw_tp") +__description("may_goto batch with offsets 2/0") +__arch_x86_64 +__xlated("0: *(u64 *)(r10 -8) = 8388608") +__xlated("1: r11 = *(u64 *)(r10 -8)") +__xlated("2: if r11 == 0x0 goto pc+3") +__xlated("3: r11 -= 1") +__xlated("4: *(u64 *)(r10 -8) = r11") +__xlated("5: r0 = 1") +__xlated("6: r0 = 2") +__xlated("7: exit") +__success +__naked void may_goto_batch_2(void) +{ + asm volatile ( + ".8byte %[may_goto1];" + ".8byte %[may_goto3];" + "r0 = 1;" + "r0 = 2;" + "exit;" + : + : __imm_insn(may_goto1, BPF_RAW_INSN(BPF_JMP | BPF_JCOND, 0, 0, 2 /* offset */, 0)), + __imm_insn(may_goto3, BPF_RAW_INSN(BPF_JMP | BPF_JCOND, 0, 0, 0 /* offset */, 0)) + : __clobber_all); +} + +char _license[] SEC("license") = "GPL"; diff --git a/tools/testing/selftests/bpf/progs/verifier_may_goto_2.c b/tools/testing/selftests/bpf/progs/verifier_may_goto_2.c new file mode 100644 index 000000000000..b891faf50660 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/verifier_may_goto_2.c @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */ + +#include "bpf_misc.h" +#include "bpf_experimental.h" + +int gvar; + +SEC("raw_tp") +__description("C code with may_goto 0") +__success +int may_goto_c_code(void) +{ + int i, tmp[3]; + + for (i = 0; i < 3 && can_loop; i++) + tmp[i] = 0; + + for (i = 0; i < 3 && can_loop; i++) + tmp[i] = gvar - i; + + for (i = 0; i < 3 && can_loop; i++) + gvar += tmp[i]; + + return 0; +} + +char _license[] SEC("license") = "GPL";