[bpf-next,11/13] bpf: add dynptr global subprog arg tag support

Commit Message

Andrii Nakryiko Dec. 4, 2023, 11:39 p.m. UTC

Add ability to pass a pointer to dynptr for global functions by using
btf_decl_tag("arg:dynptr") tag hint. This allows to have global subprogs
that accept and work with generic dynptrs that are created by caller.

This is conceptually exactly the same semantics as
bpf_user_ringbuf_drain()'s use of dynptr to pass a variable-sized
pointer to ringbuf record. So we heavily rely on CONST_PTR_TO_DYNPTR
bits of already existing logic in the verifier.

During global subprog validation, we mark such CONST_PTR_TO_DYNPTR as
having LOCAL type, as that's the most unassuming type of dynptr and it
doesn't have any special helpers that can try to free or acquire extra
references (unlike skb, xdp, or ringbuf dynptr). So that seems like a safe
"choise" to make from correctness standpoint. It's still possible to
pass any type of dynptr to such subprog, though, because generic dynptr
helpers, like getting data/slice pointers, read/write memory copying
routines, dynptr adjustment and getter routines all work correctly with
any type of dynptr.

Signed-off-by: Andrii Nakryiko <[e-mail]>
---
 kernel/bpf/btf.c      | 4 ++++
 kernel/bpf/verifier.c | 7 +++++++
 2 files changed, 11 insertions(+)

Comments

Eduard Zingerman Dec. 5, 2023, 11:22 p.m. UTC | #1

On Mon, 2023-12-04 at 15:39 -0800, Andrii Nakryiko wrote:
> Add ability to pass a pointer to dynptr for global functions by using
> btf_decl_tag("arg:dynptr") tag hint. This allows to have global subprogs
> that accept and work with generic dynptrs that are created by caller.

Why is this preferable to a check that parameter's BTF type is
STRUCT 'bpf_dynptr'?

[...]

Andrii Nakryiko Dec. 6, 2023, 6:17 p.m. UTC | #2

On Tue, Dec 5, 2023 at 3:22 PM Eduard Zingerman <[e-mail]> wrote:
>
> On Mon, 2023-12-04 at 15:39 -0800, Andrii Nakryiko wrote:
> > Add ability to pass a pointer to dynptr for global functions by using
> > btf_decl_tag("arg:dynptr") tag hint. This allows to have global subprogs
> > that accept and work with generic dynptrs that are created by caller.
>
> Why is this preferable to a check that parameter's BTF type is
> STRUCT 'bpf_dynptr'?
>

For `struct bpf_dynptr *` I think it's acceptable to guess that it's a
CONST_PTR_TO_DYNPTR instead of requiring tagging, there is little
chance for user's application to have their own data struct called
bpf_dynptr. I can make a change to infer this from the type name then.


> [...]
>
>

Patch

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 6b75774bfaae..06684e77eb43 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6870,6 +6870,10 @@  int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 				sub->args[i].arg_type = ARG_PTR_TO_CTX;
 				continue;
 			}
+			if (strcmp(tag, "dynptr") == 0) {
+				sub->args[i].arg_type = ARG_PTR_TO_DYNPTR | MEM_RDONLY;
+				continue;
+			}
 			if (strcmp(tag, "pkt_meta") == 0) {
 				sub->args[i].arg_type = ARG_PTR_TO_PACKET_META;
 				continue;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 61e778dbde10..e08677c0629c 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9292,6 +9292,10 @@  static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
 					i, reg_type_str(env, reg->type));
 				return -EINVAL;
 			}
+		} else if (arg->arg_type == (ARG_PTR_TO_DYNPTR | MEM_RDONLY)) {
+			ret = process_dynptr_func(env, regno, -1, arg->arg_type, 0);
+			if (ret)
+				return ret;
 		} else {
 			bpf_log(log, "verifier bug: unrecognized arg#%d type %d\n",
 				i, arg->arg_type);
@@ -20013,6 +20017,9 @@  static int do_check_common(struct bpf_verifier_env *env, int subprog)
 			} 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 (arg->arg_type == (ARG_PTR_TO_DYNPTR | MEM_RDONLY)) {
+				/* assume unspecial LOCAL dynptr type */
+				__mark_dynptr_reg(reg, BPF_DYNPTR_TYPE_LOCAL, true, ++env->id_gen);
 			} else if (base_type(arg->arg_type) == ARG_PTR_TO_MEM) {
 				reg->type = PTR_TO_MEM;
 				if (arg->arg_type & PTR_MAYBE_NULL)