Commit fdfd9d82 authored by Martin KaFai Lau's avatar Martin KaFai Lau
Browse files

Merge branch 'bpf: Allow skb dynptr for tp_btf' 

Philo Lu says:

====================
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/20240205121038.41344-1-lulie@linux.alibaba.com/T/
[1]
https://lore.kernel.org/all/20240430121805.104618-1-lulie@linux.alibaba.com/T/


====================

Signed-off-by: default avatarMartin KaFai Lau <martin.lau@kernel.org>
parents 23dc9867 83dff601

include/trace/events/tcp.h

+6 −6
Original line number Diff line number Diff line
@@ -91,10 +91,10 @@ DEFINE_RST_REASON(FN, FN) 
TRACE_EVENT(tcp_send_reset,



	TP_PROTO(const struct sock *sk,

		 const struct sk_buff *skb,

		 const struct sk_buff *skb__nullable,

		 const enum sk_rst_reason reason),



	TP_ARGS(sk, skb, reason),

	TP_ARGS(sk, skb__nullable, reason),



	TP_STRUCT__entry(

		__field(const void *, skbaddr)
@@ -106,7 +106,7 @@ TRACE_EVENT(tcp_send_reset, 
	),



	TP_fast_assign(

		__entry->skbaddr = skb;

		__entry->skbaddr = skb__nullable;

		__entry->skaddr = sk;

		/* Zero means unknown state. */

		__entry->state = sk ? sk->sk_state : 0;
@@ -118,13 +118,13 @@ TRACE_EVENT(tcp_send_reset, 
			const struct inet_sock *inet = inet_sk(sk);



			TP_STORE_ADDR_PORTS(__entry, inet, sk);

		} else if (skb) {

			const struct tcphdr *th = (const struct tcphdr *)skb->data;

		} else if (skb__nullable) {

			const struct tcphdr *th = (const struct tcphdr *)skb__nullable->data;

			/*

			 * We should reverse the 4-tuple of skb, so later

			 * it can print the right flow direction of rst.

			 */

			TP_STORE_ADDR_PORTS_SKB(skb, th, entry->daddr, entry->saddr);

			TP_STORE_ADDR_PORTS_SKB(skb__nullable, th, entry->daddr, entry->saddr);

		}

		__entry->reason = reason;

	),

kernel/bpf/btf.c

+3 −0
Original line number Diff line number Diff line
@@ -6523,6 +6523,9 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, 
	if (prog_args_trusted(prog))

		info->reg_type |= PTR_TRUSTED;



	if (btf_param_match_suffix(btf, &args[arg], "__nullable"))

		info->reg_type |= PTR_MAYBE_NULL;



	if (tgt_prog) {

		enum bpf_prog_type tgt_type;

kernel/bpf/verifier.c

+32 −4
Original line number Diff line number Diff line
@@ -28,6 +28,8 @@ 
#include <linux/cpumask.h>

#include <linux/bpf_mem_alloc.h>

#include <net/xdp.h>

#include <linux/trace_events.h>

#include <linux/kallsyms.h>









#include "disasm.h"
















@@ -21154,11 +21156,13 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,







{









	bool prog_extension = prog->type == BPF_PROG_TYPE_EXT;









	bool prog_tracing = prog->type == BPF_PROG_TYPE_TRACING;









	char trace_symbol[KSYM_SYMBOL_LEN];









	const char prefix[] = "btf_trace_";









	struct bpf_raw_event_map *btp;









	int ret = 0, subprog = -1, i;









	const struct btf_type *t;









	bool conservative = true;









	const char *tname;









	const char *tname, *fname;









	struct btf *btf;









	long addr = 0;









	struct module *mod = NULL;








@@ -21289,10 +21293,34 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,







			return -EINVAL;









		}









		tname += sizeof(prefix) - 1;

















		/* The func_proto of "btf_trace_##tname" is generated from typedef without argument









		 * names. Thus using bpf_raw_event_map to get argument names.









		 */









		btp = bpf_get_raw_tracepoint(tname);









		if (!btp)









			return -EINVAL;









		fname = kallsyms_lookup((unsigned long)btp->bpf_func, NULL, NULL, NULL,









					trace_symbol);









		bpf_put_raw_tracepoint(btp);

















		if (fname)









			ret = btf_find_by_name_kind(btf, fname, BTF_KIND_FUNC);

















		if (!fname || ret < 0) {









			bpf_log(log, "Cannot find btf of tracepoint template, fall back to %s%s.\n",









				prefix, tname);









			t = btf_type_by_id(btf, t->type);









			if (!btf_type_is_ptr(t))









				/* should never happen in valid vmlinux build */









				return -EINVAL;









		} else {









			t = btf_type_by_id(btf, ret);









			if (!btf_type_is_func(t))









				/* should never happen in valid vmlinux build */









				return -EINVAL;









		}

















		t = btf_type_by_id(btf, t->type);









		if (!btf_type_is_func_proto(t))









			/* should never happen in valid vmlinux build */

























net/core/filter.c



+2
−1






























Original line number
Diff line number
Diff line







@@ -12063,7 +12063,7 @@ int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags,







}



















BTF_KFUNCS_START(bpf_kfunc_check_set_skb)









BTF_ID_FLAGS(func, bpf_dynptr_from_skb)









BTF_ID_FLAGS(func, bpf_dynptr_from_skb, KF_TRUSTED_ARGS)









BTF_KFUNCS_END(bpf_kfunc_check_set_skb)



















BTF_KFUNCS_START(bpf_kfunc_check_set_xdp)








@@ -12112,6 +12112,7 @@ static int __init bpf_kfunc_init(void)







	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_XMIT, &bpf_kfunc_set_skb);









	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_LWT_SEG6LOCAL, &bpf_kfunc_set_skb);









	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_NETFILTER, &bpf_kfunc_set_skb);









	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_kfunc_set_skb);









	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_XDP, &bpf_kfunc_set_xdp);









	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_CGROUP_SOCK_ADDR,









					       &bpf_kfunc_set_sock_addr);
































tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h



+6
−0






























Original line number
Diff line number
Diff line







@@ -34,6 +34,12 @@ DECLARE_TRACE(bpf_testmod_test_write_bare,







	TP_ARGS(task, ctx)









);



















/* Used in bpf_testmod_test_read() to test __nullable suffix */









DECLARE_TRACE(bpf_testmod_test_nullable_bare,









	TP_PROTO(struct bpf_testmod_test_read_ctx *ctx__nullable),









	TP_ARGS(ctx__nullable)









);



















#undef BPF_TESTMOD_DECLARE_TRACE









#ifdef DECLARE_TRACE_WRITABLE









#define BPF_TESTMOD_DECLARE_TRACE(call, proto, args, size) \