Date:   Thu, 24 Oct 2019 13:15:24 -0700
From:   Martin KaFai Lau <kafai@...com>
To:     <bpf@...r.kernel.org>, <netdev@...r.kernel.org>
CC:     Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        David Miller <davem@...emloft.net>, <kernel-team@...com>
Subject: [PATCH bpf-next] bpf: Prepare btf_ctx_access for non raw_tp use case.

This patch makes a few changes to btf_ctx_access() to prepare
it for non raw_tp use case.

btf_ctx_access() only needs the attach_btf_id from prog.  Hence, this patch
only passes the attach_btf_id instead of passing prog.  It allows other
use cases when the prog->aux->attach_btf_id may not be a typedef.
For example, in the future, a bpf_prog can attach to
"struct tcp_congestion_ops" and its attach_btf_id is
pointing to the btf_id of "struct tcp_congestion_ops".

While at it, allow btf_ctx_access to directly take a BTF_KIND_FUNC_PROTO
btf_id.  It is to prepare for a later patch that does not need a "typedef"
to figure out the func_proto.  For example, when attaching a bpf_prog
to an ops in "struct tcp_congestion_ops", the func_proto can be
found directly by following the members of "struct tcp_congestion_ops".

For the no typedef use case, there is no extra first arg.  Hence, this
patch only limits the skip arg logic to raw_tp only.

Since a BTF_KIND_FUNC_PROTO type does not have a name (i.e. "(anon)"),
an optional name arg is added also.  If specified, this name will be used
in the bpf_log() message instead of the type's name obtained from btf_id.
For example, the function pointer member name of
"struct tcp_congestion_ops" can be used.

Signed-off-by: Martin KaFai Lau <kafai@...com>
---
 include/linux/bpf.h      |  2 +-
 kernel/bpf/btf.c         | 53 ++++++++++++++++++++++++++--------------
 kernel/trace/bpf_trace.c |  3 ++-
 3 files changed, 38 insertions(+), 20 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 2c2c29b49845..1befe59331d9 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -766,7 +766,7 @@ int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
 				     const union bpf_attr *kattr,
 				     union bpf_attr __user *uattr);
 bool btf_ctx_access(int off, int size, enum bpf_access_type type,
-		    const struct bpf_prog *prog,
+		    u32 btf_id, const char *name,
 		    struct bpf_insn_access_aux *info);
 int btf_struct_access(struct bpf_verifier_log *log,
 		      const struct btf_type *t, int off, int size,
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index f7557af39756..8c8174782675 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -346,6 +346,11 @@ static bool btf_type_is_func_proto(const struct btf_type *t)
 	return BTF_INFO_KIND(t->info) == BTF_KIND_FUNC_PROTO;
 }
 
+static bool btf_type_is_typedef(const struct btf_type *t)
+{
+	return BTF_INFO_KIND(t->info) == BTF_KIND_TYPEDEF;
+}
+
 static bool btf_type_nosize(const struct btf_type *t)
 {
 	return btf_type_is_void(t) || btf_type_is_fwd(t) ||
@@ -3439,16 +3444,16 @@ struct btf *btf_parse_vmlinux(void)
 extern struct btf *btf_vmlinux;
 
 bool btf_ctx_access(int off, int size, enum bpf_access_type type,
-		    const struct bpf_prog *prog,
+		    u32 btf_id, const char *func_name,
 		    struct bpf_insn_access_aux *info)
 {
 	struct bpf_verifier_log *log = info->log;
-	u32 btf_id = prog->aux->attach_btf_id;
 	const struct btf_param *args;
 	const struct btf_type *t;
 	const char prefix[] = "btf_trace_";
 	const char *tname;
 	u32 nr_args, arg;
+	bool is_raw_tp = false;
 
 	if (!btf_id)
 		return true;
@@ -3459,37 +3464,49 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 	}
 
 	t = btf_type_by_id(btf_vmlinux, btf_id);
-	if (!t || BTF_INFO_KIND(t->info) != BTF_KIND_TYPEDEF) {
+	if (!t || (!btf_type_is_typedef(t) && !btf_type_is_func_proto(t))) {
 		bpf_log(log, "btf_id is invalid\n");
 		return false;
 	}
 
 	tname = __btf_name_by_offset(btf_vmlinux, t->name_off);
-	if (strncmp(prefix, tname, sizeof(prefix) - 1)) {
-		bpf_log(log, "btf_id points to wrong type name %s\n", tname);
-		return false;
+	if (btf_type_is_typedef(t)) {
+		if (strncmp(prefix, tname, sizeof(prefix) - 1)) {
+			bpf_log(log, "btf_id points to wrong type name %s\n",
+				tname);
+			return false;
+		}
+		tname += sizeof(prefix) - 1;
+
+		t = btf_type_by_id(btf_vmlinux, t->type);
+		if (!btf_type_is_ptr(t))
+			return false;
+		t = btf_type_by_id(btf_vmlinux, t->type);
+		btf_id = t->type;
+		is_raw_tp = true;
 	}
-	tname += sizeof(prefix) - 1;
 
-	t = btf_type_by_id(btf_vmlinux, t->type);
-	if (!btf_type_is_ptr(t))
-		return false;
-	t = btf_type_by_id(btf_vmlinux, t->type);
 	if (!btf_type_is_func_proto(t))
 		return false;
 
+	if (func_name)
+		tname = func_name;
+
 	if (off % 8) {
-		bpf_log(log, "raw_tp '%s' offset %d is not multiple of 8\n",
+		bpf_log(log, "func '%s' offset %d is not multiple of 8\n",
 			tname, off);
 		return false;
 	}
 	arg = off / 8;
 	args = (const struct btf_param *)(t + 1);
+	nr_args = btf_type_vlen(t);
 	/* skip first 'void *__data' argument in btf_trace_##name typedef */
-	args++;
-	nr_args = btf_type_vlen(t) - 1;
+	if (is_raw_tp) {
+		args++;
+		nr_args--;
+	}
 	if (arg >= nr_args) {
-		bpf_log(log, "raw_tp '%s' doesn't have %d-th argument\n",
+		bpf_log(log, "func '%s' doesn't have %d-th argument\n",
 			tname, arg);
 		return false;
 	}
@@ -3503,7 +3520,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 		return true;
 	if (!btf_type_is_ptr(t)) {
 		bpf_log(log,
-			"raw_tp '%s' arg%d '%s' has type %s. Only pointer access is allowed\n",
+			"func '%s' arg%d '%s' has type %s. Only pointer access is allowed\n",
 			tname, arg,
 			__btf_name_by_offset(btf_vmlinux, t->name_off),
 			btf_kind_str[BTF_INFO_KIND(t->info)]);
@@ -3526,11 +3543,11 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 		t = btf_type_by_id(btf_vmlinux, t->type);
 	if (!btf_type_is_struct(t)) {
 		bpf_log(log,
-			"raw_tp '%s' arg%d type %s is not a struct\n",
+			"func '%s' arg%d type %s is not a struct\n",
 			tname, arg, btf_kind_str[BTF_INFO_KIND(t->info)]);
 		return false;
 	}
-	bpf_log(log, "raw_tp '%s' arg%d has btf_id %d type %s '%s'\n",
+	bpf_log(log, "func '%s' arg%d has btf_id %d type %s '%s'\n",
 		tname, arg, info->btf_id, btf_kind_str[BTF_INFO_KIND(t->info)],
 		__btf_name_by_offset(btf_vmlinux, t->name_off));
 	return true;
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index c3240898cc44..435869b1834a 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1080,7 +1080,8 @@ static bool raw_tp_prog_is_valid_access(int off, int size,
 		return false;
 	if (off % size != 0)
 		return false;
-	return btf_ctx_access(off, size, type, prog, info);
+	return btf_ctx_access(off, size, type, prog->aux->attach_btf_id, NULL,
+			      info);
 }
 
 const struct bpf_verifier_ops raw_tracepoint_verifier_ops = {
-- 
2.17.1

Powered by blists - more mailing lists