[PATCH v2 bpf-next 0/2] bpf, verifier: Correct tail_call_reachable for bpf prog

[PATCH v2 bpf-next 0/2] bpf, verifier: Correct tail_call_reachable for bpf prog

[Leon Hwang]

It's confusing to inspect 'prog->aux->tail_call_reachable' with drgn[0],
when bpf prog has tail call but 'tail_call_reachable' is false.

This patch corrects 'tail_call_reachable' when bpf prog has tail call.

Therefore, it's unnecessary to detect tail call in x86 jit. Let's remove
it.

Changes:
v1 -> v2:
* Address comment from Yonghong:
  * Remove unnecessary tail call detection in x86 jit.

---

Links:
[0] https://github.com/osandov/drgn

Leon Hwang (2):
  bpf, verifier: Correct tail_call_reachable for bpf prog
  bpf, x64: Remove tail call detection

 arch/x86/net/bpf_jit_comp.c | 11 ++---------
 kernel/bpf/verifier.c       |  4 +++-
 2 files changed, 5 insertions(+), 10 deletions(-)


base-commit: 2c6987105026a4395935a3db665c54eb1bafe782
-- 
2.44.0

[PATCH v2 bpf-next 1/2] bpf, verifier: Correct tail_call_reachable for bpf prog

[Leon Hwang]

It's confusing to inspect 'prog->aux->tail_call_reachable' with drgn[0],
when bpf prog has tail call but 'tail_call_reachable' is false.

This patch corrects 'tail_call_reachable' when bpf prog has tail call.

Signed-off-by: Leon Hwang <hffilwlqm@gmail.com>
---
 kernel/bpf/verifier.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 81a3d2ced78d5..d7045676246a7 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2982,8 +2982,10 @@ static int check_subprogs(struct bpf_verifier_env *env)
 
 		if (code == (BPF_JMP | BPF_CALL) &&
 		    insn[i].src_reg == 0 &&
-		    insn[i].imm == BPF_FUNC_tail_call)
+		    insn[i].imm == BPF_FUNC_tail_call) {
 			subprog[cur_subprog].has_tail_call = true;
+			subprog[cur_subprog].tail_call_reachable = true;
+		}
 		if (BPF_CLASS(code) == BPF_LD &&
 		    (BPF_MODE(code) == BPF_ABS || BPF_MODE(code) == BPF_IND))
 			subprog[cur_subprog].has_ld_abs = true;
-- 
2.44.0

[PATCH v2 bpf-next 2/2] bpf, x64: Remove tail call detection

[Leon Hwang]

As 'prog->aux->tail_call_reachable' is correct for tail call present,
it's unnecessary to detect tail call in x86 jit.

Therefore, let's remove it.

Signed-off-by: Leon Hwang <hffilwlqm@gmail.com>
---
 arch/x86/net/bpf_jit_comp.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 5159c7a229229..7c130001fbfe7 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1234,13 +1234,11 @@ bool ex_handler_bpf(const struct exception_table_entry *x, struct pt_regs *regs)
 }
 
 static void detect_reg_usage(struct bpf_insn *insn, int insn_cnt,
-			     bool *regs_used, bool *tail_call_seen)
+			     bool *regs_used)
 {
 	int i;
 
 	for (i = 1; i <= insn_cnt; i++, insn++) {
-		if (insn->code == (BPF_JMP | BPF_TAIL_CALL))
-			*tail_call_seen = true;
 		if (insn->dst_reg == BPF_REG_6 || insn->src_reg == BPF_REG_6)
 			regs_used[0] = true;
 		if (insn->dst_reg == BPF_REG_7 || insn->src_reg == BPF_REG_7)
@@ -1324,7 +1322,6 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image, u8 *rw_image
 	struct bpf_insn *insn = bpf_prog->insnsi;
 	bool callee_regs_used[4] = {};
 	int insn_cnt = bpf_prog->len;
-	bool tail_call_seen = false;
 	bool seen_exit = false;
 	u8 temp[BPF_MAX_INSN_SIZE + BPF_INSN_SAFETY];
 	u64 arena_vm_start, user_vm_start;
@@ -1336,11 +1333,7 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image, u8 *rw_image
 	arena_vm_start = bpf_arena_get_kern_vm_start(bpf_prog->aux->arena);
 	user_vm_start = bpf_arena_get_user_vm_start(bpf_prog->aux->arena);
 
-	detect_reg_usage(insn, insn_cnt, callee_regs_used,
-			 &tail_call_seen);
-
-	/* tail call's presence in current prog implies it is reachable */
-	tail_call_reachable |= tail_call_seen;
+	detect_reg_usage(insn, insn_cnt, callee_regs_used);
 
 	emit_prologue(&prog, bpf_prog->aux->stack_depth,
 		      bpf_prog_was_classic(bpf_prog), tail_call_reachable,
-- 
2.44.0

Re: [PATCH v2 bpf-next 0/2] bpf, verifier: Correct tail_call_reachable for bpf prog

[Eduard Zingerman]

On Mon, 2024-06-10 at 20:42 +0800, Leon Hwang wrote:
> It's confusing to inspect 'prog->aux->tail_call_reachable' with drgn[0],
> when bpf prog has tail call but 'tail_call_reachable' is false.
> 
> This patch corrects 'tail_call_reachable' when bpf prog has tail call.
> 
> Therefore, it's unnecessary to detect tail call in x86 jit. Let's remove
> it.
> 
> Changes:
> v1 -> v2:
> * Address comment from Yonghong:
>   * Remove unnecessary tail call detection in x86 jit.
> 
> ---

All seems correct.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

Re: [PATCH v2 bpf-next 0/2] bpf, verifier: Correct tail_call_reachable for bpf prog

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Mon, 10 Jun 2024 20:42:22 +0800 you wrote:
> It's confusing to inspect 'prog->aux->tail_call_reachable' with drgn[0],
> when bpf prog has tail call but 'tail_call_reachable' is false.
> 
> This patch corrects 'tail_call_reachable' when bpf prog has tail call.
> 
> Therefore, it's unnecessary to detect tail call in x86 jit. Let's remove
> it.
> 
> [...]

Here is the summary with links:
  - [v2,bpf-next,1/2] bpf, verifier: Correct tail_call_reachable for bpf prog
    https://git.kernel.org/bpf/bpf-next/c/01793ed86b5d
  - [v2,bpf-next,2/2] bpf, x64: Remove tail call detection
    https://git.kernel.org/bpf/bpf-next/c/f663a03c8e35

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html