[PATCH bpf-next v4 00/13] bpf: add btf func info support

[PATCH bpf-next v4 00/13] bpf: add btf func info support

[Yonghong Song]

The BTF support was added to kernel by Commit 69b693f0aefa
("bpf: btf: Introduce BPF Type Format (BTF)"), which introduced
.BTF section into ELF file and is primarily
used for map pretty print.
pahole is used to convert dwarf to BTF for ELF files.

This patch added func info support to the kernel so we can
get better ksym's for bpf function calls. Basically,
function call types are passed to kernel and the kernel
extract function names from these types in order to contruct ksym
for these functions.

The llvm patch at https://reviews.llvm.org/D53736
will generate .BTF section and one more section .BTF.ext.
The .BTF.ext section encodes function type
information. The following is a sample output for selftests
test_btf with file test_btf_haskv.o for translated insns
and jited insns respectively.

  $ bpftool prog dump xlated id 1
  int _dummy_tracepoint(struct dummy_tracepoint_args * arg):
     0: (85) call pc+2#bpf_prog_2dcecc18072623fc_test_long_fname_1
     1: (b7) r0 = 0
     2: (95) exit
  int test_long_fname_1(struct dummy_tracepoint_args * arg):
     3: (85) call pc+1#bpf_prog_89d64e4abf0f0126_test_long_fname_2
     4: (95) exit
  int test_long_fname_2(struct dummy_tracepoint_args * arg):
     5: (b7) r2 = 0
     6: (63) *(u32 *)(r10 -4) = r2
     7: (79) r1 = *(u64 *)(r1 +8)
     ...
     22: (07) r1 += 1
     23: (63) *(u32 *)(r0 +4) = r1
     24: (95) exit

  $ bpftool prog dump jited id 1
  int _dummy_tracepoint(struct dummy_tracepoint_args * arg):
  bpf_prog_b07ccb89267cf242__dummy_tracepoint:
     0:   push   %rbp
     1:   mov    %rsp,%rbp
    ......
    3c:   add    $0x28,%rbp
    40:   leaveq
    41:   retq
    
  int test_long_fname_1(struct dummy_tracepoint_args * arg):
  bpf_prog_2dcecc18072623fc_test_long_fname_1:
     0:   push   %rbp
     1:   mov    %rsp,%rbp
    ......
    3a:   add    $0x28,%rbp
    3e:   leaveq
    3f:   retq
    
  int test_long_fname_2(struct dummy_tracepoint_args * arg):
  bpf_prog_89d64e4abf0f0126_test_long_fname_2:
     0:   push   %rbp
     1:   mov    %rsp,%rbp
    ......
    80:   add    $0x28,%rbp
    84:   leaveq
    85:   retq

For the patchset,
Patch #1  refactors the code to break up btf_type_is_void().
Patch #2  introduces new BTF type BTF_KIND_FUNC.
Patch #3  syncs btf.h header to tools directory.
Patch #4  adds btf func type self tests in test_btf.
Patch #5  adds kernel interface to load func_info to kernel
          and pass func_info back to userspace.
Patch #6  syncs bpf.h header to tools directory.
Patch #7  adds news btf/func_info related fields in libbpf
          program load function.
Patch #8  extends selftest test_btf to test load/retrieve func_info.
Patch #9  adds .BTF.ext func info support.
Patch #10 changes Makefile to avoid using pahole if llvm is capable of
          generating BTF sections.
Patch #11 refactors to have btf_get_from_id() in libbpf for reuse.
Patch #12 enhance test_btf file testing to test func info.
Patch #13 adds bpftool support for func signature dump.

Changelogs:
  v3 -> v4:
    . Remove BTF_KIND_FUNC_PROTO. BTF_KIND_FUNC is used for
      both function pointer and subprogram. The name_off field
      is used to distinguish both.
    . The record size is added to the func_info subsection
      in .BTF.ext to enable future extension.
    . The bpf_prog_info interface change to make it similar
      bpf_prog_load.
    . Related kernel and libbpf changes to accommodate the
      new .BTF.ext and kernel interface changes.
  v2 -> v3:
    . Removed kernel btf extern functions btf_type_id_func()
      and btf_get_name_by_id(). Instead, exposing existing
      functions btf_type_by_id() and btf_name_by_offset().
    . Added comments about ELF section .BTF.ext layout.
    . Better codes in btftool as suggested by Edward Cree.
  v1 -> v2:
    . Added missing sign-off.
    . Limited the func_name/struct_member_name length for validity test.
    . Removed/changed several verifier messages.
    . Modified several commit messages to remove line_off reference.

Yonghong Song (13):
  bpf: btf: Break up btf_type_is_void()
  bpf: btf: Add BTF_KIND_FUNC
  tools/bpf: sync kernel btf.h header
  tools/bpf: add btf func unit tests in selftest test_btf
  bpf: get better bpf_prog ksyms based on btf func type_id
  tools/bpf: sync kernel uapi bpf.h header to tools directory
  tools/bpf: add new fields for program load in lib/bpf
  tools/bpf: extends test_btf to test load/retrieve func_type info
  tools/bpf: add support to read .BTF.ext sections
  tools/bpf: do not use pahole if clang/llvm can generate BTF sections
  tools/bpf: refactor to implement btf_get_from_id() in lib/bpf
  tools/bpf: enhance test_btf file testing to test func info
  tools/bpf: bpftool: add support for func types

 include/linux/bpf.h                          |   5 +-
 include/linux/bpf_verifier.h                 |   1 +
 include/linux/btf.h                          |   2 +
 include/uapi/linux/bpf.h                     |  13 +
 include/uapi/linux/btf.h                     |  17 +-
 kernel/bpf/btf.c                             | 342 +++++++--
 kernel/bpf/core.c                            |  13 +
 kernel/bpf/syscall.c                         |  59 +-
 kernel/bpf/verifier.c                        | 121 +++-
 samples/bpf/Makefile                         |   8 +
 tools/bpf/bpftool/btf_dumper.c               |  98 +++
 tools/bpf/bpftool/main.h                     |   2 +
 tools/bpf/bpftool/map.c                      |  68 +-
 tools/bpf/bpftool/prog.c                     |  56 ++
 tools/bpf/bpftool/xlated_dumper.c            |  33 +
 tools/bpf/bpftool/xlated_dumper.h            |   3 +
 tools/include/uapi/linux/bpf.h               |  13 +
 tools/include/uapi/linux/btf.h               |  17 +-
 tools/lib/bpf/bpf.c                          |  50 +-
 tools/lib/bpf/bpf.h                          |   4 +
 tools/lib/bpf/btf.c                          | 346 +++++++++
 tools/lib/bpf/btf.h                          |  51 ++
 tools/lib/bpf/libbpf.c                       |  87 ++-
 tools/testing/selftests/bpf/Makefile         |   8 +
 tools/testing/selftests/bpf/test_btf.c       | 694 ++++++++++++++++++-
 tools/testing/selftests/bpf/test_btf_haskv.c |  16 +-
 tools/testing/selftests/bpf/test_btf_nokv.c  |  16 +-
 27 files changed, 1988 insertions(+), 155 deletions(-)

-- 
2.17.1

[PATCH bpf-next v4 01/13] bpf: btf: Break up btf_type_is_void()

[Yonghong Song]

This patch breaks up btf_type_is_void() into
btf_type_is_void() and btf_type_is_fwd().

It also adds btf_type_nosize() to better describe it is
testing a type has nosize info.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 kernel/bpf/btf.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index ee4c82667d65..2a50d87de485 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -306,15 +306,22 @@ static bool btf_type_is_modifier(const struct btf_type *t)
 
 static bool btf_type_is_void(const struct btf_type *t)
 {
-	/* void => no type and size info.
-	 * Hence, FWD is also treated as void.
-	 */
-	return t == &btf_void || BTF_INFO_KIND(t->info) == BTF_KIND_FWD;
+	return t == &btf_void;
+}
+
+static bool btf_type_is_fwd(const struct btf_type *t)
+{
+	return BTF_INFO_KIND(t->info) == BTF_KIND_FWD;
+}
+
+static bool btf_type_nosize(const struct btf_type *t)
+{
+	return btf_type_is_void(t) || btf_type_is_fwd(t);
 }
 
-static bool btf_type_is_void_or_null(const struct btf_type *t)
+static bool btf_type_nosize_or_null(const struct btf_type *t)
 {
-	return !t || btf_type_is_void(t);
+	return !t || btf_type_nosize(t);
 }
 
 /* union is only a special case of struct:
@@ -826,7 +833,7 @@ const struct btf_type *btf_type_id_size(const struct btf *btf,
 	u32 size = 0;
 
 	size_type = btf_type_by_id(btf, size_type_id);
-	if (btf_type_is_void_or_null(size_type))
+	if (btf_type_nosize_or_null(size_type))
 		return NULL;
 
 	if (btf_type_has_size(size_type)) {
@@ -842,7 +849,7 @@ const struct btf_type *btf_type_id_size(const struct btf *btf,
 		size = btf->resolved_sizes[size_type_id];
 		size_type_id = btf->resolved_ids[size_type_id];
 		size_type = btf_type_by_id(btf, size_type_id);
-		if (btf_type_is_void(size_type))
+		if (btf_type_nosize_or_null(size_type))
 			return NULL;
 	}
 
@@ -1164,7 +1171,7 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
 	}
 
 	/* "typedef void new_void", "const void"...etc */
-	if (btf_type_is_void(next_type))
+	if (btf_type_is_void(next_type) || btf_type_is_fwd(next_type))
 		goto resolved;
 
 	if (!env_type_is_resolve_sink(env, next_type) &&
@@ -1178,7 +1185,7 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
 	 * pretty print).
 	 */
 	if (!btf_type_id_size(btf, &next_type_id, &next_type_size) &&
-	    !btf_type_is_void(btf_type_id_resolve(btf, &next_type_id))) {
+	    !btf_type_nosize(btf_type_id_resolve(btf, &next_type_id))) {
 		btf_verifier_log_type(env, v->t, "Invalid type_id");
 		return -EINVAL;
 	}
@@ -1205,7 +1212,7 @@ static int btf_ptr_resolve(struct btf_verifier_env *env,
 	}
 
 	/* "void *" */
-	if (btf_type_is_void(next_type))
+	if (btf_type_is_void(next_type) || btf_type_is_fwd(next_type))
 		goto resolved;
 
 	if (!env_type_is_resolve_sink(env, next_type) &&
@@ -1235,7 +1242,7 @@ static int btf_ptr_resolve(struct btf_verifier_env *env,
 	}
 
 	if (!btf_type_id_size(btf, &next_type_id, &next_type_size) &&
-	    !btf_type_is_void(btf_type_id_resolve(btf, &next_type_id))) {
+	    !btf_type_nosize(btf_type_id_resolve(btf, &next_type_id))) {
 		btf_verifier_log_type(env, v->t, "Invalid type_id");
 		return -EINVAL;
 	}
@@ -1396,7 +1403,7 @@ static int btf_array_resolve(struct btf_verifier_env *env,
 	/* Check array->index_type */
 	index_type_id = array->index_type;
 	index_type = btf_type_by_id(btf, index_type_id);
-	if (btf_type_is_void_or_null(index_type)) {
+	if (btf_type_nosize_or_null(index_type)) {
 		btf_verifier_log_type(env, v->t, "Invalid index");
 		return -EINVAL;
 	}
@@ -1415,7 +1422,7 @@ static int btf_array_resolve(struct btf_verifier_env *env,
 	/* Check array->type */
 	elem_type_id = array->type;
 	elem_type = btf_type_by_id(btf, elem_type_id);
-	if (btf_type_is_void_or_null(elem_type)) {
+	if (btf_type_nosize_or_null(elem_type)) {
 		btf_verifier_log_type(env, v->t,
 				      "Invalid elem");
 		return -EINVAL;
@@ -1615,7 +1622,7 @@ static int btf_struct_resolve(struct btf_verifier_env *env,
 		const struct btf_type *member_type = btf_type_by_id(env->btf,
 								member_type_id);
 
-		if (btf_type_is_void_or_null(member_type)) {
+		if (btf_type_nosize_or_null(member_type)) {
 			btf_verifier_log_member(env, v->t, member,
 						"Invalid member");
 			return -EINVAL;
-- 
2.17.1

[PATCH bpf-next v4 02/13] bpf: btf: Add BTF_KIND_FUNC

[Yonghong Song]

This patch adds BTF_KIND_FUNC support to the type section.
BTF_KIND_FUNC is used to specify the signature of a
defined subprogram or the pointee of a function pointer.

In BTF, the function type related data structures are
  struct bpf_param {
    __u32 name_off; /* parameter name */
    __u32 type; /* parameter type */
  };
  struct bpf_type {
    __u32 name_off; /* function name */
    __u32 info; /* BTF_KIND_FUNC and num of parameters (#vlen) */
    __u32 type; /* return type */
  }
The data layout of the function type:
  struct bpf_type
  #vlen number of bpf_param's

For a defined subprogram with valid function body,
  . function name and all parameter names except the vararg
    must be valid C identifier.
For the pointee of a function pointer,
  . function name and all parameter names will
have name_off = 0 to indicate a non-existing name.

As a concrete example, for the C program below,
  int foo(int (*bar)(int)) { return bar(5); }
two func types will be generated:
  FuncType #1: subprogram "foo" with parameter "bar"
  FuncType #2: pointee of function pointer "int (*)(int)"

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
 include/uapi/linux/btf.h |  17 ++-
 kernel/bpf/btf.c         | 309 ++++++++++++++++++++++++++++++++++-----
 2 files changed, 290 insertions(+), 36 deletions(-)

diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
index 972265f32871..9e7c74a83ee7 100644
--- a/include/uapi/linux/btf.h
+++ b/include/uapi/linux/btf.h
@@ -40,7 +40,8 @@ struct btf_type {
 	/* "size" is used by INT, ENUM, STRUCT and UNION.
 	 * "size" tells the size of the type it is describing.
 	 *
-	 * "type" is used by PTR, TYPEDEF, VOLATILE, CONST and RESTRICT.
+	 * "type" is used by PTR, TYPEDEF, VOLATILE, CONST, RESTRICT
+	 * and FUNC.
 	 * "type" is a type_id referring to another type.
 	 */
 	union {
@@ -64,8 +65,9 @@ struct btf_type {
 #define BTF_KIND_VOLATILE	9	/* Volatile	*/
 #define BTF_KIND_CONST		10	/* Const	*/
 #define BTF_KIND_RESTRICT	11	/* Restrict	*/
-#define BTF_KIND_MAX		11
-#define NR_BTF_KINDS		12
+#define BTF_KIND_FUNC		12	/* Function	*/
+#define BTF_KIND_MAX		12
+#define NR_BTF_KINDS		13
 
 /* For some specific BTF_KIND, "struct btf_type" is immediately
  * followed by extra data.
@@ -110,4 +112,13 @@ struct btf_member {
 	__u32	offset;	/* offset in bits */
 };
 
+/* BTF_KIND_FUNC is followed by multiple "struct btf_param".
+ * The exact number of btf_param is stored in the vlen (of the
+ * info in "struct btf_type").
+ */
+struct btf_param {
+	__u32	name_off;
+	__u32	type;
+};
+
 #endif /* _UAPI__LINUX_BTF_H__ */
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 2a50d87de485..62140da0c10d 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5,6 +5,7 @@
 #include <uapi/linux/types.h>
 #include <linux/seq_file.h>
 #include <linux/compiler.h>
+#include <linux/ctype.h>
 #include <linux/errno.h>
 #include <linux/slab.h>
 #include <linux/anon_inodes.h>
@@ -259,6 +260,7 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
 	[BTF_KIND_VOLATILE]	= "VOLATILE",
 	[BTF_KIND_CONST]	= "CONST",
 	[BTF_KIND_RESTRICT]	= "RESTRICT",
+	[BTF_KIND_FUNC]		= "FUNC",
 };
 
 struct btf_kind_operations {
@@ -281,6 +283,9 @@ struct btf_kind_operations {
 static const struct btf_kind_operations * const kind_ops[NR_BTF_KINDS];
 static struct btf_type btf_void;
 
+static int btf_resolve(struct btf_verifier_env *env,
+		       const struct btf_type *t, u32 type_id);
+
 static bool btf_type_is_modifier(const struct btf_type *t)
 {
 	/* Some of them is not strictly a C modifier
@@ -314,9 +319,27 @@ static bool btf_type_is_fwd(const struct btf_type *t)
 	return BTF_INFO_KIND(t->info) == BTF_KIND_FWD;
 }
 
+static bool btf_type_is_func(const struct btf_type *t)
+{
+	return BTF_INFO_KIND(t->info) == BTF_KIND_FUNC;
+}
+
+static bool btf_type_is_func_prog(const struct btf_type *t)
+{
+	return BTF_INFO_KIND(t->info) == BTF_KIND_FUNC &&
+	       t->name_off;
+}
+
+static bool btf_type_is_func_proto(const struct btf_type *t)
+{
+	return BTF_INFO_KIND(t->info) == BTF_KIND_FUNC &&
+	       !t->name_off;
+}
+
 static bool btf_type_nosize(const struct btf_type *t)
 {
-	return btf_type_is_void(t) || btf_type_is_fwd(t);
+	return btf_type_is_void(t) || btf_type_is_fwd(t) ||
+	       btf_type_is_func(t);
 }
 
 static bool btf_type_nosize_or_null(const struct btf_type *t)
@@ -433,6 +456,27 @@ static bool btf_name_offset_valid(const struct btf *btf, u32 offset)
 		offset < btf->hdr.str_len;
 }
 
+static bool btf_name_valid_identifier(const struct btf *btf, u32 offset)
+{
+	/* offset must be valid */
+	const char *src = &btf->strings[offset];
+	const char *src_limit;
+
+	if (!isalpha(*src) && *src != '_')
+		return false;
+
+	/* set a limit on identifier length */
+	src_limit = src + KSYM_NAME_LEN;
+	src++;
+	while (*src && src < src_limit) {
+		if (!isalnum(*src) && *src != '_')
+			return false;
+		src++;
+	}
+
+	return !*src;
+}
+
 static const char *btf_name_by_offset(const struct btf *btf, u32 offset)
 {
 	if (!offset)
@@ -747,7 +791,9 @@ static bool env_type_is_resolve_sink(const struct btf_verifier_env *env,
 		/* int, enum or void is a sink */
 		return !btf_type_needs_resolve(next_type);
 	case RESOLVE_PTR:
-		/* int, enum, void, struct or array is a sink for ptr */
+		/* int, enum, void, struct, array or func_proto is a sink
+		 * for ptr
+		 */
 		return !btf_type_is_modifier(next_type) &&
 			!btf_type_is_ptr(next_type);
 	case RESOLVE_STRUCT_OR_ARRAY:
@@ -1170,6 +1216,14 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
 		return -EINVAL;
 	}
 
+	if (btf_type_is_func_proto(next_type)) {
+		if (env->resolve_mode == RESOLVE_PTR)
+			goto resolved;
+
+		btf_verifier_log_type(env, v->t, "Invalid type_id");
+		return -EINVAL;
+	}
+
 	/* "typedef void new_void", "const void"...etc */
 	if (btf_type_is_void(next_type) || btf_type_is_fwd(next_type))
 		goto resolved;
@@ -1185,7 +1239,8 @@ static int btf_modifier_resolve(struct btf_verifier_env *env,
 	 * pretty print).
 	 */
 	if (!btf_type_id_size(btf, &next_type_id, &next_type_size) &&
-	    !btf_type_nosize(btf_type_id_resolve(btf, &next_type_id))) {
+	    (!btf_type_nosize(btf_type_id_resolve(btf, &next_type_id)) ||
+	     btf_type_is_func_prog(next_type))) {
 		btf_verifier_log_type(env, v->t, "Invalid type_id");
 		return -EINVAL;
 	}
@@ -1212,7 +1267,8 @@ static int btf_ptr_resolve(struct btf_verifier_env *env,
 	}
 
 	/* "void *" */
-	if (btf_type_is_void(next_type) || btf_type_is_fwd(next_type))
+	if (btf_type_is_void(next_type) || btf_type_is_fwd(next_type) ||
+	    btf_type_is_func_proto(next_type))
 		goto resolved;
 
 	if (!env_type_is_resolve_sink(env, next_type) &&
@@ -1242,7 +1298,8 @@ static int btf_ptr_resolve(struct btf_verifier_env *env,
 	}
 
 	if (!btf_type_id_size(btf, &next_type_id, &next_type_size) &&
-	    !btf_type_nosize(btf_type_id_resolve(btf, &next_type_id))) {
+	    (!btf_type_nosize(btf_type_id_resolve(btf, &next_type_id)) ||
+	     btf_type_is_func_prog(next_type))) {
 		btf_verifier_log_type(env, v->t, "Invalid type_id");
 		return -EINVAL;
 	}
@@ -1550,6 +1607,14 @@ static s32 btf_struct_check_meta(struct btf_verifier_env *env,
 			return -EINVAL;
 		}
 
+		if (member->name_off &&
+		    !btf_name_valid_identifier(btf, member->name_off)) {
+			btf_verifier_log_member(env, t, member,
+						"Invalid member name:%s",
+						btf_name_by_offset(btf, member->name_off));
+			return -EINVAL;
+		}
+
 		/* A member cannot be in type void */
 		if (!member->type || !BTF_TYPE_ID_VALID(member->type)) {
 			btf_verifier_log_member(env, t, member,
@@ -1787,6 +1852,174 @@ static struct btf_kind_operations enum_ops = {
 	.seq_show = btf_enum_seq_show,
 };
 
+static s32 btf_func_check_meta(struct btf_verifier_env *env,
+			       const struct btf_type *t,
+			       u32 meta_left)
+{
+	u32 meta_needed = btf_type_vlen(t) * sizeof(struct btf_param);
+
+	if (meta_left < meta_needed) {
+		btf_verifier_log_basic(env, t,
+				       "meta_left:%u meta_needed:%u",
+				       meta_left, meta_needed);
+		return -EINVAL;
+	}
+
+	btf_verifier_log_type(env, t, NULL);
+
+	return meta_needed;
+}
+
+static void btf_func_log(struct btf_verifier_env *env,
+			 const struct btf_type *t)
+{
+	u16 nr_args = btf_type_vlen(t), i;
+	struct btf_param *args = (struct btf_param *)(t + 1);
+
+	btf_verifier_log(env, "return=%u args=(", t->type);
+	if (!nr_args) {
+		btf_verifier_log(env, "void");
+		goto done;
+	}
+
+	if (nr_args == 1 && !args[0].type) {
+		/* Only one vararg */
+		btf_verifier_log(env, "vararg");
+		goto done;
+	}
+
+	btf_verifier_log(env, "%u %s", args[0].type,
+			 btf_name_by_offset(env->btf,
+					    args[0].name_off));
+	for (i = 1; i < nr_args - 1; i++)
+		btf_verifier_log(env, ", %u %s", args[i].type,
+				 btf_name_by_offset(env->btf,
+						    args[i].name_off));
+
+	if (nr_args > 1) {
+		struct btf_param *last_arg = &args[nr_args - 1];
+
+		if (last_arg->type)
+			btf_verifier_log(env, ", %u %s", last_arg->type,
+					 btf_name_by_offset(env->btf,
+							    last_arg->name_off));
+		else
+			btf_verifier_log(env, ", vararg");
+	}
+
+done:
+	btf_verifier_log(env, ")");
+}
+
+static int btf_func_check(struct btf_verifier_env *env,
+			  const struct btf_type *t,
+			  u32 type_id)
+{
+	u16 nr_args = btf_type_vlen(t), i;
+	const struct btf *btf = env->btf;
+	const struct btf_type *ret_type;
+	struct btf_param *args = (struct btf_param *)(t + 1);
+	u32 ret_type_id;
+	int err;
+
+	/* Check non-0 name_off must point to valid identifier */
+	if (t->name_off &&
+	    !btf_name_valid_identifier(btf, t->name_off)) {
+		btf_verifier_log_type(env, t, "Invalid type_id");
+		return -EINVAL;
+	}
+
+	/* Check func return type which could be "void" (t->type == 0) */
+	ret_type_id = t->type;
+	if (ret_type_id) {
+		/* return type is not "void" */
+		ret_type = btf_type_by_id(btf, ret_type_id);
+		if (btf_type_needs_resolve(ret_type) &&
+		    !env_type_is_resolved(env, ret_type_id)) {
+			err = btf_resolve(env, ret_type, ret_type_id);
+			if (err)
+				return err;
+		}
+
+		/* Ensure the return type is a type that has a size */
+		if (!btf_type_id_size(btf, &ret_type_id, NULL)) {
+			btf_verifier_log_type(env, t, "Invalid return type");
+			return -EINVAL;
+		}
+	}
+
+	if (!nr_args)
+		return 0;
+
+	/* Last func arg type_id could be 0 if it is a vararg */
+	if (!args[nr_args - 1].type) {
+		if (args[nr_args - 1].name_off) {
+			btf_verifier_log_type(env, t, "Invalid arg#%u", nr_args);
+			return -EINVAL;
+		}
+		nr_args--;
+	}
+
+	err = 0;
+	for (i = 0; i < nr_args; i++) {
+		const struct btf_type *arg_type;
+		u32 arg_type_id;
+
+		arg_type_id = args[i].type;
+		arg_type = btf_type_by_id(btf, arg_type_id);
+		if (!arg_type ||
+		    /* func has name, all its args must have names.
+		     * func has no name, all its args cannot have names.
+		     */
+		    (!!args[i].name_off != !!t->name_off)) {
+			btf_verifier_log_type(env, t, "Invalid arg#%u", i + 1);
+			err = -EINVAL;
+			break;
+		}
+
+		if (args[i].name_off &&
+		    (!btf_name_offset_valid(btf, args[i].name_off) ||
+		     !btf_name_valid_identifier(btf, args[i].name_off))) {
+			btf_verifier_log_type(env, t,
+					      "Invalid arg#%u", i + 1);
+			err = -EINVAL;
+			break;
+		}
+
+		if (btf_type_needs_resolve(arg_type) &&
+		    !env_type_is_resolved(env, arg_type_id)) {
+			err = btf_resolve(env, arg_type, arg_type_id);
+			if (err)
+				break;
+		}
+
+		if (!btf_type_id_size(btf, &arg_type_id, NULL)) {
+			btf_verifier_log_type(env, t, "Invalid arg#%u", i + 1);
+			err = -EINVAL;
+			break;
+		}
+	}
+
+	return err;
+}
+
+static struct btf_kind_operations func_ops = {
+	.check_meta = btf_func_check_meta,
+	.resolve = btf_df_resolve,
+	/*
+	 * BTF_KIND_FUNC cannot be directly referred by
+	 * a struct's member.
+	 *
+	 * It should be a funciton pointer instead.
+	 * (i.e. struct's member -> BTF_KIND_PTR -> BTF_KIND_FUNC)
+	 *
+	 * Hence, there is no btf_func_check_member().
+	 */
+	.check_member = btf_df_check_member,
+	.log_details = btf_func_log,
+	.seq_show = btf_df_seq_show,
+};
+
 static const struct btf_kind_operations * const kind_ops[NR_BTF_KINDS] = {
 	[BTF_KIND_INT] = &int_ops,
 	[BTF_KIND_PTR] = &ptr_ops,
@@ -1799,6 +2032,7 @@ static const struct btf_kind_operations * const kind_ops[NR_BTF_KINDS] = {
 	[BTF_KIND_VOLATILE] = &modifier_ops,
 	[BTF_KIND_CONST] = &modifier_ops,
 	[BTF_KIND_RESTRICT] = &modifier_ops,
+	[BTF_KIND_FUNC] = &func_ops,
 };
 
 static s32 btf_check_meta(struct btf_verifier_env *env,
@@ -1870,30 +2104,6 @@ static int btf_check_all_metas(struct btf_verifier_env *env)
 	return 0;
 }
 
-static int btf_resolve(struct btf_verifier_env *env,
-		       const struct btf_type *t, u32 type_id)
-{
-	const struct resolve_vertex *v;
-	int err = 0;
-
-	env->resolve_mode = RESOLVE_TBD;
-	env_stack_push(env, t, type_id);
-	while (!err && (v = env_stack_peak(env))) {
-		env->log_type_id = v->type_id;
-		err = btf_type_ops(v->t)->resolve(env, v);
-	}
-
-	env->log_type_id = type_id;
-	if (err == -E2BIG)
-		btf_verifier_log_type(env, t,
-				      "Exceeded max resolving depth:%u",
-				      MAX_RESOLVE_DEPTH);
-	else if (err == -EEXIST)
-		btf_verifier_log_type(env, t, "Loop detected");
-
-	return err;
-}
-
 static bool btf_resolve_valid(struct btf_verifier_env *env,
 			      const struct btf_type *t,
 			      u32 type_id)
@@ -1927,6 +2137,39 @@ static bool btf_resolve_valid(struct btf_verifier_env *env,
 	return false;
 }
 
+static int btf_resolve(struct btf_verifier_env *env,
+		       const struct btf_type *t, u32 type_id)
+{
+	u32 save_log_type_id = env->log_type_id;
+	const struct resolve_vertex *v;
+	int err = 0;
+
+	env->resolve_mode = RESOLVE_TBD;
+	env_stack_push(env, t, type_id);
+	while (!err && (v = env_stack_peak(env))) {
+		env->log_type_id = v->type_id;
+		err = btf_type_ops(v->t)->resolve(env, v);
+	}
+
+	env->log_type_id = type_id;
+	if (err == -E2BIG) {
+		btf_verifier_log_type(env, t,
+				      "Exceeded max resolving depth:%u",
+				      MAX_RESOLVE_DEPTH);
+	} else if (err == -EEXIST) {
+		btf_verifier_log_type(env, t, "Loop detected");
+	}
+
+	/* Final sanity check */
+	if (!err && !btf_resolve_valid(env, t, type_id)) {
+		btf_verifier_log_type(env, t, "Invalid resolve state");
+		err = -EINVAL;
+	}
+
+	env->log_type_id = save_log_type_id;
+	return err;
+}
+
 static int btf_check_all_types(struct btf_verifier_env *env)
 {
 	struct btf *btf = env->btf;
@@ -1949,10 +2192,10 @@ static int btf_check_all_types(struct btf_verifier_env *env)
 				return err;
 		}
 
-		if (btf_type_needs_resolve(t) &&
-		    !btf_resolve_valid(env, t, type_id)) {
-			btf_verifier_log_type(env, t, "Invalid resolve state");
-			return -EINVAL;
+		if (btf_type_is_func(t)) {
+			err = btf_func_check(env, t, type_id);
+			if (err)
+				return err;
 		}
 	}
 
-- 
2.17.1

Re: [PATCH bpf-next v4 02/13] bpf: btf: Add BTF_KIND_FUNC

[Edward Cree]

On 08/11/18 20:36, Yonghong Song wrote:
> This patch adds BTF_KIND_FUNC support to the type section.
> BTF_KIND_FUNC is used to specify the signature of a
> defined subprogram or the pointee of a function pointer.
>
> In BTF, the function type related data structures are
>   struct bpf_param {
>     __u32 name_off; /* parameter name */
>     __u32 type; /* parameter type */
>   };
>   struct bpf_type {
>     __u32 name_off; /* function name */
>     __u32 info; /* BTF_KIND_FUNC and num of parameters (#vlen) */
>     __u32 type; /* return type */
>   }
> The data layout of the function type:
>   struct bpf_type
>   #vlen number of bpf_param's
>
> For a defined subprogram with valid function body,
>   . function name and all parameter names except the vararg
>     must be valid C identifier.
Given that there's an intention to support other frontends besides
 C, what's the reason for this restriction?

> For the pointee of a function pointer,
>   . function name and all parameter names will
> have name_off = 0 to indicate a non-existing name.
Why can't function pointer parameters have names?
E.g. imagine something like struct net_device_ops.  All those
 function pointers have named parameters and that's relevant info
 when debugging.

-Ed

Re: [PATCH bpf-next v4 02/13] bpf: btf: Add BTF_KIND_FUNC

[Yonghong Song]

On 11/8/18 12:52 PM, Edward Cree wrote:
> On 08/11/18 20:36, Yonghong Song wrote:
>> This patch adds BTF_KIND_FUNC support to the type section.
>> BTF_KIND_FUNC is used to specify the signature of a
>> defined subprogram or the pointee of a function pointer.
>>
>> In BTF, the function type related data structures are
>>    struct bpf_param {
>>      __u32 name_off; /* parameter name */
>>      __u32 type; /* parameter type */
>>    };
>>    struct bpf_type {
>>      __u32 name_off; /* function name */
>>      __u32 info; /* BTF_KIND_FUNC and num of parameters (#vlen) */
>>      __u32 type; /* return type */
>>    }
>> The data layout of the function type:
>>    struct bpf_type
>>    #vlen number of bpf_param's
>>
>> For a defined subprogram with valid function body,
>>    . function name and all parameter names except the vararg
>>      must be valid C identifier.
> Given that there's an intention to support other frontends besides
>   C, what's the reason for this restriction?

This (C) is the typical usage today. If later on other frontend
generates bpf programs with more relaxed symbol name requirement,
we can certainly relax the rule.

> 
>> For the pointee of a function pointer,
>>    . function name and all parameter names will
>> have name_off = 0 to indicate a non-existing name.
> Why can't function pointer parameters have names?

Currently, both bcc and llvm does not retain function pointer
arguments in dwarf. For LLVM, the IR generation for function pointer
type discards the argument name. So I did the checking because
llvm does not generate it.

We can relax the restrictions later if the compiler starts
to keep argument names in the IR.

> E.g. imagine something like struct net_device_ops.  All those
>   function pointers have named parameters and that's relevant info
>   when debugging.
> 
> -Ed
>