BPF List
 help / color / mirror / Atom feed
From: Matt Bobrowski <mattbobrowski@google.com>
To: bpf@vger.kernel.org
Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	song@kernel.org, kpsingh@kernel.org, sdf@google.com,
	haoluo@google.com, memxor@gmail.com, void@manifault.com,
	jolsa@kernel.org
Subject: [PATCH 2/2] selftests/bpf: add negative tests for relaxed fixed offset constraint on trusted pointer arguments
Date: Mon, 17 Jun 2024 13:44:02 +0000	[thread overview]
Message-ID: <ZnA9osZKFOPFwvxa@google.com> (raw)

Adding some new negative selftests which are responsible for asserting
that the new relaxed fixed offset constraints applicable to BPF
helpers and kfuncs taking trusted pointer arguments are enforced
correctly by the BPF verifier.

The BPF programs contained within the new negative selftests are
mainly responsible for triggering the various branches and checks
performed within the check_release_arg_reg_off() helper.

Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
---
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../bpf/progs/verifier_arg_reg_off_reject.c   | 154 ++++++++++++++++++
 2 files changed, 156 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_arg_reg_off_reject.c

diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index 6816ff064516..e315bd0a1502 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -87,6 +87,7 @@
 #include "verifier_xdp.skel.h"
 #include "verifier_xdp_direct_packet_access.skel.h"
 #include "verifier_bits_iter.skel.h"
+#include "verifier_arg_reg_off_reject.skel.h"
 
 #define MAX_ENTRIES 11
 
@@ -204,6 +205,7 @@ void test_verifier_xadd(void)                 { RUN(verifier_xadd); }
 void test_verifier_xdp(void)                  { RUN(verifier_xdp); }
 void test_verifier_xdp_direct_packet_access(void) { RUN(verifier_xdp_direct_packet_access); }
 void test_verifier_bits_iter(void) { RUN(verifier_bits_iter); }
+void test_verifier_arg_reg_off_reject(void) { RUN(verifier_arg_reg_off_reject); }
 
 static int init_test_val_map(struct bpf_object *obj, char *map_name)
 {
diff --git a/tools/testing/selftests/bpf/progs/verifier_arg_reg_off_reject.c b/tools/testing/selftests/bpf/progs/verifier_arg_reg_off_reject.c
new file mode 100644
index 000000000000..b46656f4cb62
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_arg_reg_off_reject.c
@@ -0,0 +1,154 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Google LLC. */
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <linux/limits.h>
+
+#include "bpf_misc.h"
+#include "bpf_experimental.h"
+
+struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym;
+void bpf_task_release(struct task_struct *p) __ksym;
+
+struct random_type {
+	u64 id;
+	u64 ref;
+};
+
+struct alloc_type {
+	u64 id;
+	struct nested_type {
+		u64 id;
+	} n;
+	struct random_type __kptr *r;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_RINGBUF);
+	__uint(max_entries, 256 * 1024);
+} ringbuf SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__type(key, int);
+	__type(value, struct alloc_type);
+	__uint(max_entries, 1);
+} array_map SEC(".maps");
+
+SEC("tc")
+__failure
+__msg("R1 must have a fixed offset of 0 when passed to a OBJ_RELEASE/KF_RELEASE flagged BPF helper/kfunc which takes a void *")
+int alloc_obj_release(void *ctx)
+{
+	struct alloc_type *a;
+
+	a = bpf_obj_new(typeof(*a));
+	if (!a) {
+		return 0;
+	}
+	/* bpf_obj_drop_impl() takes a void *, so when we attempt to pass in
+	 * something with a reg->off, it should be rejected as we expect to have
+	 * the original pointer passed to the respective BPF helper unmodified.
+	 */
+	bpf_obj_drop(&a->n);
+	return 0;
+}
+
+SEC("lsm.s/file_open")
+__failure
+__msg("R1 must have a fixed offset of 0 when passed to a OBJ_RELEASE/KF_RELEASE flagged BPF helper/kfunc which takes a void *")
+int BPF_PROG(mem_obj_release, struct file *file)
+{
+	int ret;
+	char *buf;
+
+	buf = bpf_ringbuf_reserve(&ringbuf, PATH_MAX, 0);
+	if (!buf)
+		return 0;
+
+	ret = bpf_d_path(&file->f_path, buf, PATH_MAX);
+	if (ret <= 0) {
+		bpf_ringbuf_discard(buf += 8, 0);
+		return 0;
+	}
+
+	bpf_ringbuf_submit(buf += 8, 0);
+	return 0;
+}
+
+SEC("tp_btf/task_newtask")
+__failure
+__msg("dereference of modified ptr_ ptr R1 off=44 disallowed")
+__msg("R1 must have a fixed offset of 0 when passed to a OBJ_RELEASE/KF_RELEASE flagged BPF helper/kfunc which takes a void *")
+int BPF_PROG(type_match_mismatch, struct task_struct *task,
+	     u64 clone_flags)
+{
+	struct task_struct *acquired;
+
+	acquired = bpf_task_acquire(bpf_get_current_task_btf());
+	if (!acquired)
+		return 0;
+
+	bpf_task_release((struct task_struct *)&acquired->flags);
+	return 0;
+}
+
+SEC("tp_btf/task_newtask")
+__failure
+__msg("kernel function bpf_task_acquire args#0 expected pointer to STRUCT task_struct")
+int BPF_PROG(trusted_type_match_mismatch, struct task_struct *task,
+	     u64 clone_flags)
+{
+	/* Passing a trusted pointer with incorrect offset will result in a type
+	 * mismatch.
+	 */
+	bpf_task_acquire((struct task_struct *)&bpf_get_current_task_btf()->flags);
+	return 0;
+}
+
+SEC("tp_btf/task_newtask")
+__failure
+__msg("variable trusted_ptr_ access var_off=(0x0; 0xffffffff) disallowed")
+int BPF_PROG(trusted_type_match_mismatch_var_off, struct task_struct *task,
+	     u64 clone_flags)
+{
+	u32 var_off = bpf_get_prandom_u32();
+	task = bpf_get_current_task_btf();
+
+	task = (void *)task + var_off;
+	/* Passing a trusted pointer with an incorrect variable offset, type
+	 * match will succeed due to reg->off == 0, but the later call to
+	 * __check_ptr_off_reg should fail as it's responsible for checking
+	 * reg->var_off.
+	 */
+	bpf_task_acquire(task);
+	return 0;
+}
+
+SEC("tp_btf/task_newtask")
+__failure
+__msg("variable trusted_ptr_ access var_off=(0x0; 0xffffffffffffffff) disallowed")
+int BPF_PROG(trusted_type_match_mismatch_neg_var_off, struct task_struct *task,
+	     u64 clone_flags)
+{
+	s64 var_off = task->start_time;
+	task = bpf_get_current_task_btf();
+
+	bpf_assert_range(var_off, -64, 64);
+	/* Need one bpf_throw() reference, otherwise BTF gen fails. */
+	if (!task)
+		bpf_throw(1);
+
+	task = (void *)task + var_off;
+	/* Passing a trusted pointer with an incorrect variable offset, type
+	 * match will succeed due to reg->off == 0, but the later call to
+	 * __check_ptr_off_reg should fail as it's responsible for checking
+	 * reg->var_off.
+	 */
+	task = bpf_task_acquire(task);
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.45.2.627.g7a2c4fd464-goog

/M

             reply	other threads:[~2024-06-17 13:44 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-17 13:44 Matt Bobrowski [this message]
2024-06-19  9:02 ` [PATCH 2/2] selftests/bpf: add negative tests for relaxed fixed offset constraint on trusted pointer arguments Kumar Kartikeya Dwivedi

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=ZnA9osZKFOPFwvxa@google.com \
    --to=mattbobrowski@google.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=memxor@gmail.com \
    --cc=sdf@google.com \
    --cc=song@kernel.org \
    --cc=void@manifault.com \
    /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