All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 1/2] selftests/bpf: Negative test case for ref_obj_id in args
@ 2025-07-02 13:53 Paul Chaignon
  2025-07-02 13:54 ` [PATCH bpf-next 2/2] bpf: Avoid warning on multiple referenced args in call Paul Chaignon
  2025-07-02 15:50 ` [PATCH bpf-next 1/2] selftests/bpf: Negative test case for ref_obj_id in args patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Paul Chaignon @ 2025-07-02 13:53 UTC (permalink / raw)
  To: bpf; +Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko

This patch adds a test case, as shown below, for the verifier error
"more than one arg with ref_obj_id".

    0: (b7) r2 = 20
    1: (b7) r3 = 0
    2: (18) r1 = 0xffff92cee3cbc600
    4: (85) call bpf_ringbuf_reserve#131
    5: (55) if r0 == 0x0 goto pc+3
    6: (bf) r1 = r0
    7: (bf) r2 = r0
    8: (85) call bpf_tcp_raw_gen_syncookie_ipv4#204
    9: (95) exit

This error is currently incorrectly reported as a verifier bug, with a
warning. The next patch in this series will address that.

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

diff --git a/tools/testing/selftests/bpf/verifier/calls.c b/tools/testing/selftests/bpf/verifier/calls.c
index 18596ae0b0c1..f3492efc8834 100644
--- a/tools/testing/selftests/bpf/verifier/calls.c
+++ b/tools/testing/selftests/bpf/verifier/calls.c
@@ -2409,3 +2409,27 @@
 	.errstr_unpriv = "",
 	.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
 },
+{
+	"calls: several args with ref_obj_id",
+	.insns = {
+	/* Reserve at least sizeof(struct iphdr) bytes in the ring buffer.
+	 * With a smaller size, the verifier would reject the call to
+	 * bpf_tcp_raw_gen_syncookie_ipv4 before we can reach the
+	 * ref_obj_id error.
+	 */
+	BPF_MOV64_IMM(BPF_REG_2, 20),
+	BPF_MOV64_IMM(BPF_REG_3, 0),
+	BPF_LD_MAP_FD(BPF_REG_1, 0),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_ringbuf_reserve),
+	/* if r0 == 0 goto <exit> */
+	BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 3),
+	BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+	BPF_MOV64_REG(BPF_REG_2, BPF_REG_0),
+	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_tcp_raw_gen_syncookie_ipv4),
+	BPF_EXIT_INSN(),
+	},
+	.fixup_map_ringbuf = { 2 },
+	.result = REJECT,
+	.errstr = "more than one arg with ref_obj_id",
+	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
+},
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH bpf-next 2/2] bpf: Avoid warning on multiple referenced args in call
  2025-07-02 13:53 [PATCH bpf-next 1/2] selftests/bpf: Negative test case for ref_obj_id in args Paul Chaignon
@ 2025-07-02 13:54 ` Paul Chaignon
  2025-07-02 15:50 ` [PATCH bpf-next 1/2] selftests/bpf: Negative test case for ref_obj_id in args patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: Paul Chaignon @ 2025-07-02 13:54 UTC (permalink / raw)
  To: bpf; +Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko

The description of full helper calls in syzkaller [1] and the addition of
kernel warnings in commit 0df1a55afa83 ("bpf: Warn on internal verifier
errors") allowed syzbot to reach a verifier state that was thought to
indicate a verifier bug [2]:

    12: (85) call bpf_tcp_raw_gen_syncookie_ipv4#204
    verifier bug: more than one arg with ref_obj_id R2 2 2

This error can be reproduced with the program from the previous commit:

    0: (b7) r2 = 20
    1: (b7) r3 = 0
    2: (18) r1 = 0xffff92cee3cbc600
    4: (85) call bpf_ringbuf_reserve#131
    5: (55) if r0 == 0x0 goto pc+3
    6: (bf) r1 = r0
    7: (bf) r2 = r0
    8: (85) call bpf_tcp_raw_gen_syncookie_ipv4#204
    9: (95) exit

bpf_tcp_raw_gen_syncookie_ipv4 expects R1 and R2 to be
ARG_PTR_TO_FIXED_SIZE_MEM (with a size of at least sizeof(struct iphdr)
for R1). R0 is a ring buffer payload of 20B and therefore matches this
requirement.

The verifier reaches the check on ref_obj_id while verifying R2 and
rejects the program because the helper isn't supposed to take two
referenced arguments.

This case is a legitimate rejection and doesn't indicate a kernel bug,
so we shouldn't log it as such and shouldn't emit a kernel warning.

Link: https://github.com/google/syzkaller/pull/4313 [1]
Link: https://lore.kernel.org/all/686491d6.a70a0220.3b7e22.20ea.GAE@google.com/T/ [2]
Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier support for it")
Fixes: 0df1a55afa83 ("bpf: Warn on internal verifier errors")
Reported-by: syzbot+69014a227f8edad4d8c6@syzkaller.appspotmail.com
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
Note: I'm sending this to bpf-next instead of bpf because the kernel
warning hasn't made it into bpf yet and I consider that the main error
(vs. the incorrect verifier log).

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

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a352b35be479..712a5c4fb6a4 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9673,10 +9673,10 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 
 	if (reg->ref_obj_id && base_type(arg_type) != ARG_KPTR_XCHG_DEST) {
 		if (meta->ref_obj_id) {
-			verifier_bug(env, "more than one arg with ref_obj_id R%d %u %u",
-				     regno, reg->ref_obj_id,
-				     meta->ref_obj_id);
-			return -EFAULT;
+			verbose(env, "more than one arg with ref_obj_id R%d %u %u",
+				regno, reg->ref_obj_id,
+				meta->ref_obj_id);
+			return -EACCES;
 		}
 		meta->ref_obj_id = reg->ref_obj_id;
 	}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf-next 1/2] selftests/bpf: Negative test case for ref_obj_id in args
  2025-07-02 13:53 [PATCH bpf-next 1/2] selftests/bpf: Negative test case for ref_obj_id in args Paul Chaignon
  2025-07-02 13:54 ` [PATCH bpf-next 2/2] bpf: Avoid warning on multiple referenced args in call Paul Chaignon
@ 2025-07-02 15:50 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-07-02 15:50 UTC (permalink / raw)
  To: Paul Chaignon; +Cc: bpf, ast, daniel, andrii

Hello:

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

On Wed, 2 Jul 2025 15:53:23 +0200 you wrote:
> This patch adds a test case, as shown below, for the verifier error
> "more than one arg with ref_obj_id".
> 
>     0: (b7) r2 = 20
>     1: (b7) r3 = 0
>     2: (18) r1 = 0xffff92cee3cbc600
>     4: (85) call bpf_ringbuf_reserve#131
>     5: (55) if r0 == 0x0 goto pc+3
>     6: (bf) r1 = r0
>     7: (bf) r2 = r0
>     8: (85) call bpf_tcp_raw_gen_syncookie_ipv4#204
>     9: (95) exit
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/2] selftests/bpf: Negative test case for ref_obj_id in args
    https://git.kernel.org/bpf/bpf-next/c/1f2dfde4f36f
  - [bpf-next,2/2] bpf: Avoid warning on multiple referenced args in call
    https://git.kernel.org/bpf/bpf-next/c/564606fec540

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



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-07-02 15:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-02 13:53 [PATCH bpf-next 1/2] selftests/bpf: Negative test case for ref_obj_id in args Paul Chaignon
2025-07-02 13:54 ` [PATCH bpf-next 2/2] bpf: Avoid warning on multiple referenced args in call Paul Chaignon
2025-07-02 15:50 ` [PATCH bpf-next 1/2] selftests/bpf: Negative test case for ref_obj_id in args patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.