From: Andrii Nakryiko <andrii@kernel.org>
To: <bpf@vger.kernel.org>, <ast@kernel.org>, <daniel@iogearbox.net>,
<martin.lau@kernel.org>
Cc: <andrii@kernel.org>, <kernel-team@meta.com>
Subject: [PATCH bpf-next 10/13] bpf: support 'arg:xxx' btf_decl_tag-based hints for global subprog args
Date: Mon, 4 Dec 2023 15:39:28 -0800 [thread overview]
Message-ID: <20231204233931.49758-11-andrii@kernel.org> (raw)
In-Reply-To: <20231204233931.49758-1-andrii@kernel.org>
Add support for annotating global BPF subprog arguments to provide more
information about expected semantics of the argument. Currently,
verifier relies purely on argument's BTF type information, and supports
three general use cases: scalar, pointer-to-context, and
pointer-to-fixed-size-memory.
Scalar and pointer-to-fixed-mem work well in practice and are quite
natural to use. But pointer-to-context is a bit problematic, as typical
BPF users don't realize that they need to use a special type name to
signal to verifier that argument is not just some pointer, but actually
a PTR_TO_CTX. Further, even if users do know which type to use, it is
limiting in situations where the same BPF program logic is used across
few different program types. Common case is kprobes, tracepoints, and
perf_event programs having a helper to send some data over BPF perf
buffer. bpf_perf_event_output() requires `ctx` argument, and so it's
quite cumbersome to share such global subprog across few BPF programs of
different types, necessitating extra static subprog that is context
type-agnostic.
Long story short, there is a need to go beyond types and allow users to
add hints to global subprog arguments to define expectations.
This patch adds such support for a few initial special tags:
- pointer to context;
- pointer to packet {data, metadata, end};
- non-null qualifier for generic pointer arguments.
All of the above came up in practice already and seem generally useful
additions. Pointer to pkt_{data,meta,end} are useful for networking
applications that don't use dynptrs yet. Non-null qualifier is an often
requested feature, which currently has to be worked around by having
unnecessary NULL checks inside subprogs even if we know that arguments
are never NULL. Pointer to context was discussed earlier.
As for implementation, we utilize btf_decl_tag attribute and set up an
"arg:xxx" convention to specify argument hint. As such:
- btf_decl_tag("arg:ctx") is a PTR_TO_CTX hint;
- btf_decl_tag("arg:nonnull") marks pointer argument as not allowed to
be NULL, making NULL check inside global subprog unnecessary;
- btf_decl_tag("arg:ptr_data"), btf_decl_tag("arg:ptr_meta"), and
btf_decl_tag("arg:ptr_end"), are marking arguments as special packet
pointers.
Please check arg:ptr_xxx checks especially carefully, I suspect we might
need some extra validation for them to always be correct. Note that we
also added ARG_PTR_TO_PACKET_{DATA,META,END} enumerators to enum bpf_arg_type
to accommodate them.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
include/linux/bpf.h | 4 ++++
kernel/bpf/btf.c | 56 ++++++++++++++++++++++++++++++++++++++-----
kernel/bpf/verifier.c | 32 ++++++++++++++++++++++++-
3 files changed, 85 insertions(+), 7 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 30d9b4599f63..d33168412afb 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -721,6 +721,10 @@ enum bpf_arg_type {
ARG_PTR_TO_TIMER, /* pointer to bpf_timer */
ARG_PTR_TO_KPTR, /* pointer to referenced kptr */
ARG_PTR_TO_DYNPTR, /* pointer to bpf_dynptr. See bpf_type_flag for dynptr type */
+
+ ARG_PTR_TO_PACKET_DATA,
+ ARG_PTR_TO_PACKET_META,
+ ARG_PTR_TO_PACKET_END,
__BPF_ARG_TYPE_MAX,
/* Extended arg_types. */
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 707af8d054e4..6b75774bfaae 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6781,7 +6781,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
enum bpf_prog_type prog_type = prog->type;
struct btf *btf = prog->aux->btf;
const struct btf_param *args;
- const struct btf_type *t, *ref_t;
+ const struct btf_type *t, *ref_t, *fn_t;
u32 i, nargs, btf_id;
const char *tname;
@@ -6801,8 +6801,8 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
return -EFAULT;
}
- t = btf_type_by_id(btf, btf_id);
- if (!t || !btf_type_is_func(t)) {
+ fn_t = btf_type_by_id(btf, btf_id);
+ if (!fn_t || !btf_type_is_func(fn_t)) {
/* These checks were already done by the verifier while loading
* struct bpf_func_info
*/
@@ -6810,7 +6810,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
subprog);
return -EFAULT;
}
- tname = btf_name_by_offset(btf, t->name_off);
+ tname = btf_name_by_offset(btf, fn_t->name_off);
if (prog->aux->func_info_aux[subprog].unreliable) {
bpf_log(log, "Verifier bug in function %s()\n", tname);
@@ -6819,7 +6819,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
if (prog_type == BPF_PROG_TYPE_EXT)
prog_type = prog->aux->dst_prog->type;
- t = btf_type_by_id(btf, t->type);
+ t = btf_type_by_id(btf, fn_t->type);
if (!t || !btf_type_is_func_proto(t)) {
bpf_log(log, "Invalid type of function %s()\n", tname);
return -EFAULT;
@@ -6845,7 +6845,47 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
* Only PTR_TO_CTX and SCALAR are supported atm.
*/
for (i = 0; i < nargs; i++) {
+ bool is_nonnull = false;
+ const char *tag;
+
t = btf_type_by_id(btf, args[i].type);
+
+ tag = btf_find_decl_tag_value(btf, fn_t, i, "arg:");
+ if (IS_ERR(tag) && PTR_ERR(tag) == -ENOENT) {
+ tag = NULL;
+ } else if (IS_ERR(tag)) {
+ bpf_log(log, "arg#%d type's tag fetching failure: %ld\n", i, PTR_ERR(tag));
+ return PTR_ERR(tag);
+ }
+ /* 'arg:<tag>' decl_tag takes precedence over derivation of
+ * register type from BTF type itself
+ */
+ if (tag) {
+ /* disallow arg tags in static subprogs */
+ if (!is_global) {
+ bpf_log(log, "arg#%d type tag is not supported in static functions\n", i);
+ return -EOPNOTSUPP;
+ }
+ if (strcmp(tag, "ctx") == 0) {
+ sub->args[i].arg_type = ARG_PTR_TO_CTX;
+ continue;
+ }
+ if (strcmp(tag, "pkt_meta") == 0) {
+ sub->args[i].arg_type = ARG_PTR_TO_PACKET_META;
+ continue;
+ }
+ if (strcmp(tag, "pkt_data") == 0) {
+ sub->args[i].arg_type = ARG_PTR_TO_PACKET_DATA;
+ continue;
+ }
+ if (strcmp(tag, "pkt_end") == 0) {
+ sub->args[i].arg_type = ARG_PTR_TO_PACKET_END;
+ continue;
+ }
+ if (strcmp(tag, "nonnull") == 0)
+ is_nonnull = true;
+ }
+
while (btf_type_is_modifier(t))
t = btf_type_by_id(btf, t->type);
if (btf_type_is_int(t) || btf_is_any_enum(t)) {
@@ -6869,10 +6909,14 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
return -EINVAL;
}
- sub->args[i].arg_type = ARG_PTR_TO_MEM_OR_NULL;
+ sub->args[i].arg_type = is_nonnull ? ARG_PTR_TO_MEM : ARG_PTR_TO_MEM_OR_NULL;
sub->args[i].mem_size = mem_size;
continue;
}
+ if (is_nonnull) {
+ bpf_log(log, "arg#%d marked as non-null, but is not a pointer type\n", i);
+ return -EINVAL;
+ }
bpf_log(log, "Arg#%d type %s in %s() is not supported yet.\n",
i, btf_type_str(t), tname);
return -EINVAL;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 5787b7fd16ba..61e778dbde10 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9268,9 +9268,30 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
ret = check_func_arg_reg_off(env, reg, regno, ARG_DONTCARE);
if (ret < 0)
return ret;
-
if (check_mem_reg(env, reg, regno, arg->mem_size))
return -EINVAL;
+ if (!(arg->arg_type & PTR_MAYBE_NULL) && (reg->type & PTR_MAYBE_NULL)) {
+ bpf_log(log, "arg#%d is expected to be non-NULL\n", i);
+ return -EINVAL;
+ }
+ } else if (arg->arg_type == ARG_PTR_TO_PACKET_META) {
+ if (reg->type != PTR_TO_PACKET_META) {
+ bpf_log(log, "arg#%d expected pkt_meta, but got %s\n",
+ i, reg_type_str(env, reg->type));
+ return -EINVAL;
+ }
+ } else if (arg->arg_type == ARG_PTR_TO_PACKET_DATA) {
+ if (reg->type != PTR_TO_PACKET) {
+ bpf_log(log, "arg#%d expected pkt, but got %s\n",
+ i, reg_type_str(env, reg->type));
+ return -EINVAL;
+ }
+ } else if (arg->arg_type == ARG_PTR_TO_PACKET_END) {
+ if (reg->type != PTR_TO_PACKET_END) {
+ bpf_log(log, "arg#%d expected pkt_end, but got %s\n",
+ i, reg_type_str(env, reg->type));
+ return -EINVAL;
+ }
} else {
bpf_log(log, "verifier bug: unrecognized arg#%d type %d\n",
i, arg->arg_type);
@@ -19983,6 +20004,15 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
} else if (arg->arg_type == ARG_SCALAR) {
reg->type = SCALAR_VALUE;
mark_reg_unknown(env, regs, i);
+ } else if (arg->arg_type == ARG_PTR_TO_PACKET_META) {
+ reg->type = PTR_TO_PACKET_META;
+ mark_reg_known_zero(env, regs, i);
+ } else if (arg->arg_type == ARG_PTR_TO_PACKET_DATA) {
+ reg->type = PTR_TO_PACKET;
+ mark_reg_known_zero(env, regs, i);
+ } else if (arg->arg_type == ARG_PTR_TO_PACKET_END) {
+ reg->type = PTR_TO_PACKET_END;
+ mark_reg_known_zero(env, regs, i);
} else if (base_type(arg->arg_type) == ARG_PTR_TO_MEM) {
reg->type = PTR_TO_MEM;
if (arg->arg_type & PTR_MAYBE_NULL)
--
2.34.1
next prev parent reply other threads:[~2023-12-04 23:43 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-04 23:39 [PATCH bpf-next 00/13] Enhance BPF global subprogs with argument tags Andrii Nakryiko
2023-12-04 23:39 ` [PATCH bpf-next 01/13] bpf: log PTR_TO_MEM memory size in verifier log Andrii Nakryiko
2023-12-05 23:23 ` Eduard Zingerman
2023-12-04 23:39 ` [PATCH bpf-next 02/13] bpf: emit more dynptr information " Andrii Nakryiko
2023-12-05 23:24 ` Eduard Zingerman
2023-12-04 23:39 ` [PATCH bpf-next 03/13] bpf: tidy up exception callback management a bit Andrii Nakryiko
2023-12-05 23:25 ` Eduard Zingerman
2023-12-06 17:59 ` Andrii Nakryiko
2023-12-04 23:39 ` [PATCH bpf-next 04/13] bpf: use bitfields for simple per-subprog bool flags Andrii Nakryiko
2023-12-05 23:25 ` Eduard Zingerman
2023-12-04 23:39 ` [PATCH bpf-next 05/13] bpf: abstract away global subprog arg preparation logic from reg state setup Andrii Nakryiko
2023-12-05 23:21 ` Eduard Zingerman
2023-12-06 17:59 ` Andrii Nakryiko
2023-12-04 23:39 ` [PATCH bpf-next 06/13] bpf: remove unnecessary and (mostly) ignored BTF check for main program Andrii Nakryiko
2023-12-05 23:21 ` Eduard Zingerman
2023-12-06 17:59 ` Andrii Nakryiko
2023-12-06 18:05 ` Eduard Zingerman
2023-12-04 23:39 ` [PATCH bpf-next 07/13] bpf: prepare btf_prepare_func_args() for handling static subprogs Andrii Nakryiko
2023-12-05 23:26 ` Eduard Zingerman
2023-12-04 23:39 ` [PATCH bpf-next 08/13] bpf: move subprog call logic back to verifier.c Andrii Nakryiko
2023-12-05 8:01 ` kernel test robot
2023-12-05 18:57 ` Andrii Nakryiko
2023-12-05 9:04 ` kernel test robot
2023-12-05 11:46 ` kernel test robot
2023-12-05 23:27 ` Eduard Zingerman
2023-12-04 23:39 ` [PATCH bpf-next 09/13] bpf: reuse subprog argument parsing logic for subprog call checks Andrii Nakryiko
2023-12-05 10:21 ` kernel test robot
2023-12-05 11:25 ` kernel test robot
2023-12-05 23:21 ` Eduard Zingerman
2023-12-06 18:05 ` Andrii Nakryiko
2023-12-04 23:39 ` Andrii Nakryiko [this message]
2023-12-05 23:22 ` [PATCH bpf-next 10/13] bpf: support 'arg:xxx' btf_decl_tag-based hints for global subprog args Eduard Zingerman
2023-12-06 18:15 ` Andrii Nakryiko
2023-12-06 18:47 ` Eduard Zingerman
2023-12-04 23:39 ` [PATCH bpf-next 11/13] bpf: add dynptr global subprog arg tag support Andrii Nakryiko
2023-12-05 23:22 ` Eduard Zingerman
2023-12-06 18:17 ` Andrii Nakryiko
2023-12-04 23:39 ` [PATCH bpf-next 12/13] libbpf: add __arg_xxx macros for annotating global func args Andrii Nakryiko
2023-12-04 23:39 ` [PATCH bpf-next 13/13] selftests/bpf: add global subprog annotation tests Andrii Nakryiko
2023-12-05 23:29 ` Eduard Zingerman
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=20231204233931.49758-11-andrii@kernel.org \
--to=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@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