On Tue, 2023-12-12 at 15:25 -0800, Andrii Nakryiko wrote:
> 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 two initial special tags:
> - pointer to context;
> - non-null qualifier for generic pointer arguments.
>
> All of the above came up in practice already and seem generally useful
> additions. 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.
>
> Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> ---
Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
[...]
> @@ -6846,7 +6846,35 @@ 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;
Nit: personally, I'd keep tags parsing and processing logically separate:
- at this point set a flag 'is_ctx'
- and modify the check below as follows:
if (is_ctx || (btf_type_is_ptr(t) && btf_get_prog_ctx_type(log, btf, t, prog_type, i))) {
sub->args[i].arg_type = ARG_PTR_TO_CTX;
continue;
}
So that there is only one place where ARG_PTR_TO_CTX is assigned.
Feel free to ignore.
[...]
