BPF List
 help / color / mirror / Atom feed
From: Kumar Kartikeya Dwivedi <memxor@gmail.com>
To: bpf@vger.kernel.org
Cc: kkd@meta.com, Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <martin.lau@kernel.org>,
	Eduard Zingerman <eddyz87@gmail.com>,
	Manu Bretelle <chantra@meta.com>,
	kernel-team@fb.com
Subject: [PATCH bpf v3 3/3] selftests/bpf: Add raw_tp tests for PTR_MAYBE_NULL marking
Date: Fri,  6 Dec 2024 08:10:53 -0800	[thread overview]
Message-ID: <20241206161053.809580-4-memxor@gmail.com> (raw)
In-Reply-To: <20241206161053.809580-1-memxor@gmail.com>

Ensure that pointers with off != 0 are never unmarked as PTR_MAYBE_NULL
when doing NULL checks, while pointers that have off == 0 continue to
get unmarked and propagate unmarking to all other registers sharing id.
Lastly, ensure that in the path where pointer is NULL, the unmarking is
not performed for any registers sharing the same id.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/prog_tests/raw_tp_null.c    |  6 ++
 .../selftests/bpf/progs/raw_tp_null_fail.c    | 90 +++++++++++++++++++
 2 files changed, 96 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/raw_tp_null_fail.c

diff --git a/tools/testing/selftests/bpf/prog_tests/raw_tp_null.c b/tools/testing/selftests/bpf/prog_tests/raw_tp_null.c
index 6fa19449297e..13fcd4c31034 100644
--- a/tools/testing/selftests/bpf/prog_tests/raw_tp_null.c
+++ b/tools/testing/selftests/bpf/prog_tests/raw_tp_null.c
@@ -3,6 +3,12 @@
 
 #include <test_progs.h>
 #include "raw_tp_null.skel.h"
+#include "raw_tp_null_fail.skel.h"
+
+void test_raw_tp_null_fail(void)
+{
+	RUN_TESTS(raw_tp_null_fail);
+}
 
 void test_raw_tp_null(void)
 {
diff --git a/tools/testing/selftests/bpf/progs/raw_tp_null_fail.c b/tools/testing/selftests/bpf/progs/raw_tp_null_fail.c
new file mode 100644
index 000000000000..a87f25ee61ff
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/raw_tp_null_fail.c
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
+
+#include <vmlinux.h>
+#include <bpf/bpf_tracing.h>
+#include "bpf_misc.h"
+
+/* r1 with off=0 is checked, which marks r0 with off=8 as non-null */
+SEC("tp_btf/bpf_testmod_test_raw_tp_null")
+__success
+__log_level(2)
+__msg("3: (07) r0 += 8                       ; R0_w=trusted_ptr_or_null_sk_buff(id=1,off=8)")
+__msg("4: (15) if r1 == 0x0 goto pc+4        ; R1_w=trusted_ptr_sk_buff()")
+__msg("5: (bf) r2 = r0                       ; R0_w=trusted_ptr_sk_buff(off=8)")
+/* For the path where we saw r1 as != NULL, we will see this state */
+__msg("6: (79) r2 = *(u64 *)(r1 +0)          ; R1_w=trusted_ptr_sk_buff()")
+/* In the NULL path, ensure registers are not marked as scalar */
+/* For the path where we saw r1 as NULL, we will see this state */
+__msg("from 4 to 9: R0=trusted_ptr_or_null_sk_buff(id=1,off=8) R1=trusted_ptr_or_null_sk_buff(id=1)")
+__msg("9: (79) r2 = *(u64 *)(r1 +0)          ; R1=trusted_ptr_or_null_sk_buff(id=1)")
+int BPF_PROG(test_raw_tp_null_check_zero_off, struct sk_buff *skb)
+{
+	asm volatile (
+		"r1 = *(u64 *)(r1 +0);			\
+		 r0 = r1;				\
+		 r2 = 0;				\
+		 r0 += 8;				\
+		 if r1 == 0 goto jmp;			\
+		 r2 = r0;				\
+		 r2 = *(u64 *)(r1 +0);			\
+		 r0 = 0;				\
+		 exit;					\
+		 jmp:					\
+		 r2 = *(u64 *)(r1 +0)"
+		::
+		: __clobber_all
+	);
+	return 0;
+}
+
+/* r2 with offset is checked, which won't mark r1 with off=0 as non-NULL */
+SEC("tp_btf/bpf_testmod_test_raw_tp_null")
+__success
+__log_level(2)
+__msg("3: (07) r2 += 8                       ; R2_w=trusted_ptr_or_null_sk_buff(id=1,off=8)")
+__msg("4: (15) if r2 == 0x0 goto pc+1        ; R2_w=trusted_ptr_or_null_sk_buff(id=1,off=8)")
+__msg("5: (bf) r2 = r1                       ; R1_w=trusted_ptr_or_null_sk_buff(id=1)")
+int BPF_PROG(test_raw_tp_null_copy_check_with_off, struct sk_buff *skb)
+{
+	asm volatile (
+		"r1 = *(u64 *)(r1 +0);			\
+		 r2 = r1;				\
+		 r3 = 0;				\
+		 r2 += 8;				\
+		 if r2 == 0 goto jmp2;			\
+		 r2 = r1;				\
+		 jmp2:					"
+		::
+		: __clobber_all
+	);
+	return 0;
+}
+
+/* Ensure state doesn't change for r0 and r1 when performing repeated checks.. */
+SEC("tp_btf/bpf_testmod_test_raw_tp_null")
+__success
+__log_level(2)
+__msg("2: (07) r0 += 8                       ; R0_w=trusted_ptr_or_null_sk_buff(id=1,off=8)")
+__msg("3: (15) if r0 == 0x0 goto pc+3        ; R0_w=trusted_ptr_or_null_sk_buff(id=1,off=8)")
+__msg("4: (15) if r0 == 0x0 goto pc+2        ; R0_w=trusted_ptr_or_null_sk_buff(id=1,off=8)")
+__msg("5: (15) if r0 == 0x0 goto pc+1        ; R0_w=trusted_ptr_or_null_sk_buff(id=1,off=8)")
+__msg("6: (bf) r2 = r1                       ; R1=trusted_ptr_or_null_sk_buff(id=1)")
+int BPF_PROG(test_raw_tp_check_with_off, struct sk_buff *skb)
+{
+	asm volatile (
+		"r1 = *(u64 *)(r1 +0);			\
+		 r0 = r1;				\
+		 r0 += 8;				\
+		 if r0 == 0 goto jmp3;			\
+		 if r0 == 0 goto jmp3;			\
+		 if r0 == 0 goto jmp3;			\
+		 r2 = r1;				\
+		 jmp3:					"
+		::
+		: __clobber_all
+	);
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.43.5


      parent reply	other threads:[~2024-12-06 16:11 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-06 16:10 [PATCH bpf v3 0/3] Fix for raw_tp PTR_MAYBE_NULL handling Kumar Kartikeya Dwivedi
2024-12-06 16:10 ` [PATCH bpf v3 1/3] bpf: Suppress warning for non-zero off raw_tp arg NULL check Kumar Kartikeya Dwivedi
2024-12-06 16:10 ` [PATCH bpf v3 2/3] bpf: Do not mark NULL-checked raw_tp arg as scalar Kumar Kartikeya Dwivedi
2024-12-06 17:59   ` Alexei Starovoitov
2024-12-06 18:10     ` Kumar Kartikeya Dwivedi
2024-12-06 18:37       ` Alexei Starovoitov
2024-12-06 19:09         ` Kumar Kartikeya Dwivedi
2024-12-06 19:14           ` Alexei Starovoitov
2024-12-09 23:35       ` Jiri Olsa
2024-12-06 18:15     ` Eduard Zingerman
2024-12-06 18:24       ` Kumar Kartikeya Dwivedi
2024-12-06 18:36       ` Alexei Starovoitov
2024-12-06 19:10         ` Kumar Kartikeya Dwivedi
2024-12-06 19:18           ` Alexei Starovoitov
2024-12-06 16:10 ` Kumar Kartikeya Dwivedi [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241206161053.809580-4-memxor@gmail.com \
    --to=memxor@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=chantra@meta.com \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=kernel-team@fb.com \
    --cc=kkd@meta.com \
    --cc=martin.lau@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox