[RFC PATCH bpf-next 0/3] bpf: Implement bpf_get_kern_btf_id() kfunc

[RFC PATCH bpf-next 0/3] bpf: Implement bpf_get_kern_btf_id() kfunc

[Yonghong Song]

Currenty, a non-tracing bpf program typically has a single 'context' argument
with predefined uapi struct type. Following these uapi struct, user is able
to access other fields defined in uapi header. Inside the kernel, the
user-seen 'context' argument is replaced with 'kernel context' (or 'kcontext'
in short) which can access more information than what uapi header provides.
To access other info not in uapi header, people typically do two things:
  (1). extend uapi to access more fields rooted from 'context'.
  (2). use bpf_probe_read_kernl() helper to read particular field based on
    kcontext.
Using (1) needs uapi change and using (2) makes code more complex since
direct memory access is not allowed.

There are already a few instances trying to access more information from
kcontext:
  (1). trying to access some fields from perf_event kcontext.
  (2). trying to access some fields from xdp kcontext.

This patch set tried to allow direct memory access for kcontext fields
by introducing bpf_get_kern_btf_id() kfunc.

Martin mentioned a use case like type casting below:
  #define skb_shinfo(SKB) ((struct skb_shared_info *)(skb_end_pointer(SKB)))
basically a 'unsigned char *" casted to 'struct skb_shared_info *'. This patch
set tries to support such a use case as well with bpf_get_kern_btf_id().

For the patch series, Patch 1 is a preparation patch. Patch 2 did some
but incomplete implementation. Please see details in Patch 2 for what
is missing. The goal of this RFC patch is to seek comments about whether
such a kfunc is helpful and what scope (e.g., type casting like skb_shinfo(SKB)
can be implemented with this helper) it should cover. Patch 3 has two
simple tests.

  [1] https://lore.kernel.org/bpf/ad15b398-9069-4a0e-48cb-4bb651ec3088@meta.com/
  [2] https://lore.kernel.org/bpf/20221109215242.1279993-1-john.fastabend@gmail.com/

Yonghong Song (3):
  bpf: Add support for kfunc set with generic btf_ids
  bpf: Implement bpf_get_kern_btf_id() kfunc
  bpf: Add bpf_get_kern_btf_id() tests

 include/linux/bpf.h                           |  2 +
 kernel/bpf/btf.c                              | 75 ++++++++++++++++-
 kernel/bpf/helpers.c                          | 18 ++++-
 kernel/bpf/verifier.c                         |  8 +-
 .../bpf/prog_tests/get_kern_btf_id.c          | 81 +++++++++++++++++++
 .../selftests/bpf/progs/get_kern_btf_id.c     | 44 ++++++++++
 6 files changed, 225 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/get_kern_btf_id.c
 create mode 100644 tools/testing/selftests/bpf/progs/get_kern_btf_id.c

-- 
2.30.2

[RFC PATCH bpf-next 1/3] bpf: Add support for kfunc set with generic btf_ids

[Yonghong Song]

Later on, we will introduce a kfunc bpf_get_kern_btf_id() which
applies to all program types. Currently kfunc set only supports
individual prog types. This patch added support for a kfunc
applying to all program types. The implementation is identical
to [1].

  [1] https://lore.kernel.org/bpf/20221111165750.2526371-1-yhs@fb.com/

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 kernel/bpf/btf.c     |  8 ++++++++
 kernel/bpf/helpers.c | 13 ++++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 5579ff3a5b54..84f09235857c 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -199,6 +199,7 @@ DEFINE_IDR(btf_idr);
 DEFINE_SPINLOCK(btf_idr_lock);
 
 enum btf_kfunc_hook {
+	BTF_KFUNC_HOOK_GENERIC,
 	BTF_KFUNC_HOOK_XDP,
 	BTF_KFUNC_HOOK_TC,
 	BTF_KFUNC_HOOK_STRUCT_OPS,
@@ -7499,6 +7500,8 @@ static u32 *__btf_kfunc_id_set_contains(const struct btf *btf,
 static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
 {
 	switch (prog_type) {
+	case BPF_PROG_TYPE_UNSPEC:
+		return BTF_KFUNC_HOOK_GENERIC;
 	case BPF_PROG_TYPE_XDP:
 		return BTF_KFUNC_HOOK_XDP;
 	case BPF_PROG_TYPE_SCHED_CLS:
@@ -7527,6 +7530,11 @@ u32 *btf_kfunc_id_set_contains(const struct btf *btf,
 			       u32 kfunc_btf_id)
 {
 	enum btf_kfunc_hook hook;
+	u32 *kfunc_flags;
+
+	kfunc_flags = __btf_kfunc_id_set_contains(btf, BTF_KFUNC_HOOK_GENERIC, kfunc_btf_id);
+	if (kfunc_flags)
+		return kfunc_flags;
 
 	hook = bpf_prog_type_to_kfunc_hook(prog_type);
 	return __btf_kfunc_id_set_contains(btf, hook, kfunc_btf_id);
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 283f55bbeb70..2f11e22eefba 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1717,9 +1717,20 @@ static const struct btf_kfunc_id_set tracing_kfunc_set = {
 	.set   = &tracing_btf_ids,
 };
 
+BTF_SET8_START(generic_btf_ids)
+BTF_SET8_END(generic_btf_ids)
+
+static const struct btf_kfunc_id_set generic_kfunc_set = {
+	.owner = THIS_MODULE,
+	.set   = &generic_btf_ids,
+};
+
 static int __init kfunc_init(void)
 {
-	return register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &tracing_kfunc_set);
+	int ret;
+
+	ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &tracing_kfunc_set);
+	return ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_UNSPEC, &generic_kfunc_set);
 }
 
 late_initcall(kfunc_init);
-- 
2.30.2

[RFC PATCH bpf-next 2/3] bpf: Implement bpf_get_kern_btf_id() kfunc

[Yonghong Song]

The signature of bpf_get_kern_btf_id() function looks like
  void *bpf_get_kern_btf_id(obj, expected_btf_id)
The obj has a pointer type. The expected_btf_id is 0 or
a btf id to be returned by the kfunc. The function
currently supports two kinds of obj:
  - obj: ptr_to_ctx, expected_btf_id: 0
    return the expected kernel ctx btf id
  - obj: ptr to char/unsigned char, expected_btf_id: a struct btf id
    return expected_btf_id
The second case looks like a type casting, e.g., in kernel we have
  #define skb_shinfo(SKB) ((struct skb_shared_info *)(skb_end_pointer(SKB)))
bpf program can get a skb_shared_info btf id ptr with bpf_get_kern_btf_id()
kfunc.

The current prototype is incomplete in the following areas:
  - ptr to char/unsigned char is not supported. In btf_struct_walk(),
    for a pointer not pointing to a struct, a WALK_SCALAR is returned.
    this needs to be improved to recognize walking ptr to non-struct.
  - the current implementation didn't validate non-zero expected_btf_id
    parameter has to be a struct. This can be added easily later.
  - The forced type casting case may not be reliable, so the resulted
    ptr and later walked ptr cannot be passed to helper/kfunc's.
    We need to resolve this some how. David Vernet has some work ([1])
    in this area and it is not finalized yet.

  [1] https://lore.kernel.org/bpf/20221020222416.3415511-1-void@manifault.com/

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 include/linux/bpf.h   |  2 ++
 kernel/bpf/btf.c      | 67 ++++++++++++++++++++++++++++++++++++++++++-
 kernel/bpf/helpers.c  |  5 ++++
 kernel/bpf/verifier.c |  8 +++++-
 4 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 798aec816970..d6a1c463b87e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2087,6 +2087,7 @@ struct bpf_kfunc_arg_meta {
 	bool r0_rdonly;
 	int ref_obj_id;
 	u32 flags;
+	u32 expected_ret_btf_id;
 };
 
 struct bpf_reg_state;
@@ -2113,6 +2114,7 @@ bool bpf_prog_has_kfunc_call(const struct bpf_prog *prog);
 const struct btf_func_model *
 bpf_jit_find_kfunc_model(const struct bpf_prog *prog,
 			 const struct bpf_insn *insn);
+void *bpf_get_kern_btf_id(void *, u32 expected_btf_id);
 struct bpf_core_ctx {
 	struct bpf_verifier_log *log;
 	const struct btf *btf;
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 84f09235857c..b0a2555c8ca3 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6309,6 +6309,9 @@ static bool btf_is_kfunc_arg_mem_size(const struct btf *btf,
 	return true;
 }
 
+BTF_ID_LIST_SINGLE(bpf_get_kern_btf_id_id, func, bpf_get_kern_btf_id)
+
+
 static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 				    const struct btf *btf, u32 func_id,
 				    struct bpf_reg_state *regs,
@@ -6318,7 +6321,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 {
 	enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
 	bool rel = false, kptr_get = false, trusted_args = false;
-	bool sleepable = false;
+	bool sleepable = false, get_btf_id_kfunc = false;
 	struct bpf_verifier_log *log = &env->log;
 	u32 i, nargs, ref_id, ref_obj_id = 0;
 	bool is_kfunc = btf_is_kernel(btf);
@@ -6357,6 +6360,67 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 		kptr_get = kfunc_meta->flags & KF_KPTR_GET;
 		trusted_args = kfunc_meta->flags & KF_TRUSTED_ARGS;
 		sleepable = kfunc_meta->flags & KF_SLEEPABLE;
+		get_btf_id_kfunc = func_id == *bpf_get_kern_btf_id_id;
+	}
+
+	/* special processing for bpf_get_btf_id kfunc.
+	 * arg1:
+	 *   must be a ptr_to_ctx or ptr_to_u8/s8.
+	 * arg2:
+	 *   must be a constant, if non-zero representing an user specified expected
+	 *   ret_btf_id.
+	 * If ptr_to_ctx, arg2 must be 0 or a value equals to corresponding kctx btf_id
+	 * and the ret ptr can be passed to a helper/kfunc. Otherwise, arg2 must be a
+	 * valid struct type btf_id, and the ret ptr cannot be passed to a helper/kfunc.
+	 */
+	if (get_btf_id_kfunc) {
+		struct bpf_reg_state *reg = &regs[1];
+		int kctx_btf_id = 0;
+		s64 val;
+
+		if (nargs != 2) {
+			bpf_log(log, "Incorrect number of arguments %s, actual %d expect 2\n",
+				func_name, nargs);
+			return -EINVAL;
+		}
+
+		/* arg1 */
+		if (reg->type == PTR_TO_CTX) {
+			enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
+			const struct btf_type *conv_struct;
+			const struct btf_member *ctx_type;
+
+			conv_struct = bpf_ctx_convert.t;
+			ctx_type = btf_type_member(conv_struct) + bpf_ctx_convert_map[prog_type] * 2;
+			ctx_type++;
+
+			/* find the kctx type */
+			kctx_btf_id = ctx_type->type;
+		} else if (reg->type != SCALAR_VALUE) {
+			/* FIXME: we actually expects a pointer to char/unsigned_char */
+			bpf_log(log, "Incorrect type %x\n", reg->type);
+			return -EINVAL;
+		}
+
+		reg = &regs[2];
+		if (!tnum_is_const(reg->var_off)) {
+			bpf_log(log, "arg 2 is not constant\n");
+			return -EINVAL;
+		}
+
+		val = reg->var_off.value;
+		if (kctx_btf_id == 0) {
+			/* FIXME: ensure val is a btf_id pointing to a struct */
+			kctx_btf_id = val;
+		} else {
+			if (val != 0 && val != kctx_btf_id) {
+				bpf_log(log, "Incorrect expected_btf_id %lld, expect 0 or %d\n", val, kctx_btf_id);
+				return -EINVAL;
+			}
+		}
+
+		kfunc_meta->expected_ret_btf_id = kctx_btf_id;
+		goto check;
 	}
 
 	/* check that BTF function arguments match actual types that the
@@ -6639,6 +6703,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
 		return -EINVAL;
 	}
 
+check:
 	if (sleepable && !env->prog->aux->sleepable) {
 		bpf_log(log, "kernel function %s is sleepable but the program is not\n",
 			func_name);
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 2f11e22eefba..f8995947a790 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1717,7 +1717,12 @@ static const struct btf_kfunc_id_set tracing_kfunc_set = {
 	.set   = &tracing_btf_ids,
 };
 
+void *bpf_get_kern_btf_id(void *obj, u32 expected_btf_id) {
+	return obj;
+}
+
 BTF_SET8_START(generic_btf_ids)
+BTF_ID_FLAGS(func, bpf_get_kern_btf_id)
 BTF_SET8_END(generic_btf_ids)
 
 static const struct btf_kfunc_id_set generic_kfunc_set = {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 07c0259dfc1a..cf284af382a3 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7868,7 +7868,13 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 	} else if (btf_type_is_ptr(t)) {
 		ptr_type = btf_type_skip_modifiers(desc_btf, t->type,
 						   &ptr_type_id);
-		if (!btf_type_is_struct(ptr_type)) {
+		if (meta.expected_ret_btf_id) {
+			mark_reg_known_zero(env, regs, BPF_REG_0);
+			regs[BPF_REG_0].btf = desc_btf;
+			regs[BPF_REG_0].type = PTR_TO_BTF_ID;
+			regs[BPF_REG_0].btf_id = meta.expected_ret_btf_id;
+
+		} else if (!btf_type_is_struct(ptr_type)) {
 			if (!meta.r0_size) {
 				ptr_type_name = btf_name_by_offset(desc_btf,
 								   ptr_type->name_off);
-- 
2.30.2

Re: [RFC PATCH bpf-next 2/3] bpf: Implement bpf_get_kern_btf_id() kfunc

[Alexei Starovoitov]

On Mon, Nov 14, 2022 at 08:23:39AM -0800, Yonghong Song wrote:
> The signature of bpf_get_kern_btf_id() function looks like
>   void *bpf_get_kern_btf_id(obj, expected_btf_id)
> The obj has a pointer type. The expected_btf_id is 0 or
> a btf id to be returned by the kfunc. The function
> currently supports two kinds of obj:
>   - obj: ptr_to_ctx, expected_btf_id: 0
>     return the expected kernel ctx btf id
>   - obj: ptr to char/unsigned char, expected_btf_id: a struct btf id
>     return expected_btf_id
> The second case looks like a type casting, e.g., in kernel we have
>   #define skb_shinfo(SKB) ((struct skb_shared_info *)(skb_end_pointer(SKB)))
> bpf program can get a skb_shared_info btf id ptr with bpf_get_kern_btf_id()
> kfunc.

Kumar has proposed 
bpf_rdonly_cast(any_64bit_value, btf_id) -> PTR_TO_BTF_ID | PTR_UNTRUSTED.
The idea of bpf_get_kern_btf_id(ctx) looks complementary.
The bpf_get_kern_btf_id name is too specific imo.
How about two kfuncs:

bpf_cast_to_kern_ctx(ctx) -> ptr_to_btf_id | ptr_trusted 
bpf_rdonly_cast(any_scalar, btf_id) -> ptr_to_btf_id | ptr_untrusted

ptr_trusted flag will have semantics as discsused with David and Kumar in:
https://lore.kernel.org/bpf/CAADnVQ+KZcFZdC=W_qZ3kam9yAjORtpN-9+Ptg_Whj-gRxCZNQ@mail.gmail.com/

The verifier knows how to cast safe pointer 'ctx' to kernel 'mirror' structure.
No need for additional btf_id argument.
We can express it as ptr_to_btf_id | ptr_trusted and safely pass to kfuncs.
bpf_rdonly_cast() can accept any 64-bit value.
There is no need to limit it to 'char *' arg. Since it's ptr_to_btf_id | ptr_untrusted
it cannot be passed to kfuncs and only rdonly acccess is allowed.
Both kfuncs need to be cap_perfmon gated, of course.
Thoughts?

Re: [RFC PATCH bpf-next 2/3] bpf: Implement bpf_get_kern_btf_id() kfunc

[Kumar Kartikeya Dwivedi]

On Wed, Nov 16, 2022 at 01:13:08AM IST, Alexei Starovoitov wrote:
> On Mon, Nov 14, 2022 at 08:23:39AM -0800, Yonghong Song wrote:
> > The signature of bpf_get_kern_btf_id() function looks like
> >   void *bpf_get_kern_btf_id(obj, expected_btf_id)
> > The obj has a pointer type. The expected_btf_id is 0 or
> > a btf id to be returned by the kfunc. The function
> > currently supports two kinds of obj:
> >   - obj: ptr_to_ctx, expected_btf_id: 0
> >     return the expected kernel ctx btf id
> >   - obj: ptr to char/unsigned char, expected_btf_id: a struct btf id
> >     return expected_btf_id
> > The second case looks like a type casting, e.g., in kernel we have
> >   #define skb_shinfo(SKB) ((struct skb_shared_info *)(skb_end_pointer(SKB)))
> > bpf program can get a skb_shared_info btf id ptr with bpf_get_kern_btf_id()
> > kfunc.
>
> Kumar has proposed
> bpf_rdonly_cast(any_64bit_value, btf_id) -> PTR_TO_BTF_ID | PTR_UNTRUSTED.
> The idea of bpf_get_kern_btf_id(ctx) looks complementary.
> The bpf_get_kern_btf_id name is too specific imo.
> How about two kfuncs:
>
> bpf_cast_to_kern_ctx(ctx) -> ptr_to_btf_id | ptr_trusted
> bpf_rdonly_cast(any_scalar, btf_id) -> ptr_to_btf_id | ptr_untrusted
>
> ptr_trusted flag will have semantics as discsused with David and Kumar in:
> https://lore.kernel.org/bpf/CAADnVQ+KZcFZdC=W_qZ3kam9yAjORtpN-9+Ptg_Whj-gRxCZNQ@mail.gmail.com/
>
> The verifier knows how to cast safe pointer 'ctx' to kernel 'mirror' structure.
> No need for additional btf_id argument.
> We can express it as ptr_to_btf_id | ptr_trusted and safely pass to kfuncs.
> bpf_rdonly_cast() can accept any 64-bit value.
> There is no need to limit it to 'char *' arg. Since it's ptr_to_btf_id | ptr_untrusted
> it cannot be passed to kfuncs and only rdonly acccess is allowed.
> Both kfuncs need to be cap_perfmon gated, of course.
> Thoughts?

Here is the PoC I wrote when we discussed this:
It still uses bpf_unsafe_cast naming, but that was before Alexei suggested the
bpf_rdonly_cast name.
https://github.com/kkdwivedi/linux/commits/unsafe-cast (see the 2 latest commits)
The selftest showcases how it will be useful.

Re: [RFC PATCH bpf-next 2/3] bpf: Implement bpf_get_kern_btf_id() kfunc

[Yonghong Song]

On 11/15/22 12:05 PM, Kumar Kartikeya Dwivedi wrote:
> On Wed, Nov 16, 2022 at 01:13:08AM IST, Alexei Starovoitov wrote:
>> On Mon, Nov 14, 2022 at 08:23:39AM -0800, Yonghong Song wrote:
>>> The signature of bpf_get_kern_btf_id() function looks like
>>>    void *bpf_get_kern_btf_id(obj, expected_btf_id)
>>> The obj has a pointer type. The expected_btf_id is 0 or
>>> a btf id to be returned by the kfunc. The function
>>> currently supports two kinds of obj:
>>>    - obj: ptr_to_ctx, expected_btf_id: 0
>>>      return the expected kernel ctx btf id
>>>    - obj: ptr to char/unsigned char, expected_btf_id: a struct btf id
>>>      return expected_btf_id
>>> The second case looks like a type casting, e.g., in kernel we have
>>>    #define skb_shinfo(SKB) ((struct skb_shared_info *)(skb_end_pointer(SKB)))
>>> bpf program can get a skb_shared_info btf id ptr with bpf_get_kern_btf_id()
>>> kfunc.
>>
>> Kumar has proposed
>> bpf_rdonly_cast(any_64bit_value, btf_id) -> PTR_TO_BTF_ID | PTR_UNTRUSTED.
>> The idea of bpf_get_kern_btf_id(ctx) looks complementary.
>> The bpf_get_kern_btf_id name is too specific imo.
>> How about two kfuncs:
>>
>> bpf_cast_to_kern_ctx(ctx) -> ptr_to_btf_id | ptr_trusted
>> bpf_rdonly_cast(any_scalar, btf_id) -> ptr_to_btf_id | ptr_untrusted

Sounds good. Two helpers can make sense as it is indeed true for
bpf_cast_to_kern_ctx(ctx), the btf_id is not needed.

>>
>> ptr_trusted flag will have semantics as discsused with David and Kumar in:
>> https://lore.kernel.org/bpf/CAADnVQ+KZcFZdC=W_qZ3kam9yAjORtpN-9+Ptg_Whj-gRxCZNQ@mail.gmail.com/
>>
>> The verifier knows how to cast safe pointer 'ctx' to kernel 'mirror' structure.
>> No need for additional btf_id argument.
>> We can express it as ptr_to_btf_id | ptr_trusted and safely pass to kfuncs.
>> bpf_rdonly_cast() can accept any 64-bit value.
>> There is no need to limit it to 'char *' arg. Since it's ptr_to_btf_id | ptr_untrusted
>> it cannot be passed to kfuncs and only rdonly acccess is allowed.
>> Both kfuncs need to be cap_perfmon gated, of course.
>> Thoughts?

Currently, we only have SCALAR_VALUE to represent 'void *', 'char *', 
'unsigned char *'. yes, some pointer might be long and cast to 'struct 
foo *', so the generalization of bpf_rdonly_cast() to all scalar value
should be fine. Although it is possible the it might be abused and 
incuring some exception handling, but guarding it with cap_perfmon
should be okay.

> 
> Here is the PoC I wrote when we discussed this:
> It still uses bpf_unsafe_cast naming, but that was before Alexei suggested the
> bpf_rdonly_cast name.
> https://github.com/kkdwivedi/linux/commits/unsafe-cast (see the 2 latest commits)
> The selftest showcases how it will be useful.

Sounds good. I can redo may patch for bpf_cast_to_kern_ctx(), which 
should cover some of existing cases. Kumar, since you are working on
bpf_rdonly_cast(), you could work on that later. If you want me to do 
it, just let me know I can incorporate it in my patch set.

Re: [RFC PATCH bpf-next 2/3] bpf: Implement bpf_get_kern_btf_id() kfunc

[Kumar Kartikeya Dwivedi]

On Wed, Nov 16, 2022 at 01:56:14AM IST, Yonghong Song wrote:
>
>
> On 11/15/22 12:05 PM, Kumar Kartikeya Dwivedi wrote:
> > On Wed, Nov 16, 2022 at 01:13:08AM IST, Alexei Starovoitov wrote:
> > > On Mon, Nov 14, 2022 at 08:23:39AM -0800, Yonghong Song wrote:
> > > > The signature of bpf_get_kern_btf_id() function looks like
> > > >    void *bpf_get_kern_btf_id(obj, expected_btf_id)
> > > > The obj has a pointer type. The expected_btf_id is 0 or
> > > > a btf id to be returned by the kfunc. The function
> > > > currently supports two kinds of obj:
> > > >    - obj: ptr_to_ctx, expected_btf_id: 0
> > > >      return the expected kernel ctx btf id
> > > >    - obj: ptr to char/unsigned char, expected_btf_id: a struct btf id
> > > >      return expected_btf_id
> > > > The second case looks like a type casting, e.g., in kernel we have
> > > >    #define skb_shinfo(SKB) ((struct skb_shared_info *)(skb_end_pointer(SKB)))
> > > > bpf program can get a skb_shared_info btf id ptr with bpf_get_kern_btf_id()
> > > > kfunc.
> > >
> > > Kumar has proposed
> > > bpf_rdonly_cast(any_64bit_value, btf_id) -> PTR_TO_BTF_ID | PTR_UNTRUSTED.
> > > The idea of bpf_get_kern_btf_id(ctx) looks complementary.
> > > The bpf_get_kern_btf_id name is too specific imo.
> > > How about two kfuncs:
> > >
> > > bpf_cast_to_kern_ctx(ctx) -> ptr_to_btf_id | ptr_trusted
> > > bpf_rdonly_cast(any_scalar, btf_id) -> ptr_to_btf_id | ptr_untrusted
>
> Sounds good. Two helpers can make sense as it is indeed true for
> bpf_cast_to_kern_ctx(ctx), the btf_id is not needed.
>
> > >
> > > ptr_trusted flag will have semantics as discsused with David and Kumar in:
> > > https://lore.kernel.org/bpf/CAADnVQ+KZcFZdC=W_qZ3kam9yAjORtpN-9+Ptg_Whj-gRxCZNQ@mail.gmail.com/
> > >
> > > The verifier knows how to cast safe pointer 'ctx' to kernel 'mirror' structure.
> > > No need for additional btf_id argument.
> > > We can express it as ptr_to_btf_id | ptr_trusted and safely pass to kfuncs.
> > > bpf_rdonly_cast() can accept any 64-bit value.
> > > There is no need to limit it to 'char *' arg. Since it's ptr_to_btf_id | ptr_untrusted
> > > it cannot be passed to kfuncs and only rdonly acccess is allowed.
> > > Both kfuncs need to be cap_perfmon gated, of course.
> > > Thoughts?
>
> Currently, we only have SCALAR_VALUE to represent 'void *', 'char *',
> 'unsigned char *'. yes, some pointer might be long and cast to 'struct foo
> *', so the generalization of bpf_rdonly_cast() to all scalar value
> should be fine. Although it is possible the it might be abused and incuring
> some exception handling, but guarding it with cap_perfmon
> should be okay.
>
> >
> > Here is the PoC I wrote when we discussed this:
> > It still uses bpf_unsafe_cast naming, but that was before Alexei suggested the
> > bpf_rdonly_cast name.
> > https://github.com/kkdwivedi/linux/commits/unsafe-cast (see the 2 latest commits)
> > The selftest showcases how it will be useful.
>
> Sounds good. I can redo may patch for bpf_cast_to_kern_ctx(), which should
> cover some of existing cases. Kumar, since you are working on
> bpf_rdonly_cast(), you could work on that later. If you want me to do it,
> just let me know I can incorporate it in my patch set.

I think the patch itself is pretty trivial. What's needed is a bit of
refactoring, since I would also want to make this work for module BTF types.

In that case, we need to take a type in prog BTF, look it up in the kernel, and
mark the reg using looked up BTF and BTF ID. However this raises module BTF
reference, and it needs to be kept until verifier is done (as it gets set to
reg->btf).

This is why the helper takes local type ID instead of bpf_core_type_id_kernel,
since that doesn't work for module types (IIUC).

Instead of the current used_btfs array logic, Alexei suggested guarding module
BTF free path with a rwsem, which the verifier can hold in bpf_check, so that we
don't have to worry about keeping module BTF references around during verification.
Modules are loaded/unloaded infrequently so it should be fine.

Then it also became clear we currently stash BTFs in some places unecessarily
and we could simply drop those after prog is verified. So it would make sense
to drop those cases too (kfunc_btf_tab, used_btfs btf_mod_pair, etc.). After
verification the prog only needs to pin the module references, not mod BTF
references.

Maybe all of this does not have to be done together.

So let me know if you want to take it, I have no problems with that, otherwise I
can get to it once I am done with the linked list and dynptr stuff.

Re: [RFC PATCH bpf-next 2/3] bpf: Implement bpf_get_kern_btf_id() kfunc

[Yonghong Song]

On 11/17/22 10:24 AM, Kumar Kartikeya Dwivedi wrote:
> On Wed, Nov 16, 2022 at 01:56:14AM IST, Yonghong Song wrote:
>>
>>
>> On 11/15/22 12:05 PM, Kumar Kartikeya Dwivedi wrote:
>>> On Wed, Nov 16, 2022 at 01:13:08AM IST, Alexei Starovoitov wrote:
>>>> On Mon, Nov 14, 2022 at 08:23:39AM -0800, Yonghong Song wrote:
>>>>> The signature of bpf_get_kern_btf_id() function looks like
>>>>>     void *bpf_get_kern_btf_id(obj, expected_btf_id)
>>>>> The obj has a pointer type. The expected_btf_id is 0 or
>>>>> a btf id to be returned by the kfunc. The function
>>>>> currently supports two kinds of obj:
>>>>>     - obj: ptr_to_ctx, expected_btf_id: 0
>>>>>       return the expected kernel ctx btf id
>>>>>     - obj: ptr to char/unsigned char, expected_btf_id: a struct btf id
>>>>>       return expected_btf_id
>>>>> The second case looks like a type casting, e.g., in kernel we have
>>>>>     #define skb_shinfo(SKB) ((struct skb_shared_info *)(skb_end_pointer(SKB)))
>>>>> bpf program can get a skb_shared_info btf id ptr with bpf_get_kern_btf_id()
>>>>> kfunc.
>>>>
>>>> Kumar has proposed
>>>> bpf_rdonly_cast(any_64bit_value, btf_id) -> PTR_TO_BTF_ID | PTR_UNTRUSTED.
>>>> The idea of bpf_get_kern_btf_id(ctx) looks complementary.
>>>> The bpf_get_kern_btf_id name is too specific imo.
>>>> How about two kfuncs:
>>>>
>>>> bpf_cast_to_kern_ctx(ctx) -> ptr_to_btf_id | ptr_trusted
>>>> bpf_rdonly_cast(any_scalar, btf_id) -> ptr_to_btf_id | ptr_untrusted
>>
>> Sounds good. Two helpers can make sense as it is indeed true for
>> bpf_cast_to_kern_ctx(ctx), the btf_id is not needed.
>>
>>>>
>>>> ptr_trusted flag will have semantics as discsused with David and Kumar in:
>>>> https://lore.kernel.org/bpf/CAADnVQ+KZcFZdC=W_qZ3kam9yAjORtpN-9+Ptg_Whj-gRxCZNQ@mail.gmail.com/
>>>>
>>>> The verifier knows how to cast safe pointer 'ctx' to kernel 'mirror' structure.
>>>> No need for additional btf_id argument.
>>>> We can express it as ptr_to_btf_id | ptr_trusted and safely pass to kfuncs.
>>>> bpf_rdonly_cast() can accept any 64-bit value.
>>>> There is no need to limit it to 'char *' arg. Since it's ptr_to_btf_id | ptr_untrusted
>>>> it cannot be passed to kfuncs and only rdonly acccess is allowed.
>>>> Both kfuncs need to be cap_perfmon gated, of course.
>>>> Thoughts?
>>
>> Currently, we only have SCALAR_VALUE to represent 'void *', 'char *',
>> 'unsigned char *'. yes, some pointer might be long and cast to 'struct foo
>> *', so the generalization of bpf_rdonly_cast() to all scalar value
>> should be fine. Although it is possible the it might be abused and incuring
>> some exception handling, but guarding it with cap_perfmon
>> should be okay.
>>
>>>
>>> Here is the PoC I wrote when we discussed this:
>>> It still uses bpf_unsafe_cast naming, but that was before Alexei suggested the
>>> bpf_rdonly_cast name.
>>> https://github.com/kkdwivedi/linux/commits/unsafe-cast (see the 2 latest commits)
>>> The selftest showcases how it will be useful.
>>
>> Sounds good. I can redo may patch for bpf_cast_to_kern_ctx(), which should
>> cover some of existing cases. Kumar, since you are working on
>> bpf_rdonly_cast(), you could work on that later. If you want me to do it,
>> just let me know I can incorporate it in my patch set.

I just prototyped a little bit with Alexei's suggested interface. It has 
some differences. I will explain in my next revision.

My prototype already added bpf_rdonly_cast(). As you suggested, it is
not too hard. I have not done with module btf yet. Will add it
as you suggested below.

> 
> I think the patch itself is pretty trivial. What's needed is a bit of
> refactoring, since I would also want to make this work for module BTF types.
> 
> In that case, we need to take a type in prog BTF, look it up in the kernel, and
> mark the reg using looked up BTF and BTF ID. However this raises module BTF
> reference, and it needs to be kept until verifier is done (as it gets set to
> reg->btf).
> 
> This is why the helper takes local type ID instead of bpf_core_type_id_kernel,
> since that doesn't work for module types (IIUC).
> 
> Instead of the current used_btfs array logic, Alexei suggested guarding module
> BTF free path with a rwsem, which the verifier can hold in bpf_check, so that we
> don't have to worry about keeping module BTF references around during verification.
> Modules are loaded/unloaded infrequently so it should be fine.
> 
> Then it also became clear we currently stash BTFs in some places unecessarily
> and we could simply drop those after prog is verified. So it would make sense
> to drop those cases too (kfunc_btf_tab, used_btfs btf_mod_pair, etc.). After
> verification the prog only needs to pin the module references, not mod BTF
> references.
> 
> Maybe all of this does not have to be done together.

I will focus on implementing bpf_cast_to_kern_ctx() and 
bpf_rdonly_cast(). Others can be delayed for later patches.

> 
> So let me know if you want to take it, I have no problems with that, otherwise I
> can get to it once I am done with the linked list and dynptr stuff.

Re: [RFC PATCH bpf-next 2/3] bpf: Implement bpf_get_kern_btf_id() kfunc

[Kumar Kartikeya Dwivedi]

On Fri, Nov 18, 2022 at 04:22:40AM IST, Yonghong Song wrote:
>
>
> On 11/17/22 10:24 AM, Kumar Kartikeya Dwivedi wrote:
> > On Wed, Nov 16, 2022 at 01:56:14AM IST, Yonghong Song wrote:
> > >
> > >
> > > On 11/15/22 12:05 PM, Kumar Kartikeya Dwivedi wrote:
> > > > On Wed, Nov 16, 2022 at 01:13:08AM IST, Alexei Starovoitov wrote:
> > > > > On Mon, Nov 14, 2022 at 08:23:39AM -0800, Yonghong Song wrote:
> > > > > > The signature of bpf_get_kern_btf_id() function looks like
> > > > > >     void *bpf_get_kern_btf_id(obj, expected_btf_id)
> > > > > > The obj has a pointer type. The expected_btf_id is 0 or
> > > > > > a btf id to be returned by the kfunc. The function
> > > > > > currently supports two kinds of obj:
> > > > > >     - obj: ptr_to_ctx, expected_btf_id: 0
> > > > > >       return the expected kernel ctx btf id
> > > > > >     - obj: ptr to char/unsigned char, expected_btf_id: a struct btf id
> > > > > >       return expected_btf_id
> > > > > > The second case looks like a type casting, e.g., in kernel we have
> > > > > >     #define skb_shinfo(SKB) ((struct skb_shared_info *)(skb_end_pointer(SKB)))
> > > > > > bpf program can get a skb_shared_info btf id ptr with bpf_get_kern_btf_id()
> > > > > > kfunc.
> > > > >
> > > > > Kumar has proposed
> > > > > bpf_rdonly_cast(any_64bit_value, btf_id) -> PTR_TO_BTF_ID | PTR_UNTRUSTED.
> > > > > The idea of bpf_get_kern_btf_id(ctx) looks complementary.
> > > > > The bpf_get_kern_btf_id name is too specific imo.
> > > > > How about two kfuncs:
> > > > >
> > > > > bpf_cast_to_kern_ctx(ctx) -> ptr_to_btf_id | ptr_trusted
> > > > > bpf_rdonly_cast(any_scalar, btf_id) -> ptr_to_btf_id | ptr_untrusted
> > >
> > > Sounds good. Two helpers can make sense as it is indeed true for
> > > bpf_cast_to_kern_ctx(ctx), the btf_id is not needed.
> > >
> > > > >
> > > > > ptr_trusted flag will have semantics as discsused with David and Kumar in:
> > > > > https://lore.kernel.org/bpf/CAADnVQ+KZcFZdC=W_qZ3kam9yAjORtpN-9+Ptg_Whj-gRxCZNQ@mail.gmail.com/
> > > > >
> > > > > The verifier knows how to cast safe pointer 'ctx' to kernel 'mirror' structure.
> > > > > No need for additional btf_id argument.
> > > > > We can express it as ptr_to_btf_id | ptr_trusted and safely pass to kfuncs.
> > > > > bpf_rdonly_cast() can accept any 64-bit value.
> > > > > There is no need to limit it to 'char *' arg. Since it's ptr_to_btf_id | ptr_untrusted
> > > > > it cannot be passed to kfuncs and only rdonly acccess is allowed.
> > > > > Both kfuncs need to be cap_perfmon gated, of course.
> > > > > Thoughts?
> > >
> > > Currently, we only have SCALAR_VALUE to represent 'void *', 'char *',
> > > 'unsigned char *'. yes, some pointer might be long and cast to 'struct foo
> > > *', so the generalization of bpf_rdonly_cast() to all scalar value
> > > should be fine. Although it is possible the it might be abused and incuring
> > > some exception handling, but guarding it with cap_perfmon
> > > should be okay.
> > >
> > > >
> > > > Here is the PoC I wrote when we discussed this:
> > > > It still uses bpf_unsafe_cast naming, but that was before Alexei suggested the
> > > > bpf_rdonly_cast name.
> > > > https://github.com/kkdwivedi/linux/commits/unsafe-cast (see the 2 latest commits)
> > > > The selftest showcases how it will be useful.
> > >
> > > Sounds good. I can redo may patch for bpf_cast_to_kern_ctx(), which should
> > > cover some of existing cases. Kumar, since you are working on
> > > bpf_rdonly_cast(), you could work on that later. If you want me to do it,
> > > just let me know I can incorporate it in my patch set.
>
> I just prototyped a little bit with Alexei's suggested interface. It has
> some differences. I will explain in my next revision.
>
> My prototype already added bpf_rdonly_cast(). As you suggested, it is
> not too hard. I have not done with module btf yet. Will add it
> as you suggested below.
>

It's fine to also leave out types in module BTFs for now, atleast as long you
return a reasonable error message from the verifier. Just relying on btf_vmlinux
is enough for the FIXMEs in selftests.

Re: [RFC PATCH bpf-next 2/3] bpf: Implement bpf_get_kern_btf_id() kfunc

[Yonghong Song]

On 11/17/22 3:01 PM, Kumar Kartikeya Dwivedi wrote:
> On Fri, Nov 18, 2022 at 04:22:40AM IST, Yonghong Song wrote:
>>
>>
>> On 11/17/22 10:24 AM, Kumar Kartikeya Dwivedi wrote:
>>> On Wed, Nov 16, 2022 at 01:56:14AM IST, Yonghong Song wrote:
>>>>
>>>>
>>>> On 11/15/22 12:05 PM, Kumar Kartikeya Dwivedi wrote:
>>>>> On Wed, Nov 16, 2022 at 01:13:08AM IST, Alexei Starovoitov wrote:
>>>>>> On Mon, Nov 14, 2022 at 08:23:39AM -0800, Yonghong Song wrote:
>>>>>>> The signature of bpf_get_kern_btf_id() function looks like
>>>>>>>      void *bpf_get_kern_btf_id(obj, expected_btf_id)
>>>>>>> The obj has a pointer type. The expected_btf_id is 0 or
>>>>>>> a btf id to be returned by the kfunc. The function
>>>>>>> currently supports two kinds of obj:
>>>>>>>      - obj: ptr_to_ctx, expected_btf_id: 0
>>>>>>>        return the expected kernel ctx btf id
>>>>>>>      - obj: ptr to char/unsigned char, expected_btf_id: a struct btf id
>>>>>>>        return expected_btf_id
>>>>>>> The second case looks like a type casting, e.g., in kernel we have
>>>>>>>      #define skb_shinfo(SKB) ((struct skb_shared_info *)(skb_end_pointer(SKB)))
>>>>>>> bpf program can get a skb_shared_info btf id ptr with bpf_get_kern_btf_id()
>>>>>>> kfunc.
>>>>>>
>>>>>> Kumar has proposed
>>>>>> bpf_rdonly_cast(any_64bit_value, btf_id) -> PTR_TO_BTF_ID | PTR_UNTRUSTED.
>>>>>> The idea of bpf_get_kern_btf_id(ctx) looks complementary.
>>>>>> The bpf_get_kern_btf_id name is too specific imo.
>>>>>> How about two kfuncs:
>>>>>>
>>>>>> bpf_cast_to_kern_ctx(ctx) -> ptr_to_btf_id | ptr_trusted
>>>>>> bpf_rdonly_cast(any_scalar, btf_id) -> ptr_to_btf_id | ptr_untrusted
>>>>
>>>> Sounds good. Two helpers can make sense as it is indeed true for
>>>> bpf_cast_to_kern_ctx(ctx), the btf_id is not needed.
>>>>
>>>>>>
>>>>>> ptr_trusted flag will have semantics as discsused with David and Kumar in:
>>>>>> https://lore.kernel.org/bpf/CAADnVQ+KZcFZdC=W_qZ3kam9yAjORtpN-9+Ptg_Whj-gRxCZNQ@mail.gmail.com/
>>>>>>
>>>>>> The verifier knows how to cast safe pointer 'ctx' to kernel 'mirror' structure.
>>>>>> No need for additional btf_id argument.
>>>>>> We can express it as ptr_to_btf_id | ptr_trusted and safely pass to kfuncs.
>>>>>> bpf_rdonly_cast() can accept any 64-bit value.
>>>>>> There is no need to limit it to 'char *' arg. Since it's ptr_to_btf_id | ptr_untrusted
>>>>>> it cannot be passed to kfuncs and only rdonly acccess is allowed.
>>>>>> Both kfuncs need to be cap_perfmon gated, of course.
>>>>>> Thoughts?
>>>>
>>>> Currently, we only have SCALAR_VALUE to represent 'void *', 'char *',
>>>> 'unsigned char *'. yes, some pointer might be long and cast to 'struct foo
>>>> *', so the generalization of bpf_rdonly_cast() to all scalar value
>>>> should be fine. Although it is possible the it might be abused and incuring
>>>> some exception handling, but guarding it with cap_perfmon
>>>> should be okay.
>>>>
>>>>>
>>>>> Here is the PoC I wrote when we discussed this:
>>>>> It still uses bpf_unsafe_cast naming, but that was before Alexei suggested the
>>>>> bpf_rdonly_cast name.
>>>>> https://github.com/kkdwivedi/linux/commits/unsafe-cast (see the 2 latest commits)
>>>>> The selftest showcases how it will be useful.
>>>>
>>>> Sounds good. I can redo may patch for bpf_cast_to_kern_ctx(), which should
>>>> cover some of existing cases. Kumar, since you are working on
>>>> bpf_rdonly_cast(), you could work on that later. If you want me to do it,
>>>> just let me know I can incorporate it in my patch set.
>>
>> I just prototyped a little bit with Alexei's suggested interface. It has
>> some differences. I will explain in my next revision.
>>
>> My prototype already added bpf_rdonly_cast(). As you suggested, it is
>> not too hard. I have not done with module btf yet. Will add it
>> as you suggested below.
>>
> 
> It's fine to also leave out types in module BTFs for now, atleast as long you
> return a reasonable error message from the verifier. Just relying on btf_vmlinux
> is enough for the FIXMEs in selftests.

Okay, will leave module support at this point then.

[RFC PATCH bpf-next 3/3] bpf: Add bpf_get_kern_btf_id() tests

[Yonghong Song]

Two tests are added. One is from John Fastabend ({1]) which tests
tracing style access for xdp program from the kernel ctx.
Another is a tc test to test both kernel ctx tracing style access
and explicit non-ctx type cast.

  [1] https://lore.kernel.org/bpf/20221109215242.1279993-1-john.fastabend@gmail.com/

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 .../bpf/prog_tests/get_kern_btf_id.c          | 81 +++++++++++++++++++
 .../selftests/bpf/progs/get_kern_btf_id.c     | 44 ++++++++++
 2 files changed, 125 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/get_kern_btf_id.c
 create mode 100644 tools/testing/selftests/bpf/progs/get_kern_btf_id.c

diff --git a/tools/testing/selftests/bpf/prog_tests/get_kern_btf_id.c b/tools/testing/selftests/bpf/prog_tests/get_kern_btf_id.c
new file mode 100644
index 000000000000..205631916564
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/get_kern_btf_id.c
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <test_progs.h>
+#include <network_helpers.h>
+#include "get_kern_btf_id.skel.h"
+
+static void test_xdp(void)
+{
+	struct get_kern_btf_id *skel;
+	int err, prog_fd;
+	char buf[128];
+
+	LIBBPF_OPTS(bpf_test_run_opts, topts,
+		.data_in = &pkt_v4,
+		.data_size_in = sizeof(pkt_v4),
+		.data_out = buf,
+		.data_size_out = sizeof(buf),
+		.repeat = 1,
+	);
+
+	skel = get_kern_btf_id__open();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		return;
+
+	bpf_program__set_autoload(skel->progs.md_xdp, true);
+	err = get_kern_btf_id__load(skel);
+	if (!ASSERT_OK(err, "skel_load"))
+		goto out;
+
+	prog_fd = bpf_program__fd(skel->progs.md_xdp);
+	err = bpf_prog_test_run_opts(prog_fd, &topts);
+	ASSERT_OK(err, "test_run");
+	ASSERT_EQ(topts.retval, XDP_PASS, "xdp test_run retval");
+
+	ASSERT_EQ(skel->bss->ifindex, 1, "xdp_md ifindex");
+	ASSERT_EQ(skel->bss->ifindex, skel->bss->ingress_ifindex, "xdp_md ingress_ifindex");
+	ASSERT_STREQ(skel->bss->name, "lo", "xdp_md name");
+	ASSERT_NEQ(skel->bss->inum, 0, "xdp_md inum");
+
+out:
+	get_kern_btf_id__destroy(skel);
+}
+
+static void test_tc(void)
+{
+	struct get_kern_btf_id *skel;
+	int err, prog_fd;
+
+	LIBBPF_OPTS(bpf_test_run_opts, topts,
+		.data_in = &pkt_v4,
+		.data_size_in = sizeof(pkt_v4),
+		.repeat = 1,
+	);
+
+	skel = get_kern_btf_id__open();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		return;
+
+	bpf_program__set_autoload(skel->progs.md_skb, true);
+	err = get_kern_btf_id__load(skel);
+	if (!ASSERT_OK(err, "skel_load"))
+		goto out;
+
+	prog_fd = bpf_program__fd(skel->progs.md_skb);
+	err = bpf_prog_test_run_opts(prog_fd, &topts);
+	ASSERT_OK(err, "test_run");
+	ASSERT_EQ(topts.retval, 0, "tc test_run retval");
+
+	ASSERT_EQ(skel->bss->meta_len, 0, "skb meta_len");
+	ASSERT_EQ(skel->bss->frag0_len, 0, "skb frag0_len");
+
+out:
+	get_kern_btf_id__destroy(skel);
+}
+
+void test_get_kern_btf_id(void)
+{
+	if (test__start_subtest("xdp"))
+		test_xdp();
+	if (test__start_subtest("tc"))
+		test_tc();
+}
diff --git a/tools/testing/selftests/bpf/progs/get_kern_btf_id.c b/tools/testing/selftests/bpf/progs/get_kern_btf_id.c
new file mode 100644
index 000000000000..b530c7c52ff3
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/get_kern_btf_id.c
@@ -0,0 +1,44 @@
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_core_read.h>
+
+#define	IFNAMSIZ 16
+
+int ifindex, ingress_ifindex;
+char name[IFNAMSIZ];
+unsigned int inum;
+int meta_len, frag0_len;
+
+extern void *bpf_get_kern_btf_id(void *, __u32) __ksym;
+
+SEC("?xdp")
+int md_xdp(struct xdp_md *ctx)
+{
+	struct xdp_buff *kctx = bpf_get_kern_btf_id(ctx, 0);
+	struct net_device *dev;
+
+	dev = kctx->rxq->dev;
+	ifindex = dev->ifindex;
+	inum = dev->nd_net.net->ns.inum;
+	__builtin_memcpy(name, dev->name, IFNAMSIZ);
+	ingress_ifindex = ctx->ingress_ifindex;
+	return XDP_PASS;
+}
+
+SEC("?tc")
+int md_skb(struct __sk_buff *skb)
+{
+	struct sk_buff *kskb = bpf_get_kern_btf_id(skb, 0);
+	struct skb_shared_info *shared_info;
+
+	/* Simulate the following kernel macro:
+	 *   #define skb_shinfo(SKB) ((struct skb_shared_info *)(skb_end_pointer(SKB)))
+	 */
+	shared_info = bpf_get_kern_btf_id(kskb->head + kskb->end,
+		bpf_core_type_id_kernel(struct skb_shared_info));
+	meta_len = shared_info->meta_len;
+	frag0_len = shared_info->frag_list->len;
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.30.2

Re: [RFC PATCH bpf-next 0/3] bpf: Implement bpf_get_kern_btf_id() kfunc

[Toke Høiland-Jørgensen]

Yonghong Song <yhs@fb.com> writes:

> Currenty, a non-tracing bpf program typically has a single 'context' argument
> with predefined uapi struct type. Following these uapi struct, user is able
> to access other fields defined in uapi header. Inside the kernel, the
> user-seen 'context' argument is replaced with 'kernel context' (or 'kcontext'
> in short) which can access more information than what uapi header provides.
> To access other info not in uapi header, people typically do two things:
>   (1). extend uapi to access more fields rooted from 'context'.
>   (2). use bpf_probe_read_kernl() helper to read particular field based on
>     kcontext.
> Using (1) needs uapi change and using (2) makes code more complex since
> direct memory access is not allowed.
>
> There are already a few instances trying to access more information from
> kcontext:
>   (1). trying to access some fields from perf_event kcontext.
>   (2). trying to access some fields from xdp kcontext.
>
> This patch set tried to allow direct memory access for kcontext fields
> by introducing bpf_get_kern_btf_id() kfunc.

I think the general idea is neat. However, I'm a bit concerned that with
this we're allowing networking BPF programs (like XDP) to walk more or
less arbitrary kernel structures, which has thus far been something only
tracing programs have had access to. So we probably require programs to
have CAP_PERFMON to use this kfunc, no?

-Toke

Re: [RFC PATCH bpf-next 0/3] bpf: Implement bpf_get_kern_btf_id() kfunc

[Yonghong Song]

On 11/15/22 8:30 AM, Toke Høiland-Jørgensen wrote:
> Yonghong Song <yhs@fb.com> writes:
> 
>> Currenty, a non-tracing bpf program typically has a single 'context' argument
>> with predefined uapi struct type. Following these uapi struct, user is able
>> to access other fields defined in uapi header. Inside the kernel, the
>> user-seen 'context' argument is replaced with 'kernel context' (or 'kcontext'
>> in short) which can access more information than what uapi header provides.
>> To access other info not in uapi header, people typically do two things:
>>    (1). extend uapi to access more fields rooted from 'context'.
>>    (2). use bpf_probe_read_kernl() helper to read particular field based on
>>      kcontext.
>> Using (1) needs uapi change and using (2) makes code more complex since
>> direct memory access is not allowed.
>>
>> There are already a few instances trying to access more information from
>> kcontext:
>>    (1). trying to access some fields from perf_event kcontext.
>>    (2). trying to access some fields from xdp kcontext.
>>
>> This patch set tried to allow direct memory access for kcontext fields
>> by introducing bpf_get_kern_btf_id() kfunc.
> 
> I think the general idea is neat. However, I'm a bit concerned that with
> this we're allowing networking BPF programs (like XDP) to walk more or
> less arbitrary kernel structures, which has thus far been something only
> tracing programs have had access to. So we probably require programs to
> have CAP_PERFMON to use this kfunc, no?

Make sense, since the kfunc is to enable tracing, yes CAP_PERFMON is
a good idea. I will make the change in the next revision.

> 
> -Toke