From: Kumar Kartikeya Dwivedi <memxor@gmail.com>
To: bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Martin KaFai Lau <martin.lau@kernel.org>,
Dave Marchevsky <davemarchevsky@meta.com>
Subject: [PATCH bpf-next v9 08/23] bpf: Allow locking bpf_spin_lock in allocated objects
Date: Fri, 18 Nov 2022 04:24:55 +0530 [thread overview]
Message-ID: <20221117225510.1676785-9-memxor@gmail.com> (raw)
In-Reply-To: <20221117225510.1676785-1-memxor@gmail.com>
Allow locking a bpf_spin_lock in an allocated object, in addition to
already supported map value pointers. The handling is similar to that of
map values, by just preserving the reg->id of PTR_TO_BTF_ID | MEM_ALLOC
as well, and adjusting process_spin_lock to work with them and remember
the id in verifier state.
Refactor the existing process_spin_lock to work with PTR_TO_BTF_ID |
MEM_ALLOC in addition to PTR_TO_MAP_VALUE. We need to update the
reg_may_point_to_spin_lock which is used in mark_ptr_or_null_reg to
preserve reg->id, that will be used in env->cur_state->active_spin_lock
to remember the currently held spin lock.
Also update the comment describing bpf_spin_lock implementation details
to also talk about PTR_TO_BTF_ID | MEM_ALLOC type.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
kernel/bpf/helpers.c | 2 +
kernel/bpf/verifier.c | 89 +++++++++++++++++++++++++++++++------------
2 files changed, 66 insertions(+), 25 deletions(-)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 7bc71995f17c..5bc0b9f0f306 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -336,6 +336,7 @@ const struct bpf_func_proto bpf_spin_lock_proto = {
.gpl_only = false,
.ret_type = RET_VOID,
.arg1_type = ARG_PTR_TO_SPIN_LOCK,
+ .arg1_btf_id = BPF_PTR_POISON,
};
static inline void __bpf_spin_unlock_irqrestore(struct bpf_spin_lock *lock)
@@ -358,6 +359,7 @@ const struct bpf_func_proto bpf_spin_unlock_proto = {
.gpl_only = false,
.ret_type = RET_VOID,
.arg1_type = ARG_PTR_TO_SPIN_LOCK,
+ .arg1_btf_id = BPF_PTR_POISON,
};
void copy_map_value_locked(struct bpf_map *map, void *dst, void *src,
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 49e08c1c2c61..8eddecfc3a5e 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -453,8 +453,16 @@ static bool reg_type_not_null(enum bpf_reg_type type)
static bool reg_may_point_to_spin_lock(const struct bpf_reg_state *reg)
{
- return reg->type == PTR_TO_MAP_VALUE &&
- btf_record_has_field(reg->map_ptr->record, BPF_SPIN_LOCK);
+ struct btf_record *rec = NULL;
+
+ if (reg->type == PTR_TO_MAP_VALUE) {
+ rec = reg->map_ptr->record;
+ } else if (reg->type == (PTR_TO_BTF_ID | MEM_ALLOC)) {
+ struct btf_struct_meta *meta = btf_find_struct_meta(reg->btf, reg->btf_id);
+ if (meta)
+ rec = meta->record;
+ }
+ return btf_record_has_field(rec, BPF_SPIN_LOCK);
}
static bool type_is_rdonly_mem(u32 type)
@@ -5564,23 +5572,26 @@ int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state
}
/* Implementation details:
- * bpf_map_lookup returns PTR_TO_MAP_VALUE_OR_NULL
+ * bpf_map_lookup returns PTR_TO_MAP_VALUE_OR_NULL.
+ * bpf_obj_new returns PTR_TO_BTF_ID | MEM_ALLOC | PTR_MAYBE_NULL.
* Two bpf_map_lookups (even with the same key) will have different reg->id.
- * For traditional PTR_TO_MAP_VALUE the verifier clears reg->id after
- * value_or_null->value transition, since the verifier only cares about
- * the range of access to valid map value pointer and doesn't care about actual
- * address of the map element.
+ * Two separate bpf_obj_new will also have different reg->id.
+ * For traditional PTR_TO_MAP_VALUE or PTR_TO_BTF_ID | MEM_ALLOC, the verifier
+ * clears reg->id after value_or_null->value transition, since the verifier only
+ * cares about the range of access to valid map value pointer and doesn't care
+ * about actual address of the map element.
* For maps with 'struct bpf_spin_lock' inside map value the verifier keeps
* reg->id > 0 after value_or_null->value transition. By doing so
* two bpf_map_lookups will be considered two different pointers that
- * point to different bpf_spin_locks.
+ * point to different bpf_spin_locks. Likewise for pointers to allocated objects
+ * returned from bpf_obj_new.
* The verifier allows taking only one bpf_spin_lock at a time to avoid
* dead-locks.
* Since only one bpf_spin_lock is allowed the checks are simpler than
* reg_is_refcounted() logic. The verifier needs to remember only
* one spin_lock instead of array of acquired_refs.
- * cur_state->active_spin_lock remembers which map value element got locked
- * and clears it after bpf_spin_unlock.
+ * cur_state->active_spin_lock remembers which map value element or allocated
+ * object got locked and clears it after bpf_spin_unlock.
*/
static int process_spin_lock(struct bpf_verifier_env *env, int regno,
bool is_lock)
@@ -5588,8 +5599,10 @@ static int process_spin_lock(struct bpf_verifier_env *env, int regno,
struct bpf_reg_state *regs = cur_regs(env), *reg = ®s[regno];
struct bpf_verifier_state *cur = env->cur_state;
bool is_const = tnum_is_const(reg->var_off);
- struct bpf_map *map = reg->map_ptr;
+ struct btf_record *rec = NULL;
u64 val = reg->var_off.value;
+ struct bpf_map *map = NULL;
+ struct btf *btf = NULL;
if (!is_const) {
verbose(env,
@@ -5597,19 +5610,32 @@ static int process_spin_lock(struct bpf_verifier_env *env, int regno,
regno);
return -EINVAL;
}
- if (!map->btf) {
- verbose(env,
- "map '%s' has to have BTF in order to use bpf_spin_lock\n",
- map->name);
- return -EINVAL;
+ if (reg->type == PTR_TO_MAP_VALUE) {
+ map = reg->map_ptr;
+ if (!map->btf) {
+ verbose(env,
+ "map '%s' has to have BTF in order to use bpf_spin_lock\n",
+ map->name);
+ return -EINVAL;
+ }
+ rec = map->record;
+ } else {
+ struct btf_struct_meta *meta;
+
+ btf = reg->btf;
+ meta = btf_find_struct_meta(reg->btf, reg->btf_id);
+ if (meta)
+ rec = meta->record;
}
- if (!btf_record_has_field(map->record, BPF_SPIN_LOCK)) {
- verbose(env, "map '%s' has no valid bpf_spin_lock\n", map->name);
+
+ if (!btf_record_has_field(rec, BPF_SPIN_LOCK)) {
+ verbose(env, "%s '%s' has no valid bpf_spin_lock\n", map ? "map" : "local",
+ map ? map->name : "kptr");
return -EINVAL;
}
- if (map->record->spin_lock_off != val + reg->off) {
+ if (rec->spin_lock_off != val + reg->off) {
verbose(env, "off %lld doesn't point to 'struct bpf_spin_lock' that is at %d\n",
- val + reg->off, map->record->spin_lock_off);
+ val + reg->off, rec->spin_lock_off);
return -EINVAL;
}
if (is_lock) {
@@ -5815,13 +5841,19 @@ static const struct bpf_reg_types int_ptr_types = {
},
};
+static const struct bpf_reg_types spin_lock_types = {
+ .types = {
+ PTR_TO_MAP_VALUE,
+ PTR_TO_BTF_ID | MEM_ALLOC,
+ }
+};
+
static const struct bpf_reg_types fullsock_types = { .types = { PTR_TO_SOCKET } };
static const struct bpf_reg_types scalar_types = { .types = { SCALAR_VALUE } };
static const struct bpf_reg_types context_types = { .types = { PTR_TO_CTX } };
static const struct bpf_reg_types ringbuf_mem_types = { .types = { PTR_TO_MEM | MEM_RINGBUF } };
static const struct bpf_reg_types const_map_ptr_types = { .types = { CONST_PTR_TO_MAP } };
static const struct bpf_reg_types btf_ptr_types = { .types = { PTR_TO_BTF_ID } };
-static const struct bpf_reg_types spin_lock_types = { .types = { PTR_TO_MAP_VALUE } };
static const struct bpf_reg_types percpu_btf_ptr_types = { .types = { PTR_TO_BTF_ID | MEM_PERCPU } };
static const struct bpf_reg_types func_ptr_types = { .types = { PTR_TO_FUNC } };
static const struct bpf_reg_types stack_ptr_types = { .types = { PTR_TO_STACK } };
@@ -5946,6 +5978,11 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno,
return -EACCES;
}
}
+ } else if (type_is_alloc(reg->type)) {
+ if (meta->func_id != BPF_FUNC_spin_lock && meta->func_id != BPF_FUNC_spin_unlock) {
+ verbose(env, "verifier internal error: unimplemented handling of MEM_ALLOC\n");
+ return -EFAULT;
+ }
}
return 0;
@@ -6062,7 +6099,8 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
goto skip_type_check;
/* arg_btf_id and arg_size are in a union. */
- if (base_type(arg_type) == ARG_PTR_TO_BTF_ID)
+ if (base_type(arg_type) == ARG_PTR_TO_BTF_ID ||
+ base_type(arg_type) == ARG_PTR_TO_SPIN_LOCK)
arg_btf_id = fn->arg_btf_id[arg];
err = check_reg_type(env, regno, arg_type, arg_btf_id, meta);
@@ -6680,9 +6718,10 @@ static bool check_btf_id_ok(const struct bpf_func_proto *fn)
int i;
for (i = 0; i < ARRAY_SIZE(fn->arg_type); i++) {
- if (base_type(fn->arg_type[i]) == ARG_PTR_TO_BTF_ID && !fn->arg_btf_id[i])
- return false;
-
+ if (base_type(fn->arg_type[i]) == ARG_PTR_TO_BTF_ID)
+ return !!fn->arg_btf_id[i];
+ if (base_type(fn->arg_type[i]) == ARG_PTR_TO_SPIN_LOCK)
+ return fn->arg_btf_id[i] == BPF_PTR_POISON;
if (base_type(fn->arg_type[i]) != ARG_PTR_TO_BTF_ID && fn->arg_btf_id[i] &&
/* arg_btf_id and arg_size are in a union. */
(base_type(fn->arg_type[i]) != ARG_PTR_TO_MEM ||
--
2.38.1
next prev parent reply other threads:[~2022-11-17 22:56 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-17 22:54 [PATCH bpf-next v9 00/23] Allocated objects, BPF linked lists Kumar Kartikeya Dwivedi
2022-11-17 22:54 ` [PATCH bpf-next v9 01/23] bpf: Fix early return in map_check_btf Kumar Kartikeya Dwivedi
2022-11-17 22:54 ` [PATCH bpf-next v9 02/23] bpf: Do btf_record_free outside map_free callback Kumar Kartikeya Dwivedi
2022-11-17 22:54 ` [PATCH bpf-next v9 03/23] bpf: Free inner_map_meta when btf_record_dup fails Kumar Kartikeya Dwivedi
2022-11-17 22:54 ` [PATCH bpf-next v9 04/23] bpf: Populate field_offs for inner_map_meta Kumar Kartikeya Dwivedi
2022-11-17 22:54 ` [PATCH bpf-next v9 05/23] bpf: Introduce allocated objects support Kumar Kartikeya Dwivedi
2022-11-17 22:54 ` [PATCH bpf-next v9 06/23] bpf: Recognize lock and list fields in allocated objects Kumar Kartikeya Dwivedi
2022-11-17 22:54 ` [PATCH bpf-next v9 07/23] bpf: Verify ownership relationships for user BTF types Kumar Kartikeya Dwivedi
2022-11-17 22:54 ` Kumar Kartikeya Dwivedi [this message]
2022-11-17 22:54 ` [PATCH bpf-next v9 09/23] bpf: Allow locking bpf_spin_lock global variables Kumar Kartikeya Dwivedi
2022-11-17 22:54 ` [PATCH bpf-next v9 10/23] bpf: Allow locking bpf_spin_lock in inner map values Kumar Kartikeya Dwivedi
2022-11-17 22:54 ` [PATCH bpf-next v9 11/23] bpf: Rewrite kfunc argument handling Kumar Kartikeya Dwivedi
2022-11-17 22:54 ` [PATCH bpf-next v9 12/23] bpf: Support constant scalar arguments for kfuncs Kumar Kartikeya Dwivedi
2022-11-17 22:55 ` [PATCH bpf-next v9 13/23] bpf: Introduce bpf_obj_new Kumar Kartikeya Dwivedi
2022-11-17 22:55 ` [PATCH bpf-next v9 14/23] bpf: Introduce bpf_obj_drop Kumar Kartikeya Dwivedi
2022-11-17 22:55 ` [PATCH bpf-next v9 15/23] bpf: Permit NULL checking pointer with non-zero fixed offset Kumar Kartikeya Dwivedi
2022-11-17 22:55 ` [PATCH bpf-next v9 16/23] bpf: Introduce single ownership BPF linked list API Kumar Kartikeya Dwivedi
2022-11-18 0:12 ` Alexei Starovoitov
2022-11-17 22:55 ` [PATCH bpf-next v9 17/23] bpf: Add 'release on unlock' logic for bpf_list_push_{front,back} Kumar Kartikeya Dwivedi
2022-11-17 22:55 ` [PATCH bpf-next v9 18/23] bpf: Add comments for map BTF matching requirement for bpf_list_head Kumar Kartikeya Dwivedi
2022-11-17 22:55 ` [PATCH bpf-next v9 19/23] selftests/bpf: Add __contains macro to bpf_experimental.h Kumar Kartikeya Dwivedi
2022-11-17 22:55 ` [PATCH bpf-next v9 20/23] selftests/bpf: Update spinlock selftest Kumar Kartikeya Dwivedi
2022-11-17 22:55 ` [PATCH bpf-next v9 21/23] selftests/bpf: Add failure test cases for spin lock pairing Kumar Kartikeya Dwivedi
2022-11-17 22:55 ` [PATCH bpf-next v9 22/23] selftests/bpf: Add BPF linked list API tests Kumar Kartikeya Dwivedi
2022-11-17 23:05 ` Alexei Starovoitov
2022-11-17 23:27 ` Dave Marchevsky
2022-11-17 23:58 ` Kumar Kartikeya Dwivedi
2022-11-18 0:05 ` Alexei Starovoitov
2022-11-17 23:29 ` Yonghong Song
2022-11-17 22:55 ` [PATCH bpf-next v9 23/23] selftests/bpf: Add BTF sanity tests 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=20221117225510.1676785-9-memxor@gmail.com \
--to=memxor@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=davemarchevsky@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