From: David Vernet <void@manifault.com>
To: bpf@vger.kernel.org
Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
martin.lau@linux.dev, song@kernel.org, yhs@fb.com,
john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
haoluo@google.com, jolsa@kernel.org,
linux-kernel@vger.kernel.org, kernel-team@meta.com,
memxor@gmail.com
Subject: [PATCH bpf-next 1/3] bpf: Remove bpf_kfunc_call_test_kptr_get() test kfunc
Date: Sat, 15 Apr 2023 05:32:29 -0500 [thread overview]
Message-ID: <20230415103231.236063-2-void@manifault.com> (raw)
In-Reply-To: <20230415103231.236063-1-void@manifault.com>
We've managed to improve the UX for kptrs significantly over the last 9
months. All of the prior main use cases, struct bpf_cpumask *, struct
task_struct *, and struct cgroup *, have all been updated to be
synchronized using RCU. In other words, their KF_ACQUIRE kfunc calls are
all KF_RCU, and the pointers themselves are MEM_RCU and can be accessed
in an RCU read region in BPF.
In a follow-on change, we'll be removing the KF_KPTR_GET kfunc flag.
This patch prepares for that by removing the
bpf_kfunc_call_test_kptr_get() kfunc, and all associated selftests.
Signed-off-by: David Vernet <void@manifault.com>
---
net/bpf/test_run.c | 12 ---
tools/testing/selftests/bpf/progs/map_kptr.c | 40 ++--------
.../selftests/bpf/progs/map_kptr_fail.c | 78 -------------------
.../testing/selftests/bpf/verifier/map_kptr.c | 27 -------
4 files changed, 5 insertions(+), 152 deletions(-)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 0b9bd9b39990..f170e8a17974 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -679,17 +679,6 @@ __bpf_kfunc void bpf_kfunc_call_int_mem_release(int *p)
{
}
-__bpf_kfunc struct prog_test_ref_kfunc *
-bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **pp, int a, int b)
-{
- struct prog_test_ref_kfunc *p = READ_ONCE(*pp);
-
- if (!p)
- return NULL;
- refcount_inc(&p->cnt);
- return p;
-}
-
struct prog_test_pass1 {
int x0;
struct {
@@ -804,7 +793,6 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test_get_rdwr_mem, KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_kfunc_call_test_get_rdonly_mem, KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_kfunc_call_test_acq_rdonly_mem, KF_ACQUIRE | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_kfunc_call_int_mem_release, KF_RELEASE)
-BTF_ID_FLAGS(func, bpf_kfunc_call_test_kptr_get, KF_ACQUIRE | KF_RET_NULL | KF_KPTR_GET)
BTF_ID_FLAGS(func, bpf_kfunc_call_test_pass_ctx)
BTF_ID_FLAGS(func, bpf_kfunc_call_test_pass1)
BTF_ID_FLAGS(func, bpf_kfunc_call_test_pass2)
diff --git a/tools/testing/selftests/bpf/progs/map_kptr.c b/tools/testing/selftests/bpf/progs/map_kptr.c
index dae5dab1bbf7..d7150041e5d1 100644
--- a/tools/testing/selftests/bpf/progs/map_kptr.c
+++ b/tools/testing/selftests/bpf/progs/map_kptr.c
@@ -115,8 +115,6 @@ DEFINE_MAP_OF_MAP(BPF_MAP_TYPE_HASH_OF_MAPS, hash_malloc_map, hash_of_hash_mallo
DEFINE_MAP_OF_MAP(BPF_MAP_TYPE_HASH_OF_MAPS, lru_hash_map, hash_of_lru_hash_maps);
extern struct prog_test_ref_kfunc *bpf_kfunc_call_test_acquire(unsigned long *sp) __ksym;
-extern struct prog_test_ref_kfunc *
-bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **p, int a, int b) __ksym;
extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
void bpf_kfunc_call_test_ref(struct prog_test_ref_kfunc *p) __ksym;
@@ -187,25 +185,10 @@ static void test_kptr_ref(struct map_value *v)
bpf_kfunc_call_test_release(p);
}
-static void test_kptr_get(struct map_value *v)
-{
- struct prog_test_ref_kfunc *p;
-
- p = bpf_kfunc_call_test_kptr_get(&v->ref_ptr, 0, 0);
- if (!p)
- return;
- if (p->a + p->b > 100) {
- bpf_kfunc_call_test_release(p);
- return;
- }
- bpf_kfunc_call_test_release(p);
-}
-
static void test_kptr(struct map_value *v)
{
test_kptr_unref(v);
test_kptr_ref(v);
- test_kptr_get(v);
}
SEC("tc")
@@ -338,38 +321,25 @@ int test_map_kptr_ref_pre(struct map_value *v)
if (p_st->cnt.refs.counter != ref)
return 4;
- p = bpf_kfunc_call_test_kptr_get(&v->ref_ptr, 0, 0);
- if (!p)
- return 5;
- ref++;
- if (p_st->cnt.refs.counter != ref) {
- ret = 6;
- goto end;
- }
- bpf_kfunc_call_test_release(p);
- ref--;
- if (p_st->cnt.refs.counter != ref)
- return 7;
-
p = bpf_kptr_xchg(&v->ref_ptr, NULL);
if (!p)
- return 8;
+ return 5;
bpf_kfunc_call_test_release(p);
ref--;
if (p_st->cnt.refs.counter != ref)
- return 9;
+ return 6;
p = bpf_kfunc_call_test_acquire(&arg);
if (!p)
- return 10;
+ return 7;
ref++;
p = bpf_kptr_xchg(&v->ref_ptr, p);
if (p) {
- ret = 11;
+ ret = 8;
goto end;
}
if (p_st->cnt.refs.counter != ref)
- return 12;
+ return 9;
/* Leave in map */
return 0;
diff --git a/tools/testing/selftests/bpf/progs/map_kptr_fail.c b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
index 15bf3127dba3..da8c724f839b 100644
--- a/tools/testing/selftests/bpf/progs/map_kptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
@@ -21,8 +21,6 @@ struct array_map {
extern struct prog_test_ref_kfunc *bpf_kfunc_call_test_acquire(unsigned long *sp) __ksym;
extern void bpf_kfunc_call_test_release(struct prog_test_ref_kfunc *p) __ksym;
-extern struct prog_test_ref_kfunc *
-bpf_kfunc_call_test_kptr_get(struct prog_test_ref_kfunc **p, int a, int b) __ksym;
SEC("?tc")
__failure __msg("kptr access size must be BPF_DW")
@@ -220,67 +218,6 @@ int reject_kptr_xchg_on_unref(struct __sk_buff *ctx)
return 0;
}
-SEC("?tc")
-__failure __msg("arg#0 expected pointer to map value")
-int reject_kptr_get_no_map_val(struct __sk_buff *ctx)
-{
- bpf_kfunc_call_test_kptr_get((void *)&ctx, 0, 0);
- return 0;
-}
-
-SEC("?tc")
-__failure __msg("arg#0 expected pointer to map value")
-int reject_kptr_get_no_null_map_val(struct __sk_buff *ctx)
-{
- bpf_kfunc_call_test_kptr_get(bpf_map_lookup_elem(&array_map, &(int){0}), 0, 0);
- return 0;
-}
-
-SEC("?tc")
-__failure __msg("arg#0 no referenced kptr at map value offset=0")
-int reject_kptr_get_no_kptr(struct __sk_buff *ctx)
-{
- struct map_value *v;
- int key = 0;
-
- v = bpf_map_lookup_elem(&array_map, &key);
- if (!v)
- return 0;
-
- bpf_kfunc_call_test_kptr_get((void *)v, 0, 0);
- return 0;
-}
-
-SEC("?tc")
-__failure __msg("arg#0 no referenced kptr at map value offset=8")
-int reject_kptr_get_on_unref(struct __sk_buff *ctx)
-{
- struct map_value *v;
- int key = 0;
-
- v = bpf_map_lookup_elem(&array_map, &key);
- if (!v)
- return 0;
-
- bpf_kfunc_call_test_kptr_get(&v->unref_ptr, 0, 0);
- return 0;
-}
-
-SEC("?tc")
-__failure __msg("kernel function bpf_kfunc_call_test_kptr_get args#0")
-int reject_kptr_get_bad_type_match(struct __sk_buff *ctx)
-{
- struct map_value *v;
- int key = 0;
-
- v = bpf_map_lookup_elem(&array_map, &key);
- if (!v)
- return 0;
-
- bpf_kfunc_call_test_kptr_get((void *)&v->ref_memb_ptr, 0, 0);
- return 0;
-}
-
SEC("?tc")
__failure __msg("R1 type=rcu_ptr_or_null_ expected=percpu_ptr_")
int mark_ref_as_untrusted_or_null(struct __sk_buff *ctx)
@@ -428,21 +365,6 @@ int kptr_xchg_ref_state(struct __sk_buff *ctx)
return 0;
}
-SEC("?tc")
-__failure __msg("Unreleased reference id=3 alloc_insn=")
-int kptr_get_ref_state(struct __sk_buff *ctx)
-{
- struct map_value *v;
- int key = 0;
-
- v = bpf_map_lookup_elem(&array_map, &key);
- if (!v)
- return 0;
-
- bpf_kfunc_call_test_kptr_get(&v->ref_ptr, 0, 0);
- return 0;
-}
-
SEC("?tc")
__failure __msg("Possibly NULL pointer passed to helper arg2")
int kptr_xchg_possibly_null(struct __sk_buff *ctx)
diff --git a/tools/testing/selftests/bpf/verifier/map_kptr.c b/tools/testing/selftests/bpf/verifier/map_kptr.c
index d775ccb01989..a0cfc06d75bc 100644
--- a/tools/testing/selftests/bpf/verifier/map_kptr.c
+++ b/tools/testing/selftests/bpf/verifier/map_kptr.c
@@ -288,33 +288,6 @@
.result = REJECT,
.errstr = "off=0 kptr isn't referenced kptr",
},
-{
- "map_kptr: unref: bpf_kfunc_call_test_kptr_get rejected",
- .insns = {
- BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
- BPF_LD_MAP_FD(BPF_REG_6, 0),
- BPF_MOV64_REG(BPF_REG_1, BPF_REG_6),
- BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -4),
- BPF_MOV64_IMM(BPF_REG_0, 0),
- BPF_ST_MEM(BPF_W, BPF_REG_2, 0, 0),
- BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_map_lookup_elem),
- BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
- BPF_EXIT_INSN(),
- BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
- BPF_MOV64_IMM(BPF_REG_2, 0),
- BPF_MOV64_IMM(BPF_REG_3, 0),
- BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, BPF_PSEUDO_KFUNC_CALL, 0, 0),
- BPF_MOV64_IMM(BPF_REG_0, 0),
- BPF_EXIT_INSN(),
- },
- .prog_type = BPF_PROG_TYPE_SCHED_CLS,
- .fixup_map_kptr = { 1 },
- .result = REJECT,
- .errstr = "arg#0 no referenced kptr at map value offset=0",
- .fixup_kfunc_btf_id = {
- { "bpf_kfunc_call_test_kptr_get", 13 },
- }
-},
/* Tests for referenced PTR_TO_BTF_ID */
{
"map_kptr: ref: loaded pointer marked as untrusted",
--
2.40.0
next prev parent reply other threads:[~2023-04-15 10:32 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-15 10:32 [PATCH bpf-next 0/3] Remove KF_KPTR_GET kfunc flag David Vernet
2023-04-15 10:32 ` David Vernet [this message]
2023-04-15 10:32 ` [PATCH bpf-next 2/3] bpf: " David Vernet
2023-04-15 23:08 ` Alexei Starovoitov
2023-04-15 10:32 ` [PATCH bpf-next 3/3] bpf,docs: Remove KF_KPTR_GET from documentation David Vernet
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=20230415103231.236063-2-void@manifault.com \
--to=void@manifault.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kernel-team@meta.com \
--cc=kpsingh@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=sdf@google.com \
--cc=song@kernel.org \
--cc=yhs@fb.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 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.