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.
