[bpf-next,v3,0/5] bpf: Allow skb dynptr for tp_btf

Message

Philo Lu Sept. 11, 2024, 3:37 a.m. UTC

This makes bpf_dynptr_from_skb usable for tp_btf, so that we can easily
parse skb in tracepoints. This has been discussed in [0], and Martin
suggested to use dynptr (instead of helpers like bpf_skb_load_bytes).

For safety, skb dynptr shouldn't be used in fentry/fexit. This is achieved
by add KF_TRUSTED_ARGS flag in bpf_dynptr_from_skb defination, because
pointers passed by tracepoint are trusted (PTR_TRUSTED) while those of
fentry/fexit are not.

Another problem raises that NULL pointers could be passed to tracepoint,
such as trace_tcp_send_reset, and we need to recognize them. This is done
by add a "__nullable" suffix in the func_proto of the tracepoint,
discussed in [1].

2 Test cases are added, one for "__nullable" suffix, and the other for
using skb dynptr in tp_btf.

changelog
v2 -> v3 (Andrii Nakryiko):
 Patch 1:
  - Remove prog type check in prog_arg_maybe_null()
  - Add bpf_put_raw_tracepoint() after get()
  - Use kallsyms_lookup() instead of sprintf("%ps")
 Patch 2: Add separate test "tp_btf_nullable", and use full failure msg
v1 -> v2:
 - Add "__nullable" suffix support (Alexei Starovoitov)
 - Replace "struct __sk_buff*" with "void*" in test (Martin KaFai Lau)

[0]
https://lore.kernel.org/all/[e-mail]/T/
[1]
https://lore.kernel.org/all/[e-mail]/T/

Philo Lu (5):
  bpf: Support __nullable argument suffix for tp_btf
  selftests/bpf: Add test for __nullable suffix in tp_btf
  tcp: Use skb__nullable in trace_tcp_send_reset
  bpf: Allow bpf_dynptr_from_skb() for tp_btf
  selftests/bpf: Expand skb dynptr selftests for tp_btf

 include/trace/events/tcp.h                    | 12 +++----
 kernel/bpf/btf.c                              |  9 +++++
 kernel/bpf/verifier.c                         | 36 ++++++++++++++++---
 net/core/filter.c                             |  3 +-
 .../bpf/bpf_testmod/bpf_testmod-events.h      |  6 ++++
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   |  2 ++
 .../testing/selftests/bpf/prog_tests/dynptr.c | 36 +++++++++++++++++--
 .../bpf/prog_tests/tp_btf_nullable.c          | 14 ++++++++
 .../testing/selftests/bpf/progs/dynptr_fail.c | 25 +++++++++++++
 .../selftests/bpf/progs/dynptr_success.c      | 23 ++++++++++++
 .../bpf/progs/test_tp_btf_nullable.c          | 24 +++++++++++++
 11 files changed, 177 insertions(+), 13 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/tp_btf_nullable.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_tp_btf_nullable.c

--
2.32.0.3.g01195cf9f

Comments

[e-mail] Sept. 11, 2024, 5:30 p.m. UTC | #1

Hello:

This series was applied to bpf/bpf-next.git (net)
by Martin KaFai Lau <[e-mail]>:

On Wed, 11 Sep 2024 11:37:14 +0800 you wrote:
> This makes bpf_dynptr_from_skb usable for tp_btf, so that we can easily
> parse skb in tracepoints. This has been discussed in [0], and Martin
> suggested to use dynptr (instead of helpers like bpf_skb_load_bytes).
> 
> For safety, skb dynptr shouldn't be used in fentry/fexit. This is achieved
> by add KF_TRUSTED_ARGS flag in bpf_dynptr_from_skb defination, because
> pointers passed by tracepoint are trusted (PTR_TRUSTED) while those of
> fentry/fexit are not.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v3,1/5] bpf: Support __nullable argument suffix for tp_btf
    https://git.kernel.org/bpf/bpf-next/c/8aeaed21befc
  - [bpf-next,v3,2/5] selftests/bpf: Add test for __nullable suffix in tp_btf
    https://git.kernel.org/bpf/bpf-next/c/2060f07f861a
  - [bpf-next,v3,3/5] tcp: Use skb__nullable in trace_tcp_send_reset
    https://git.kernel.org/bpf/bpf-next/c/edd3f6f7588c
  - [bpf-next,v3,4/5] bpf: Allow bpf_dynptr_from_skb() for tp_btf
    https://git.kernel.org/bpf/bpf-next/c/ffc83860d8c0
  - [bpf-next,v3,5/5] selftests/bpf: Expand skb dynptr selftests for tp_btf
    https://git.kernel.org/bpf/bpf-next/c/83dff601715b

You are awesome, thank you!