mbox series

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

Message ID 20221114162328.622665-1-yhs@fb.com (mailing list archive)
Headers show
Series bpf: Implement bpf_get_kern_btf_id() kfunc | expand

Message

Yonghong Song Nov. 14, 2022, 4:23 p.m. UTC
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

Comments

Toke Høiland-Jørgensen Nov. 15, 2022, 4:30 p.m. UTC | #1
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
Yonghong Song Nov. 15, 2022, 7:53 p.m. UTC | #2
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