Skip to contentSkip to conversation

bpf, verifier: Correct tail_call_reachable for bpf prog

PATCH Applied Leon Hwang 3 participants 5 messages Jun 10, 2024 – Jun 21, 2024

Subsystems (4)
x86 BPF BPF core BPF JIT for x86 64-bit
lore
Share & tools
lore

Leon Hwang

author
#0 lore raw

v2 cover bpf, verifier: Correct tail_call_reachable for bpf prog

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

Signature · 1 line
2.44.0
author
#1 lore raw

PATCH v2 1/2 bpf, verifier: Correct tail_call_reachable for bpf prog

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 <[email protected]>
---
 kernel/bpf/verifier.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
kernel/bpf/verifier.c +3 -1
index 81a3d2ced78d5..d7045676246a7 100644
@@ -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;
Signature · 1 line
2.44.0
author
#2 lore raw

PATCH v2 2/2 bpf, x64: Remove tail call detection

In reply to 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 <[email protected]>
---
 arch/x86/net/bpf_jit_comp.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)
arch/x86/net/bpf_jit_comp.c +2 -9
index 5159c7a229229..7c130001fbfe7 100644
@@ -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,
Signature · 1 line
2.44.0
maintainer Acked-by on the v2 series
#3 lore raw
In reply to Leon Hwang
Quoted 8 lines · 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.

Quoted 6 lines

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

---

All seems correct.

Acked-by Eduard Zingerman <[email protected]>
CI Applied
#4 lore raw
In reply to Leon Hwang
This series was applied to bpf/bpf-next.git (master) by Alexei Starovoitov <[email protected]>:

Hello:

This series was applied to bpf/bpf-next.git (master) by Alexei Starovoitov <[email protected]>:

Quoted 9 lines · 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!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html