[PATCH bpf-next 1/2] bpf: Avoid warning on unexpected map for tail call

[PATCH bpf-next 1/2] bpf: Avoid warning on unexpected map for tail call

[Paul Chaignon]

Before handling the tail call in record_func_key(), we check that the
map is of the expected type and log a verifier error if it isn't. Such
an error however doesn't indicate anything wrong with the verifier. The
check for map<>func compatibility is done after record_func_key(), by
check_map_func_compatibility().

Therefore, this patch logs the error as a typical reject instead of a
verifier error.

Fixes: d2e4c1e6c294 ("bpf: Constant map key tracking for prog array pokes")
Fixes: 0df1a55afa83 ("bpf: Warn on internal verifier errors")
Reported-by: syzbot+efb099d5833bca355e51@syzkaller.appspotmail.com
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
Note: I'm sending this to bpf-next and not bpf because the warning
addition from commit 0df1a55afa83 didn't make it into bpf yet.

 kernel/bpf/verifier.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 52e36fd23f40..c71e75e4740a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11081,8 +11081,8 @@ record_func_key(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta,
 	if (func_id != BPF_FUNC_tail_call)
 		return 0;
 	if (!map || map->map_type != BPF_MAP_TYPE_PROG_ARRAY) {
-		verifier_bug(env, "expected array map for tail call");
-		return -EFAULT;
+		verbose(env, "expected prog array map for tail call");
+		return -EINVAL;
 	}
 
 	reg = &regs[BPF_REG_3];
-- 
2.43.0

[PATCH bpf-next 2/2] selftests/bpf: Negative test case for tail call map

[Paul Chaignon]

This patch adds a negative test case for the following verifier error.

    expected prog array map for tail call

Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
 tools/testing/selftests/bpf/verifier/calls.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/testing/selftests/bpf/verifier/calls.c b/tools/testing/selftests/bpf/verifier/calls.c
index f3492efc8834..d0771da83a2f 100644
--- a/tools/testing/selftests/bpf/verifier/calls.c
+++ b/tools/testing/selftests/bpf/verifier/calls.c
@@ -2433,3 +2433,16 @@
 	.errstr = "more than one arg with ref_obj_id",
 	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
 },
+{
+	"calls: wrong map type for tail call",
+	.insns = {
+	BPF_MOV64_IMM(BPF_REG_3, 0),
+	BPF_LD_MAP_FD(BPF_REG_2, 0),
+	BPF_EMIT_CALL(BPF_FUNC_tail_call),
+	BPF_EXIT_INSN(),
+	},
+	.fixup_map_array_small = { 1 },
+	.result = REJECT,
+	.errstr = "expected prog array map for tail call",
+	.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+},
-- 
2.43.0

Re: [PATCH bpf-next 1/2] bpf: Avoid warning on unexpected map for tail call

[Eduard Zingerman]

On Thu, 2025-07-03 at 23:35 +0200, Paul Chaignon wrote:
> Before handling the tail call in record_func_key(), we check that the
> map is of the expected type and log a verifier error if it isn't. Such
> an error however doesn't indicate anything wrong with the verifier. The
> check for map<>func compatibility is done after record_func_key(), by
> check_map_func_compatibility().
> 
> Therefore, this patch logs the error as a typical reject instead of a
> verifier error.
> 
> Fixes: d2e4c1e6c294 ("bpf: Constant map key tracking for prog array pokes")
> Fixes: 0df1a55afa83 ("bpf: Warn on internal verifier errors")
> Reported-by: syzbot+efb099d5833bca355e51@syzkaller.appspotmail.com
> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> ---

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

[...]

Re: [PATCH bpf-next 2/2] selftests/bpf: Negative test case for tail call map

[Eduard Zingerman]

On Thu, 2025-07-03 at 23:36 +0200, Paul Chaignon wrote:
> This patch adds a negative test case for the following verifier error.
> 
>     expected prog array map for tail call
> 
> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> ---

Nit: selftests/bpf/verifier binary is considered obsolete,
     new tests are mostly added as a part test_progs binary.
     E.g. you can add a file progs/verifier_tailcalls.c,
     register it in test_progs/verifier.c and define
     tests using test_loader framework, e.g. see
     progs/verifier_and.c.

[...]

Re: [PATCH bpf-next 1/2] bpf: Avoid warning on unexpected map for tail call

[patchwork-bot+netdevbpf]

Hello:

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

On Thu, 3 Jul 2025 23:35:09 +0200 you wrote:
> Before handling the tail call in record_func_key(), we check that the
> map is of the expected type and log a verifier error if it isn't. Such
> an error however doesn't indicate anything wrong with the verifier. The
> check for map<>func compatibility is done after record_func_key(), by
> check_map_func_compatibility().
> 
> Therefore, this patch logs the error as a typical reject instead of a
> verifier error.
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/2] bpf: Avoid warning on unexpected map for tail call
    https://git.kernel.org/bpf/bpf-next/c/032547272eb0
  - [bpf-next,2/2] selftests/bpf: Negative test case for tail call map
    (no matching commit)

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

Re: [PATCH bpf-next 2/2] selftests/bpf: Negative test case for tail call map

[Paul Chaignon]

On Thu, Jul 03, 2025 at 02:55:01PM -0700, Eduard Zingerman wrote:
> On Thu, 2025-07-03 at 23:36 +0200, Paul Chaignon wrote:
> > This patch adds a negative test case for the following verifier error.
> > 
> >     expected prog array map for tail call
> > 
> > Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> > ---
> 
> Nit: selftests/bpf/verifier binary is considered obsolete,

I didn't know that. Too bad, those verifier/ tests were easy to extract
and reuse as syzkaller seeds :)

I've sent a v2 with the new test format. Thanks for the review!

>      new tests are mostly added as a part test_progs binary.
>      E.g. you can add a file progs/verifier_tailcalls.c,
>      register it in test_progs/verifier.c and define
>      tests using test_loader framework, e.g. see
>      progs/verifier_and.c.
> 
> [...]