Add a few fields for each arg (argN_release) that when set to true,
tells verifier that for a release function, that argument's register
will be the one for which meta.ref_obj_id will be set, and which will
then be released using release_reference. To capture the regno,
introduce a release_regno field in bpf_call_arg_meta.
This would be required in the next patch, where we may either pass NULL
or a refcounted pointer as an argument to the release function
bpf_kptr_xchg. Just releasing only when meta.ref_obj_id is set is not
enough, as there is a case where the type of argument needed matches,
but the ref_obj_id is set to 0. Hence, we must enforce that whenever
meta.ref_obj_id is zero, the register that is to be released can only
be NULL for a release function.
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
---
include/linux/bpf.h | 10 ++++++++++
kernel/bpf/ringbuf.c | 2 ++
kernel/bpf/verifier.c | 39 +++++++++++++++++++++++++++++++++------
net/core/filter.c | 1 +
4 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index f35920d279dd..48ddde854d67 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -487,6 +487,16 @@ struct bpf_func_proto {
};
u32 *arg_btf_id[5];
};
+ union {
+ struct {
+ bool arg1_release;
+ bool arg2_release;
+ bool arg3_release;
+ bool arg4_release;
+ bool arg5_release;
+ };
+ bool arg_release[5];
+ };
int *ret_btf_id; /* return value btf_id */
bool (*allowed)(const struct bpf_prog *prog);
};
diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
index 710ba9de12ce..f40ce718630e 100644
--- a/kernel/bpf/ringbuf.c
+++ b/kernel/bpf/ringbuf.c
@@ -405,6 +405,7 @@ const struct bpf_func_proto bpf_ringbuf_submit_proto = {
.func = bpf_ringbuf_submit,
.ret_type = RET_VOID,
.arg1_type = ARG_PTR_TO_ALLOC_MEM,
+ .arg1_release = true,
.arg2_type = ARG_ANYTHING,
};
@@ -418,6 +419,7 @@ const struct bpf_func_proto bpf_ringbuf_discard_proto = {
.func = bpf_ringbuf_discard,
.ret_type = RET_VOID,
.arg1_type = ARG_PTR_TO_ALLOC_MEM,
+ .arg1_release = true,
.arg2_type = ARG_ANYTHING,
};
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 744b7362e52e..b8cd34607215 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -245,6 +245,7 @@ struct bpf_call_arg_meta {
struct bpf_map *map_ptr;
bool raw_mode;
bool pkt_access;
+ u8 release_regno;
int regno;
int access_size;
int mem_size;
@@ -6101,12 +6102,31 @@ static bool check_btf_id_ok(const struct bpf_func_proto *fn)
return true;
}
-static int check_func_proto(const struct bpf_func_proto *fn, int func_id)
+static bool check_release_regno(const struct bpf_func_proto *fn, int func_id,
+ struct bpf_call_arg_meta *meta)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(fn->arg_release); i++) {
+ if (fn->arg_release[i]) {
+ if (!is_release_function(func_id))
+ return false;
+ if (meta->release_regno)
+ return false;
+ meta->release_regno = i + 1;
+ }
+ }
+ return !is_release_function(func_id) || meta->release_regno;
+}
+
+static int check_func_proto(const struct bpf_func_proto *fn, int func_id,
+ struct bpf_call_arg_meta *meta)
{
return check_raw_mode_ok(fn) &&
check_arg_pair_ok(fn) &&
check_btf_id_ok(fn) &&
- check_refcount_ok(fn, func_id) ? 0 : -EINVAL;
+ check_refcount_ok(fn, func_id) &&
+ check_release_regno(fn, func_id, meta) ? 0 : -EINVAL;
}
/* Packet data might have moved, any old PTR_TO_PACKET[_META,_END]
@@ -6785,7 +6805,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
memset(&meta, 0, sizeof(meta));
meta.pkt_access = fn->pkt_access;
- err = check_func_proto(fn, func_id);
+ err = check_func_proto(fn, func_id, &meta);
if (err) {
verbose(env, "kernel subsystem misconfigured func %s#%d\n",
func_id_name(func_id), func_id);
@@ -6818,8 +6838,17 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
return err;
}
+ regs = cur_regs(env);
+
if (is_release_function(func_id)) {
- err = release_reference(env, meta.ref_obj_id);
+ err = -EINVAL;
+ if (meta.ref_obj_id)
+ err = release_reference(env, meta.ref_obj_id);
+ /* meta.ref_obj_id can only be 0 if register that is meant to be
+ * released is NULL, which must be > R0.
+ */
+ else if (meta.release_regno && register_is_null(®s[meta.release_regno]))
+ err = 0;
if (err) {
verbose(env, "func %s#%d reference has not been acquired before\n",
func_id_name(func_id), func_id);
@@ -6827,8 +6856,6 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
}
}
- regs = cur_regs(env);
-
switch (func_id) {
case BPF_FUNC_tail_call:
err = check_reference_leak(env);
diff --git a/net/core/filter.c b/net/core/filter.c
index 03655f2074ae..17eff4731b06 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -6622,6 +6622,7 @@ static const struct bpf_func_proto bpf_sk_release_proto = {
.gpl_only = false,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_BTF_ID_SOCK_COMMON,
+ .arg1_release = true,
};
BPF_CALL_5(bpf_xdp_sk_lookup_udp, struct xdp_buff *, ctx,
--
2.35.1
