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@xxxxxxxxxxxxxx/
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.
