[PATCH bpf-next 0/7] BTF uapi cleanup

[PATCH bpf-next 0/7] BTF uapi cleanup

[Martin KaFai Lau]

This patch set makes some changes to cleanup the unused
bits in BTF uapi.  It also makes the btf_header extensible.

Please see individual patches for details.

Martin KaFai Lau (7):
  bpf: Expose check_uarg_tail_zero()
  bpf: btf: Change how section is supported in btf_header
  bpf: btf: Check array->index_type
  bpf: btf: Remove unused bits from uapi/linux/btf.h
  bpf: btf: Rename btf_key_id and btf_value_id in bpf_map_info
  bpf: btf: Sync bpf.h and btf.h to tools/include/uapi/linux/
  bpf: btf: Add tests for the btf uapi changes

 include/linux/bpf.h                    |   6 +-
 include/uapi/linux/bpf.h               |   8 +-
 include/uapi/linux/btf.h               |  28 +-
 kernel/bpf/arraymap.c                  |   2 +-
 kernel/bpf/btf.c                       | 318 ++++++++++++++------
 kernel/bpf/syscall.c                   |  32 +-
 tools/include/uapi/linux/bpf.h         |   8 +-
 tools/include/uapi/linux/btf.h         |  28 +-
 tools/lib/bpf/bpf.c                    |   4 +-
 tools/lib/bpf/bpf.h                    |   4 +-
 tools/lib/bpf/btf.c                    |   5 +-
 tools/lib/bpf/libbpf.c                 |  34 +--
 tools/lib/bpf/libbpf.h                 |   4 +-
 tools/testing/selftests/bpf/test_btf.c | 528 ++++++++++++++++++++++++++-------
 14 files changed, 724 insertions(+), 285 deletions(-)

-- 
2.9.5

[PATCH bpf-next 1/7] bpf: Expose check_uarg_tail_zero()

[Martin KaFai Lau]

This patch exposes check_uarg_tail_zero() which will
be reused by a later BTF patch.  Its name is changed to
bpf_check_uarg_tail_zero().

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 include/linux/bpf.h  |  2 ++
 kernel/bpf/syscall.c | 14 +++++++-------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index ed0122b45b63..f6fe3c719ca8 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -463,6 +463,8 @@ int bpf_fd_htab_map_update_elem(struct bpf_map *map, struct file *map_file,
 int bpf_fd_htab_map_lookup_elem(struct bpf_map *map, void *key, u32 *value);
 
 int bpf_get_file_flag(int flags);
+int bpf_check_uarg_tail_zero(void __user *uaddr, size_t expected_size,
+			     size_t actual_size);
 
 /* memcpy that is used with 8-byte aligned pointers, power-of-8 size and
  * forced to use 'long' read/writes to try to atomically copy long counters.
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index bfcde949c7f8..2b29ef84ded3 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -65,9 +65,9 @@ static const struct bpf_map_ops * const bpf_map_types[] = {
  * copy_from_user() call. However, this is not a concern since this function is
  * meant to be a future-proofing of bits.
  */
-static int check_uarg_tail_zero(void __user *uaddr,
-				size_t expected_size,
-				size_t actual_size)
+int bpf_check_uarg_tail_zero(void __user *uaddr,
+			     size_t expected_size,
+			     size_t actual_size)
 {
 	unsigned char __user *addr;
 	unsigned char __user *end;
@@ -1899,7 +1899,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
 	u32 ulen;
 	int err;
 
-	err = check_uarg_tail_zero(uinfo, sizeof(info), info_len);
+	err = bpf_check_uarg_tail_zero(uinfo, sizeof(info), info_len);
 	if (err)
 		return err;
 	info_len = min_t(u32, sizeof(info), info_len);
@@ -1998,7 +1998,7 @@ static int bpf_map_get_info_by_fd(struct bpf_map *map,
 	u32 info_len = attr->info.info_len;
 	int err;
 
-	err = check_uarg_tail_zero(uinfo, sizeof(info), info_len);
+	err = bpf_check_uarg_tail_zero(uinfo, sizeof(info), info_len);
 	if (err)
 		return err;
 	info_len = min_t(u32, sizeof(info), info_len);
@@ -2038,7 +2038,7 @@ static int bpf_btf_get_info_by_fd(struct btf *btf,
 	u32 info_len = attr->info.info_len;
 	int err;
 
-	err = check_uarg_tail_zero(uinfo, sizeof(*uinfo), info_len);
+	err = bpf_check_uarg_tail_zero(uinfo, sizeof(*uinfo), info_len);
 	if (err)
 		return err;
 
@@ -2110,7 +2110,7 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
 	if (sysctl_unprivileged_bpf_disabled && !capable(CAP_SYS_ADMIN))
 		return -EPERM;
 
-	err = check_uarg_tail_zero(uattr, sizeof(attr), size);
+	err = bpf_check_uarg_tail_zero(uattr, sizeof(attr), size);
 	if (err)
 		return err;
 	size = min_t(u32, size, sizeof(attr));
-- 
2.9.5

[PATCH bpf-next 2/7] bpf: btf: Change how section is supported in btf_header

[Martin KaFai Lau]

There are currently unused section descriptions in the btf_header.  Those
sections are here to support future BTF use cases.  For example, the
func section (func_off) is to support function signature (e.g. the BPF
prog function signature).

Instead of spelling out all potential sections up-front in the btf_header.
This patch makes changes to btf_header such that extending it (e.g. adding
a section) is possible later.  The unused ones can be removed for now and
they can be added back later.

This patch:
1. adds a hdr_len to the btf_header.  It will allow adding
sections (and other info like parent_label and parent_name)
later.  The check is similar to the existing bpf_attr.
If a user passes in a longer hdr_len, the kernel
ensures the extra tailing bytes are 0.

2. allows the section order in the BTF object to be
different from its sec_off order in btf_header.

3. each sec_off is followed by a sec_len.  It must not have gap or
overlapping among sections.

The string section is ensured to be at the end due to the 4 bytes
alignment requirement of the type section.

The above changes will allow enough flexibility to
add new sections (and other info) to the btf_header later.

This patch also removes an unnecessary !err check
at the end of btf_parse().

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 include/uapi/linux/btf.h |   8 +-
 kernel/bpf/btf.c         | 207 +++++++++++++++++++++++++++++++++++------------
 2 files changed, 158 insertions(+), 57 deletions(-)

diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
index bcb56ee47014..4fa479741a02 100644
--- a/include/uapi/linux/btf.h
+++ b/include/uapi/linux/btf.h
@@ -12,15 +12,11 @@ struct btf_header {
 	__u16	magic;
 	__u8	version;
 	__u8	flags;
-
-	__u32	parent_label;
-	__u32	parent_name;
+	__u32	hdr_len;
 
 	/* All offsets are in bytes relative to the end of this header */
-	__u32	label_off;	/* offset of label section	*/
-	__u32	object_off;	/* offset of data object section*/
-	__u32	func_off;	/* offset of function section	*/
 	__u32	type_off;	/* offset of type section	*/
+	__u32	type_len;	/* length of type section	*/
 	__u32	str_off;	/* offset of string section	*/
 	__u32	str_len;	/* length of string section	*/
 };
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index ded10ab47b8a..536e5981ad8c 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -12,6 +12,7 @@
 #include <linux/uaccess.h>
 #include <linux/kernel.h>
 #include <linux/idr.h>
+#include <linux/sort.h>
 #include <linux/bpf_verifier.h>
 #include <linux/btf.h>
 
@@ -184,15 +185,13 @@ static DEFINE_IDR(btf_idr);
 static DEFINE_SPINLOCK(btf_idr_lock);
 
 struct btf {
-	union {
-		struct btf_header *hdr;
-		void *data;
-	};
+	void *data;
 	struct btf_type **types;
 	u32 *resolved_ids;
 	u32 *resolved_sizes;
 	const char *strings;
 	void *nohdr_data;
+	struct btf_header hdr;
 	u32 nr_types;
 	u32 types_size;
 	u32 data_size;
@@ -227,6 +226,12 @@ enum resolve_mode {
 };
 
 #define MAX_RESOLVE_DEPTH 32
+#define NR_SECS 2
+
+struct btf_sec_info {
+	u32 off;
+	u32 len;
+};
 
 struct btf_verifier_env {
 	struct btf *btf;
@@ -418,14 +423,14 @@ static const struct btf_kind_operations *btf_type_ops(const struct btf_type *t)
 static bool btf_name_offset_valid(const struct btf *btf, u32 offset)
 {
 	return !BTF_STR_TBL_ELF_ID(offset) &&
-		BTF_STR_OFFSET(offset) < btf->hdr->str_len;
+		BTF_STR_OFFSET(offset) < btf->hdr.str_len;
 }
 
 static const char *btf_name_by_offset(const struct btf *btf, u32 offset)
 {
 	if (!BTF_STR_OFFSET(offset))
 		return "(anon)";
-	else if (BTF_STR_OFFSET(offset) < btf->hdr->str_len)
+	else if (BTF_STR_OFFSET(offset) < btf->hdr.str_len)
 		return &btf->strings[BTF_STR_OFFSET(offset)];
 	else
 		return "(invalid-name-offset)";
@@ -536,7 +541,8 @@ static void btf_verifier_log_member(struct btf_verifier_env *env,
 	__btf_verifier_log(log, "\n");
 }
 
-static void btf_verifier_log_hdr(struct btf_verifier_env *env)
+static void btf_verifier_log_hdr(struct btf_verifier_env *env,
+				 u32 btf_data_size)
 {
 	struct bpf_verifier_log *log = &env->log;
 	const struct btf *btf = env->btf;
@@ -545,19 +551,16 @@ static void btf_verifier_log_hdr(struct btf_verifier_env *env)
 	if (!bpf_verifier_log_needed(log))
 		return;
 
-	hdr = btf->hdr;
+	hdr = &btf->hdr;
 	__btf_verifier_log(log, "magic: 0x%x\n", hdr->magic);
 	__btf_verifier_log(log, "version: %u\n", hdr->version);
 	__btf_verifier_log(log, "flags: 0x%x\n", hdr->flags);
-	__btf_verifier_log(log, "parent_label: %u\n", hdr->parent_label);
-	__btf_verifier_log(log, "parent_name: %u\n", hdr->parent_name);
-	__btf_verifier_log(log, "label_off: %u\n", hdr->label_off);
-	__btf_verifier_log(log, "object_off: %u\n", hdr->object_off);
-	__btf_verifier_log(log, "func_off: %u\n", hdr->func_off);
+	__btf_verifier_log(log, "hdr_len: %u\n", hdr->hdr_len);
 	__btf_verifier_log(log, "type_off: %u\n", hdr->type_off);
+	__btf_verifier_log(log, "type_len: %u\n", hdr->type_len);
 	__btf_verifier_log(log, "str_off: %u\n", hdr->str_off);
 	__btf_verifier_log(log, "str_len: %u\n", hdr->str_len);
-	__btf_verifier_log(log, "btf_total_size: %u\n", btf->data_size);
+	__btf_verifier_log(log, "btf_total_size: %u\n", btf_data_size);
 }
 
 static int btf_add_type(struct btf_verifier_env *env, struct btf_type *t)
@@ -1754,9 +1757,9 @@ static int btf_check_all_metas(struct btf_verifier_env *env)
 	struct btf_header *hdr;
 	void *cur, *end;
 
-	hdr = btf->hdr;
+	hdr = &btf->hdr;
 	cur = btf->nohdr_data + hdr->type_off;
-	end = btf->nohdr_data + hdr->str_off;
+	end = btf->nohdr_data + hdr->type_len;
 
 	env->log_type_id = 1;
 	while (cur < end) {
@@ -1866,8 +1869,20 @@ static int btf_check_all_types(struct btf_verifier_env *env)
 
 static int btf_parse_type_sec(struct btf_verifier_env *env)
 {
+	const struct btf_header *hdr = &env->btf->hdr;
 	int err;
 
+	/* Type section must align to 4 bytes */
+	if (hdr->type_off & (sizeof(u32) - 1)) {
+		btf_verifier_log(env, "Unaligned type_off");
+		return -EINVAL;
+	}
+
+	if (!hdr->type_len) {
+		btf_verifier_log(env, "No type found");
+		return -EINVAL;
+	}
+
 	err = btf_check_all_metas(env);
 	if (err)
 		return err;
@@ -1881,10 +1896,15 @@ static int btf_parse_str_sec(struct btf_verifier_env *env)
 	struct btf *btf = env->btf;
 	const char *start, *end;
 
-	hdr = btf->hdr;
+	hdr = &btf->hdr;
 	start = btf->nohdr_data + hdr->str_off;
 	end = start + hdr->str_len;
 
+	if (end != btf->data + btf->data_size) {
+		btf_verifier_log(env, "String section is not at the end");
+		return -EINVAL;
+	}
+
 	if (!hdr->str_len || hdr->str_len - 1 > BTF_MAX_NAME_OFFSET ||
 	    start[0] || end[-1]) {
 		btf_verifier_log(env, "Invalid string section");
@@ -1896,20 +1916,119 @@ static int btf_parse_str_sec(struct btf_verifier_env *env)
 	return 0;
 }
 
-static int btf_parse_hdr(struct btf_verifier_env *env)
+static const size_t btf_sec_info_offset[] = {
+	offsetof(struct btf_header, type_off),
+	offsetof(struct btf_header, str_off),
+};
+
+static int btf_sec_info_cmp(const void *a, const void *b)
 {
+	const struct btf_sec_info *x = a;
+	const struct btf_sec_info *y = b;
+
+	return (int)(x->off - y->off) ? : (int)(x->len - y->len);
+}
+
+static int btf_check_sec_info(struct btf_verifier_env *env,
+			      u32 btf_data_size)
+{
+	struct btf_sec_info secs[NR_SECS];
+	u32 total, expected_total, i;
 	const struct btf_header *hdr;
-	struct btf *btf = env->btf;
-	u32 meta_left;
+	const struct btf *btf;
+
+	BUILD_BUG_ON(ARRAY_SIZE(btf_sec_info_offset) != NR_SECS);
+
+	btf = env->btf;
+	hdr = &btf->hdr;
+
+	/* Populate the secs from hdr */
+	for (i = 0; i < NR_SECS; i++)
+		secs[i] = *(struct btf_sec_info *)((void *)hdr +
+						   btf_sec_info_offset[i]);
+
+	sort(secs, NR_SECS, sizeof(struct btf_sec_info),
+	     btf_sec_info_cmp, NULL);
+
+	/* Check for gaps and overlap among sections */
+	total = 0;
+	expected_total = btf_data_size - hdr->hdr_len;
+	for (i = 0; i < NR_SECS; i++) {
+		if (expected_total < secs[i].off) {
+			btf_verifier_log(env, "Invalid section offset");
+			return -EINVAL;
+		}
+		if (total < secs[i].off) {
+			/* gap */
+			btf_verifier_log(env, "Unsupported section found");
+			return -EINVAL;
+		}
+		if (total > secs[i].off) {
+			btf_verifier_log(env, "Section overlap found");
+			return -EINVAL;
+		}
+		if (expected_total - total < secs[i].len) {
+			btf_verifier_log(env,
+					 "Total section length too long");
+			return -EINVAL;
+		}
+		total += secs[i].len;
+	}
+
+	/* There is data other than hdr and known sections */
+	if (expected_total != total) {
+		btf_verifier_log(env, "Unsupported section found");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int btf_parse_hdr(struct btf_verifier_env *env, void __user *btf_data,
+			 u32 btf_data_size)
+{
+	const struct btf_header *hdr;
+	u32 hdr_len, hdr_copy;
+	struct btf_min_header {
+		u16	magic;
+		u8	version;
+		u8	flags;
+		u32	hdr_len;
+	} __user *min_hdr;
+	struct btf *btf;
+	int err;
+
+	btf = env->btf;
+	min_hdr = btf_data;
+
+	if (btf_data_size < sizeof(*min_hdr)) {
+		btf_verifier_log(env, "hdr_len not found");
+		return -EINVAL;
+	}
 
-	if (btf->data_size < sizeof(*hdr)) {
+	if (get_user(hdr_len, &min_hdr->hdr_len))
+		return -EFAULT;
+
+	if (btf_data_size < hdr_len) {
 		btf_verifier_log(env, "btf_header not found");
 		return -EINVAL;
 	}
 
-	btf_verifier_log_hdr(env);
+	err = bpf_check_uarg_tail_zero(btf_data, sizeof(btf->hdr), hdr_len);
+	if (err) {
+		if (err == -E2BIG)
+			btf_verifier_log(env, "Unsupported btf_header");
+		return err;
+	}
+
+	hdr_copy = min_t(u32, hdr_len, sizeof(btf->hdr));
+	if (copy_from_user(&btf->hdr, btf_data, hdr_copy))
+		return -EFAULT;
+
+	hdr = &btf->hdr;
+
+	btf_verifier_log_hdr(env, btf_data_size);
 
-	hdr = btf->hdr;
 	if (hdr->magic != BTF_MAGIC) {
 		btf_verifier_log(env, "Invalid magic");
 		return -EINVAL;
@@ -1925,26 +2044,14 @@ static int btf_parse_hdr(struct btf_verifier_env *env)
 		return -ENOTSUPP;
 	}
 
-	meta_left = btf->data_size - sizeof(*hdr);
-	if (!meta_left) {
+	if (btf_data_size == hdr->hdr_len) {
 		btf_verifier_log(env, "No data");
 		return -EINVAL;
 	}
 
-	if (meta_left < hdr->type_off || hdr->str_off <= hdr->type_off ||
-	    /* Type section must align to 4 bytes */
-	    hdr->type_off & (sizeof(u32) - 1)) {
-		btf_verifier_log(env, "Invalid type_off");
-		return -EINVAL;
-	}
-
-	if (meta_left < hdr->str_off ||
-	    meta_left - hdr->str_off < hdr->str_len) {
-		btf_verifier_log(env, "Invalid str_off or str_len");
-		return -EINVAL;
-	}
-
-	btf->nohdr_data = btf->hdr + 1;
+	err = btf_check_sec_info(env, btf_data_size);
+	if (err)
+		return err;
 
 	return 0;
 }
@@ -1987,6 +2094,11 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
 		err = -ENOMEM;
 		goto errout;
 	}
+	env->btf = btf;
+
+	err = btf_parse_hdr(env, btf_data, btf_data_size);
+	if (err)
+		goto errout;
 
 	data = kvmalloc(btf_data_size, GFP_KERNEL | __GFP_NOWARN);
 	if (!data) {
@@ -1996,18 +2108,13 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
 
 	btf->data = data;
 	btf->data_size = btf_data_size;
+	btf->nohdr_data = btf->data + btf->hdr.hdr_len;
 
 	if (copy_from_user(data, btf_data, btf_data_size)) {
 		err = -EFAULT;
 		goto errout;
 	}
 
-	env->btf = btf;
-
-	err = btf_parse_hdr(env);
-	if (err)
-		goto errout;
-
 	err = btf_parse_str_sec(env);
 	if (err)
 		goto errout;
@@ -2016,16 +2123,14 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
 	if (err)
 		goto errout;
 
-	if (!err && log->level && bpf_verifier_log_full(log)) {
+	if (log->level && bpf_verifier_log_full(log)) {
 		err = -ENOSPC;
 		goto errout;
 	}
 
-	if (!err) {
-		btf_verifier_env_free(env);
-		refcount_set(&btf->refcnt, 1);
-		return btf;
-	}
+	btf_verifier_env_free(env);
+	refcount_set(&btf->refcnt, 1);
+	return btf;
 
 errout:
 	btf_verifier_env_free(env);
-- 
2.9.5

[PATCH bpf-next 3/7] bpf: btf: Check array->index_type

[Martin KaFai Lau]

Instead of ingoring the array->index_type field.  Enforce that
it must be an unsigned BTF_KIND_INT.

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

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 536e5981ad8c..b4e48dae2240 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -444,6 +444,28 @@ static const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id)
 	return btf->types[type_id];
 }
 
+/*
+ * Regular int is not a bit field and it must be either
+ * u8/u16/u32/u64.
+ */
+static bool btf_type_int_is_regular(const struct btf_type *t)
+{
+	u16 nr_bits, nr_bytes;
+	u32 int_data;
+
+	int_data = btf_type_int(t);
+	nr_bits = BTF_INT_BITS(int_data);
+	nr_bytes = BITS_ROUNDUP_BYTES(nr_bits);
+	if (BITS_PER_BYTE_MASKED(nr_bits) ||
+	    BTF_INT_OFFSET(int_data) ||
+	    (nr_bytes != sizeof(u8) && nr_bytes != sizeof(u16) &&
+	     nr_bytes != sizeof(u32) && nr_bytes != sizeof(u64))) {
+		return false;
+	}
+
+	return true;
+}
+
 __printf(2, 3) static void __btf_verifier_log(struct bpf_verifier_log *log,
 					      const char *fmt, ...)
 {
@@ -1309,14 +1331,16 @@ static s32 btf_array_check_meta(struct btf_verifier_env *env,
 		return -EINVAL;
 	}
 
-	/* We are a little forgiving on array->index_type since
-	 * the kernel is not using it.
-	 */
-	/* Array elem cannot be in type void,
-	 * so !array->type is not allowed.
+	/* Array elem type and index type cannot be in type void,
+	 * so !array->type and !array->index_type are not allowed.
 	 */
 	if (!array->type || BTF_TYPE_PARENT(array->type)) {
-		btf_verifier_log_type(env, t, "Invalid type_id");
+		btf_verifier_log_type(env, t, "Invalid elem");
+		return -EINVAL;
+	}
+
+	if (!array->index_type || BTF_TYPE_PARENT(array->index_type)) {
+		btf_verifier_log_type(env, t, "Invalid index");
 		return -EINVAL;
 	}
 
@@ -1329,11 +1353,35 @@ static int btf_array_resolve(struct btf_verifier_env *env,
 			     const struct resolve_vertex *v)
 {
 	const struct btf_array *array = btf_type_array(v->t);
-	const struct btf_type *elem_type;
-	u32 elem_type_id = array->type;
+	const struct btf_type *elem_type, *index_type;
+	u32 elem_type_id, index_type_id;
 	struct btf *btf = env->btf;
 	u32 elem_size;
 
+	/* 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)) {
+		btf_verifier_log_type(env, v->t, "Invalid index");
+		return -EINVAL;
+	}
+
+	if (!env_type_is_resolve_sink(env, index_type) &&
+	    !env_type_is_resolved(env, index_type_id))
+		return env_stack_push(env, index_type, index_type_id);
+
+	index_type = btf_type_id_size(btf, &index_type_id, NULL);
+	if (!index_type || !btf_type_is_int(index_type) ||
+	    /* bit field int is not allowed */
+	    !btf_type_int_is_regular(index_type) ||
+	    /* unsigned only */
+	    BTF_INT_ENCODING(btf_type_int(index_type))) {
+		btf_verifier_log_type(env, v->t, "Invalid index");
+		return -EINVAL;
+	}
+
+	/* 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)) {
 		btf_verifier_log_type(env, v->t,
@@ -1351,22 +1399,9 @@ static int btf_array_resolve(struct btf_verifier_env *env,
 		return -EINVAL;
 	}
 
-	if (btf_type_is_int(elem_type)) {
-		int int_type_data = btf_type_int(elem_type);
-		u16 nr_bits = BTF_INT_BITS(int_type_data);
-		u16 nr_bytes = BITS_ROUNDUP_BYTES(nr_bits);
-
-		/* Put more restriction on array of int.  The int cannot
-		 * be a bit field and it must be either u8/u16/u32/u64.
-		 */
-		if (BITS_PER_BYTE_MASKED(nr_bits) ||
-		    BTF_INT_OFFSET(int_type_data) ||
-		    (nr_bytes != sizeof(u8) && nr_bytes != sizeof(u16) &&
-		     nr_bytes != sizeof(u32) && nr_bytes != sizeof(u64))) {
-			btf_verifier_log_type(env, v->t,
-					      "Invalid array of int");
-			return -EINVAL;
-		}
+	if (btf_type_is_int(elem_type) && !btf_type_int_is_regular(elem_type)) {
+		btf_verifier_log_type(env, v->t, "Invalid array of int");
+		return -EINVAL;
 	}
 
 	if (array->nelems && elem_size > U32_MAX / array->nelems) {
-- 
2.9.5

[PATCH bpf-next 4/7] bpf: btf: Remove unused bits from uapi/linux/btf.h

[Martin KaFai Lau]

This patch does the followings:
1. Limit BTF_MAX_TYPES and BTF_MAX_NAME_OFFSET to 64k.  We can
   raise it later.

2. Remove the BTF_TYPE_PARENT and BTF_STR_TBL_ELF_ID.  They are
   currently encoded at the highest bit of a u32.
   It is because the current use case does not require supporting
   parent type (i.e type_id referring to a type in another BTF file).
   It also does not support referring to a string in ELF.

   The BTF_TYPE_PARENT and BTF_STR_TBL_ELF_ID checks are replaced
   by BTF_TYPE_ID_CHECK and BTF_STR_OFFSET_CHECK which are
   defined in btf.c instead of uapi/linux/btf.h.

3. Limit the BTF_INFO_KIND from 5 bits to 4 bits which is enough.
   There is unused bits headroom if we ever needed it later.

4. The root bit in BTF_INFO is also removed because it is not
   used in the current use case.

The above can be added back later because the verifier
ensures the unused bits are zeros.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 include/uapi/linux/btf.h | 20 +++++---------------
 kernel/bpf/btf.c         | 34 +++++++++++++++++++++-------------
 2 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
index 4fa479741a02..b89b56f2b099 100644
--- a/include/uapi/linux/btf.h
+++ b/include/uapi/linux/btf.h
@@ -22,28 +22,19 @@ struct btf_header {
 };
 
 /* Max # of type identifier */
-#define BTF_MAX_TYPE	0x7fffffff
+#define BTF_MAX_TYPE	0x0000ffff
 /* Max offset into the string section */
-#define BTF_MAX_NAME_OFFSET	0x7fffffff
+#define BTF_MAX_NAME_OFFSET	0x0000ffff
 /* Max # of struct/union/enum members or func args */
 #define BTF_MAX_VLEN	0xffff
 
-/* The type id is referring to a parent BTF */
-#define BTF_TYPE_PARENT(id)	(((id) >> 31) & 0x1)
-#define BTF_TYPE_ID(id)		((id) & BTF_MAX_TYPE)
-
-/* String is in the ELF string section */
-#define BTF_STR_TBL_ELF_ID(ref)	(((ref) >> 31) & 0x1)
-#define BTF_STR_OFFSET(ref)	((ref) & BTF_MAX_NAME_OFFSET)
-
 struct btf_type {
 	__u32 name_off;
 	/* "info" bits arrangement
 	 * bits  0-15: vlen (e.g. # of struct's members)
 	 * bits 16-23: unused
-	 * bits 24-28: kind (e.g. int, ptr, array...etc)
-	 * bits 29-30: unused
-	 * bits    31: root
+	 * bits 24-27: kind (e.g. int, ptr, array...etc)
+	 * bits 28-31: unused
 	 */
 	__u32 info;
 	/* "size" is used by INT, ENUM, STRUCT and UNION.
@@ -58,8 +49,7 @@ struct btf_type {
 	};
 };
 
-#define BTF_INFO_KIND(info)	(((info) >> 24) & 0x1f)
-#define BTF_INFO_ISROOT(info)	(!!(((info) >> 24) & 0x80))
+#define BTF_INFO_KIND(info)	(((info) >> 24) & 0x0f)
 #define BTF_INFO_VLEN(info)	((info) & 0xffff)
 
 #define BTF_KIND_UNKN		0	/* Unknown	*/
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index b4e48dae2240..5d1967d4fb62 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -163,13 +163,15 @@
 #define BITS_ROUNDUP_BYTES(bits) \
 	(BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits))
 
+#define BTF_INFO_MASK 0x0f00ffff
+#define BTF_TYPE_ID_CHECK(type_id) ((type_id) <= BTF_MAX_TYPE)
+#define BTF_STR_OFFSET_CHECK(name_off) ((name_off) <= BTF_MAX_NAME_OFFSET)
+
 /* 16MB for 64k structs and each has 16 members and
  * a few MB spaces for the string section.
  * The hard limit is S32_MAX.
  */
 #define BTF_MAX_SIZE (16 * 1024 * 1024)
-/* 64k. We can raise it later. The hard limit is S32_MAX. */
-#define BTF_MAX_NR_TYPES 65535
 
 #define for_each_member(i, struct_type, member)			\
 	for (i = 0, member = btf_type_member(struct_type);	\
@@ -422,16 +424,16 @@ static const struct btf_kind_operations *btf_type_ops(const struct btf_type *t)
 
 static bool btf_name_offset_valid(const struct btf *btf, u32 offset)
 {
-	return !BTF_STR_TBL_ELF_ID(offset) &&
-		BTF_STR_OFFSET(offset) < btf->hdr.str_len;
+	return BTF_STR_OFFSET_CHECK(offset) &&
+		offset < btf->hdr.str_len;
 }
 
 static const char *btf_name_by_offset(const struct btf *btf, u32 offset)
 {
-	if (!BTF_STR_OFFSET(offset))
+	if (!offset)
 		return "(anon)";
-	else if (BTF_STR_OFFSET(offset) < btf->hdr.str_len)
-		return &btf->strings[BTF_STR_OFFSET(offset)];
+	else if (offset < btf->hdr.str_len)
+		return &btf->strings[offset];
 	else
 		return "(invalid-name-offset)";
 }
@@ -599,13 +601,13 @@ static int btf_add_type(struct btf_verifier_env *env, struct btf_type *t)
 		struct btf_type **new_types;
 		u32 expand_by, new_size;
 
-		if (btf->types_size == BTF_MAX_NR_TYPES) {
+		if (btf->types_size == BTF_MAX_TYPE) {
 			btf_verifier_log(env, "Exceeded max num of types");
 			return -E2BIG;
 		}
 
 		expand_by = max_t(u32, btf->types_size >> 2, 16);
-		new_size = min_t(u32, BTF_MAX_NR_TYPES,
+		new_size = min_t(u32, BTF_MAX_TYPE,
 				 btf->types_size + expand_by);
 
 		new_types = kvzalloc(new_size * sizeof(*new_types),
@@ -1127,7 +1129,7 @@ static int btf_ref_type_check_meta(struct btf_verifier_env *env,
 		return -EINVAL;
 	}
 
-	if (BTF_TYPE_PARENT(t->type)) {
+	if (!BTF_TYPE_ID_CHECK(t->type)) {
 		btf_verifier_log_type(env, t, "Invalid type_id");
 		return -EINVAL;
 	}
@@ -1334,12 +1336,12 @@ static s32 btf_array_check_meta(struct btf_verifier_env *env,
 	/* Array elem type and index type cannot be in type void,
 	 * so !array->type and !array->index_type are not allowed.
 	 */
-	if (!array->type || BTF_TYPE_PARENT(array->type)) {
+	if (!array->type || !BTF_TYPE_ID_CHECK(array->type)) {
 		btf_verifier_log_type(env, t, "Invalid elem");
 		return -EINVAL;
 	}
 
-	if (!array->index_type || BTF_TYPE_PARENT(array->index_type)) {
+	if (!array->index_type || !BTF_TYPE_ID_CHECK(array->index_type)) {
 		btf_verifier_log_type(env, t, "Invalid index");
 		return -EINVAL;
 	}
@@ -1511,7 +1513,7 @@ static s32 btf_struct_check_meta(struct btf_verifier_env *env,
 		}
 
 		/* A member cannot be in type void */
-		if (!member->type || BTF_TYPE_PARENT(member->type)) {
+		if (!member->type || !BTF_TYPE_ID_CHECK(member->type)) {
 			btf_verifier_log_member(env, t, member,
 						"Invalid type_id");
 			return -EINVAL;
@@ -1764,6 +1766,12 @@ static s32 btf_check_meta(struct btf_verifier_env *env,
 	}
 	meta_left -= sizeof(*t);
 
+	if (t->info & ~BTF_INFO_MASK) {
+		btf_verifier_log(env, "[%u] Invalid btf_info:%x",
+				 env->log_type_id, t->info);
+		return -EINVAL;
+	}
+
 	if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX ||
 	    BTF_INFO_KIND(t->info) == BTF_KIND_UNKN) {
 		btf_verifier_log(env, "[%u] Invalid kind:%u",
-- 
2.9.5

[PATCH bpf-next 5/7] bpf: btf: Rename btf_key_id and btf_value_id in bpf_map_info

[Martin KaFai Lau]

In "struct bpf_map_info", the name "btf_id", "btf_key_id" and "btf_value_id"
could cause confusion because the "id" of "btf_id" means the BPF obj id
given to the BTF object while
"btf_key_id" and "btf_value_id" means the BTF type id within
that BTF object.

To make it clear, btf_key_id and btf_value_id are
renamed to btf_key_type_id and btf_value_type_id.

Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 include/linux/bpf.h      |  4 ++--
 include/uapi/linux/bpf.h |  8 ++++----
 kernel/bpf/arraymap.c    |  2 +-
 kernel/bpf/syscall.c     | 18 +++++++++---------
 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index f6fe3c719ca8..1795eeee846c 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -69,8 +69,8 @@ struct bpf_map {
 	u32 pages;
 	u32 id;
 	int numa_node;
-	u32 btf_key_id;
-	u32 btf_value_id;
+	u32 btf_key_type_id;
+	u32 btf_value_type_id;
 	struct btf *btf;
 	bool unpriv_array;
 	/* 55 bytes hole */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index d94d333a8225..123ebe4b3662 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -284,8 +284,8 @@ union bpf_attr {
 		char	map_name[BPF_OBJ_NAME_LEN];
 		__u32	map_ifindex;	/* ifindex of netdev to create on */
 		__u32	btf_fd;		/* fd pointing to a BTF type data */
-		__u32	btf_key_id;	/* BTF type_id of the key */
-		__u32	btf_value_id;	/* BTF type_id of the value */
+		__u32	btf_key_type_id;	/* BTF type_id of the key */
+		__u32	btf_value_type_id;	/* BTF type_id of the value */
 	};
 
 	struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */
@@ -2211,8 +2211,8 @@ struct bpf_map_info {
 	__u64 netns_dev;
 	__u64 netns_ino;
 	__u32 btf_id;
-	__u32 btf_key_id;
-	__u32 btf_value_id;
+	__u32 btf_key_type_id;
+	__u32 btf_value_type_id;
 } __attribute__((aligned(8)));
 
 struct bpf_btf_info {
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 0fd8d8f1a398..544e58f5f642 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -352,7 +352,7 @@ static void array_map_seq_show_elem(struct bpf_map *map, void *key,
 	}
 
 	seq_printf(m, "%u: ", *(u32 *)key);
-	btf_type_seq_show(map->btf, map->btf_value_id, value, m);
+	btf_type_seq_show(map->btf, map->btf_value_type_id, value, m);
 	seq_puts(m, "\n");
 
 	rcu_read_unlock();
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 2b29ef84ded3..0b4c94551001 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -422,7 +422,7 @@ static int bpf_obj_name_cpy(char *dst, const char *src)
 	return 0;
 }
 
-#define BPF_MAP_CREATE_LAST_FIELD btf_value_id
+#define BPF_MAP_CREATE_LAST_FIELD btf_value_type_id
 /* called via syscall */
 static int map_create(union bpf_attr *attr)
 {
@@ -457,10 +457,10 @@ static int map_create(union bpf_attr *attr)
 	atomic_set(&map->usercnt, 1);
 
 	if (bpf_map_support_seq_show(map) &&
-	    (attr->btf_key_id || attr->btf_value_id)) {
+	    (attr->btf_key_type_id || attr->btf_value_type_id)) {
 		struct btf *btf;
 
-		if (!attr->btf_key_id || !attr->btf_value_id) {
+		if (!attr->btf_key_type_id || !attr->btf_value_type_id) {
 			err = -EINVAL;
 			goto free_map_nouncharge;
 		}
@@ -471,16 +471,16 @@ static int map_create(union bpf_attr *attr)
 			goto free_map_nouncharge;
 		}
 
-		err = map->ops->map_check_btf(map, btf, attr->btf_key_id,
-					      attr->btf_value_id);
+		err = map->ops->map_check_btf(map, btf, attr->btf_key_type_id,
+					      attr->btf_value_type_id);
 		if (err) {
 			btf_put(btf);
 			goto free_map_nouncharge;
 		}
 
 		map->btf = btf;
-		map->btf_key_id = attr->btf_key_id;
-		map->btf_value_id = attr->btf_value_id;
+		map->btf_key_type_id = attr->btf_key_type_id;
+		map->btf_value_type_id = attr->btf_value_type_id;
 	}
 
 	err = security_bpf_map_alloc(map);
@@ -2013,8 +2013,8 @@ static int bpf_map_get_info_by_fd(struct bpf_map *map,
 
 	if (map->btf) {
 		info.btf_id = btf_id(map->btf);
-		info.btf_key_id = map->btf_key_id;
-		info.btf_value_id = map->btf_value_id;
+		info.btf_key_type_id = map->btf_key_type_id;
+		info.btf_value_type_id = map->btf_value_type_id;
 	}
 
 	if (bpf_map_is_dev_bound(map)) {
-- 
2.9.5

[PATCH bpf-next 7/7] bpf: btf: Add tests for the btf uapi changes

[Martin KaFai Lau]

This patch does the followings:
1. Modify libbpf and test_btf to reflect the uapi changes in btf
2. Add test for the btf_header changes
3. Add tests for array->index_type
4. Add err_str check to the tests
5. Fix a 4 bytes hole in "struct test #1" by swapping "m" and "n"

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/lib/bpf/bpf.c                    |   4 +-
 tools/lib/bpf/bpf.h                    |   4 +-
 tools/lib/bpf/btf.c                    |   5 +-
 tools/lib/bpf/libbpf.c                 |  34 +--
 tools/lib/bpf/libbpf.h                 |   4 +-
 tools/testing/selftests/bpf/test_btf.c | 528 ++++++++++++++++++++++++++-------
 6 files changed, 448 insertions(+), 131 deletions(-)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 6a8a00097fd8..442b4cdfeb71 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -89,8 +89,8 @@ int bpf_create_map_xattr(const struct bpf_create_map_attr *create_attr)
 	       min(name_len, BPF_OBJ_NAME_LEN - 1));
 	attr.numa_node = create_attr->numa_node;
 	attr.btf_fd = create_attr->btf_fd;
-	attr.btf_key_id = create_attr->btf_key_id;
-	attr.btf_value_id = create_attr->btf_value_id;
+	attr.btf_key_type_id = create_attr->btf_key_type_id;
+	attr.btf_value_type_id = create_attr->btf_value_type_id;
 	attr.map_ifindex = create_attr->map_ifindex;
 
 	return sys_bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 15bff7728cf1..d12344f66d4e 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -36,8 +36,8 @@ struct bpf_create_map_attr {
 	__u32 max_entries;
 	__u32 numa_node;
 	__u32 btf_fd;
-	__u32 btf_key_id;
-	__u32 btf_value_id;
+	__u32 btf_key_type_id;
+	__u32 btf_value_type_id;
 	__u32 map_ifindex;
 };
 
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 2bac710e3194..8c54a4b6f187 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -35,9 +35,8 @@ struct btf {
 
 static const char *btf_name_by_offset(const struct btf *btf, uint32_t offset)
 {
-	if (!BTF_STR_TBL_ELF_ID(offset) &&
-	    BTF_STR_OFFSET(offset) < btf->hdr->str_len)
-		return &btf->strings[BTF_STR_OFFSET(offset)];
+	if (offset < btf->hdr->str_len)
+		return &btf->strings[offset];
 	else
 		return NULL;
 }
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 3dbe217bf23e..8f1707dbfcfa 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -216,8 +216,8 @@ struct bpf_map {
 	size_t offset;
 	int map_ifindex;
 	struct bpf_map_def def;
-	uint32_t btf_key_id;
-	uint32_t btf_value_id;
+	uint32_t btf_key_type_id;
+	uint32_t btf_value_type_id;
 	void *priv;
 	bpf_map_clear_priv_t clear_priv;
 };
@@ -1074,8 +1074,8 @@ static int bpf_map_find_btf_info(struct bpf_map *map, const struct btf *btf)
 		return -EINVAL;
 	}
 
-	map->btf_key_id = key_id;
-	map->btf_value_id = value_id;
+	map->btf_key_type_id = key_id;
+	map->btf_value_type_id = value_id;
 
 	return 0;
 }
@@ -1100,24 +1100,24 @@ bpf_object__create_maps(struct bpf_object *obj)
 		create_attr.value_size = def->value_size;
 		create_attr.max_entries = def->max_entries;
 		create_attr.btf_fd = 0;
-		create_attr.btf_key_id = 0;
-		create_attr.btf_value_id = 0;
+		create_attr.btf_key_type_id = 0;
+		create_attr.btf_value_type_id = 0;
 
 		if (obj->btf && !bpf_map_find_btf_info(map, obj->btf)) {
 			create_attr.btf_fd = btf__fd(obj->btf);
-			create_attr.btf_key_id = map->btf_key_id;
-			create_attr.btf_value_id = map->btf_value_id;
+			create_attr.btf_key_type_id = map->btf_key_type_id;
+			create_attr.btf_value_type_id = map->btf_value_type_id;
 		}
 
 		*pfd = bpf_create_map_xattr(&create_attr);
-		if (*pfd < 0 && create_attr.btf_key_id) {
+		if (*pfd < 0 && create_attr.btf_key_type_id) {
 			pr_warning("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
 				   map->name, strerror(errno), errno);
 			create_attr.btf_fd = 0;
-			create_attr.btf_key_id = 0;
-			create_attr.btf_value_id = 0;
-			map->btf_key_id = 0;
-			map->btf_value_id = 0;
+			create_attr.btf_key_type_id = 0;
+			create_attr.btf_value_type_id = 0;
+			map->btf_key_type_id = 0;
+			map->btf_value_type_id = 0;
 			*pfd = bpf_create_map_xattr(&create_attr);
 		}
 
@@ -2085,14 +2085,14 @@ const char *bpf_map__name(struct bpf_map *map)
 	return map ? map->name : NULL;
 }
 
-uint32_t bpf_map__btf_key_id(const struct bpf_map *map)
+uint32_t bpf_map__btf_key_type_id(const struct bpf_map *map)
 {
-	return map ? map->btf_key_id : 0;
+	return map ? map->btf_key_type_id : 0;
 }
 
-uint32_t bpf_map__btf_value_id(const struct bpf_map *map)
+uint32_t bpf_map__btf_value_type_id(const struct bpf_map *map)
 {
-	return map ? map->btf_value_id : 0;
+	return map ? map->btf_value_type_id : 0;
 }
 
 int bpf_map__set_priv(struct bpf_map *map, void *priv,
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index cd3fd8d782c7..09976531aa74 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -244,8 +244,8 @@ bpf_map__next(struct bpf_map *map, struct bpf_object *obj);
 int bpf_map__fd(struct bpf_map *map);
 const struct bpf_map_def *bpf_map__def(struct bpf_map *map);
 const char *bpf_map__name(struct bpf_map *map);
-uint32_t bpf_map__btf_key_id(const struct bpf_map *map);
-uint32_t bpf_map__btf_value_id(const struct bpf_map *map);
+uint32_t bpf_map__btf_key_type_id(const struct bpf_map *map);
+uint32_t bpf_map__btf_value_type_id(const struct bpf_map *map);
 
 typedef void (*bpf_map_clear_priv_t)(struct bpf_map *, void *);
 int bpf_map__set_priv(struct bpf_map *map, void *priv,
diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
index c8bceae7ec02..4635d5557639 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -113,22 +113,26 @@ static char btf_log_buf[BTF_LOG_BUF_SIZE];
 static struct btf_header hdr_tmpl = {
 	.magic = BTF_MAGIC,
 	.version = BTF_VERSION,
+	.hdr_len = sizeof(struct btf_header),
 };
 
 struct btf_raw_test {
 	const char *descr;
 	const char *str_sec;
 	const char *map_name;
+	const char *err_str;
+	int (*special_test)(unsigned int test_num);
 	__u32 raw_types[MAX_NR_RAW_TYPES];
 	__u32 str_sec_size;
 	enum bpf_map_type map_type;
 	__u32 key_size;
 	__u32 value_size;
-	__u32 key_id;
-	__u32 value_id;
+	__u32 key_type_id;
+	__u32 value_type_id;
 	__u32 max_entries;
 	bool btf_load_err;
 	bool map_create_err;
+	int hdr_len_delta;
 	int type_off_delta;
 	int str_off_delta;
 	int str_len_delta;
@@ -141,8 +145,8 @@ static struct btf_raw_test raw_tests[] = {
  * };
  *
  * struct A {
- *	int m;
- *	unsigned long long n;
+ *	unsigned long long m;
+ *	int n;
  *	char o;
  *	[3 bytes hole]
  *	int p[8];
@@ -160,21 +164,24 @@ static struct btf_raw_test raw_tests[] = {
 		/* char */
 		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 8, 1),	/* [3] */
 		/* int[8] */
-		BTF_TYPE_ARRAY_ENC(1, 1, 8),			/* [4] */
+		BTF_TYPE_ARRAY_ENC(1, 8, 8),			/* [4] */
 		/* struct A { */				/* [5] */
 		BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 6), 180),
-		BTF_MEMBER_ENC(NAME_TBD, 1, 0),	/* int m;		*/
-		BTF_MEMBER_ENC(NAME_TBD, 2, 32),/* unsigned long long n;*/
+		BTF_MEMBER_ENC(NAME_TBD, 2, 0),	/* unsigned long long m;*/
+		BTF_MEMBER_ENC(NAME_TBD, 1, 64),/* int n;		*/
 		BTF_MEMBER_ENC(NAME_TBD, 3, 96),/* char o;		*/
 		BTF_MEMBER_ENC(NAME_TBD, 4, 128),/* int p[8]		*/
 		BTF_MEMBER_ENC(NAME_TBD, 6, 384),/* int q[4][8]		*/
 		BTF_MEMBER_ENC(NAME_TBD, 7, 1408), /* enum E r		*/
 		/* } */
 		/* int[4][8] */
-		BTF_TYPE_ARRAY_ENC(4, 1, 4),			/* [6] */
+		BTF_TYPE_ARRAY_ENC(4, 8, 4),			/* [6]  */
+		/* enum E */					/* [7] */
 		BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_ENUM, 0, 2), sizeof(int)),
 		BTF_ENUM_ENC(NAME_TBD, 0),
 		BTF_ENUM_ENC(NAME_TBD, 1),
+		/* unsigned int */				/* [8] */
+		BTF_TYPE_INT_ENC(0, 0, 0, 32, 4),
 		BTF_END_RAW,
 	},
 	.str_sec = "\0A\0m\0n\0o\0p\0q\0r\0E\0E0\0E1",
@@ -183,8 +190,8 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "struct_test1_map",
 	.key_size = sizeof(int),
 	.value_size = 180,
-	.key_id = 1,
-	.value_id = 5,
+	.key_type_id = 1,
+	.value_type_id = 5,
 	.max_entries = 4,
 },
 
@@ -207,7 +214,7 @@ static struct btf_raw_test raw_tests[] = {
 		/* int */					/* [1] */
 		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
 		/* struct b [4] */				/* [2] */
-		BTF_TYPE_ARRAY_ENC(4, 1, 4),
+		BTF_TYPE_ARRAY_ENC(4, 9, 4),
 
 		/* struct A { */				/* [3] */
 		BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 3), 68),
@@ -229,7 +236,9 @@ static struct btf_raw_test raw_tests[] = {
 		/* const Struct_B */				/* [7] */
 		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 6),
 		/* const Struct_B [4] */			/* [8] */
-		BTF_TYPE_ARRAY_ENC(7, 1, 4),
+		BTF_TYPE_ARRAY_ENC(7, 9, 4),
+		/* unsigned int */				/* [9] */
+		BTF_TYPE_INT_ENC(0, 0, 0, 32, 4),
 		BTF_END_RAW,
 	},
 	.str_sec = "\0A\0m\0n\0o\0B\0m\0n\0Struct_B",
@@ -238,8 +247,8 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "struct_test2_map",
 	.key_size = sizeof(int),
 	.value_size = 68,
-	.key_id = 1,
-	.value_id = 3,
+	.key_type_id = 1,
+	.value_type_id = 3,
 	.max_entries = 4,
 },
 
@@ -258,7 +267,7 @@ static struct btf_raw_test raw_tests[] = {
 		/* struct A { */				/* [2] */
 		BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 2), sizeof(int) * 2 -  1),
 		BTF_MEMBER_ENC(NAME_TBD, 1, 0),	/* int m; */
-		BTF_MEMBER_ENC(NAME_TBD, 2, 32),/* int n; */
+		BTF_MEMBER_ENC(NAME_TBD, 1, 32),/* int n; */
 		/* } */
 		BTF_END_RAW,
 	},
@@ -268,10 +277,11 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "size_check1_map",
 	.key_size = sizeof(int),
 	.value_size = 1,
-	.key_id = 1,
-	.value_id = 2,
+	.key_type_id = 1,
+	.value_type_id = 2,
 	.max_entries = 4,
 	.btf_load_err = true,
+	.err_str = "Member exceeds struct_size",
 },
 
 /* Test member exeeds the size of struct
@@ -287,12 +297,14 @@ static struct btf_raw_test raw_tests[] = {
 		/* int */					/* [1] */
 		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, sizeof(int)),
 		/* int[2] */					/* [2] */
-		BTF_TYPE_ARRAY_ENC(1, 1, 2),
+		BTF_TYPE_ARRAY_ENC(1, 4, 2),
 		/* struct A { */				/* [3] */
 		BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 2), sizeof(int) * 3 - 1),
 		BTF_MEMBER_ENC(NAME_TBD, 1, 0),	/* int m; */
 		BTF_MEMBER_ENC(NAME_TBD, 2, 32),/* int n[2]; */
 		/* } */
+		/* unsigned int */				/* [4] */
+		BTF_TYPE_INT_ENC(0, 0, 0, 32, 4),
 		BTF_END_RAW,
 	},
 	.str_sec = "\0A\0m\0n",
@@ -301,11 +313,11 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "size_check2_map",
 	.key_size = sizeof(int),
 	.value_size = 1,
-	.key_id = 1,
-	.value_id = 3,
+	.key_type_id = 1,
+	.value_type_id = 3,
 	.max_entries = 4,
 	.btf_load_err = true,
-
+	.err_str = "Member exceeds struct_size",
 },
 
 /* Test member exeeds the size of struct
@@ -335,10 +347,11 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "size_check3_map",
 	.key_size = sizeof(int),
 	.value_size = 1,
-	.key_id = 1,
-	.value_id = 3,
+	.key_type_id = 1,
+	.value_type_id = 3,
 	.max_entries = 4,
 	.btf_load_err = true,
+	.err_str = "Member exceeds struct_size",
 },
 
 /* Test member exceeds the size of struct
@@ -376,10 +389,11 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "size_check4_map",
 	.key_size = sizeof(int),
 	.value_size = 1,
-	.key_id = 1,
-	.value_id = 3,
+	.key_type_id = 1,
+	.value_type_id = 3,
 	.max_entries = 4,
 	.btf_load_err = true,
+	.err_str = "Member exceeds struct_size",
 },
 
 /* typedef const void * const_void_ptr;
@@ -411,8 +425,8 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "void_test1_map",
 	.key_size = sizeof(int),
 	.value_size = sizeof(void *),
-	.key_id = 1,
-	.value_id = 4,
+	.key_type_id = 1,
+	.value_type_id = 4,
 	.max_entries = 4,
 },
 
@@ -440,10 +454,11 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "void_test2_map",
 	.key_size = sizeof(int),
 	.value_size = sizeof(void *),
-	.key_id = 1,
-	.value_id = 3,
+	.key_type_id = 1,
+	.value_type_id = 3,
 	.max_entries = 4,
 	.btf_load_err = true,
+	.err_str = "Invalid member",
 },
 
 /* typedef const void * const_void_ptr;
@@ -458,10 +473,12 @@ static struct btf_raw_test raw_tests[] = {
 		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 0),
 		/* const void* */	/* [3] */
 		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 2),
-		/* typedef const void * const_void_ptr */
+		/* typedef const void * const_void_ptr */	/* [4] */
 		BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 3),
-		/* const_void_ptr[4] */	/* [4] */
-		BTF_TYPE_ARRAY_ENC(3, 1, 4),
+		/* const_void_ptr[4] */	/* [5] */
+		BTF_TYPE_ARRAY_ENC(3, 6, 4),
+		/* unsigned int */	/* [6] */
+		BTF_TYPE_INT_ENC(0, 0, 0, 32, 4),
 		BTF_END_RAW,
 	},
 	.str_sec = "\0const_void_ptr",
@@ -470,8 +487,8 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "void_test3_map",
 	.key_size = sizeof(int),
 	.value_size = sizeof(void *) * 4,
-	.key_id = 1,
-	.value_id = 4,
+	.key_type_id = 1,
+	.value_type_id = 4,
 	.max_entries = 4,
 },
 
@@ -484,7 +501,9 @@ static struct btf_raw_test raw_tests[] = {
 		/* const void */	/* [2] */
 		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 0),
 		/* const void[4] */	/* [3] */
-		BTF_TYPE_ARRAY_ENC(2, 1, 4),
+		BTF_TYPE_ARRAY_ENC(2, 4, 4),
+		/* unsigned int */	/* [4] */
+		BTF_TYPE_INT_ENC(0, 0, 0, 32, 4),
 		BTF_END_RAW,
 	},
 	.str_sec = "\0A\0m",
@@ -493,10 +512,11 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "void_test4_map",
 	.key_size = sizeof(int),
 	.value_size = sizeof(void *) * 4,
-	.key_id = 1,
-	.value_id = 3,
+	.key_type_id = 1,
+	.value_type_id = 3,
 	.max_entries = 4,
 	.btf_load_err = true,
+	.err_str = "Invalid elem",
 },
 
 /* Array_A  <------------------+
@@ -512,9 +532,11 @@ static struct btf_raw_test raw_tests[] = {
 		/* int */			/* [1] */
 		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
 		/* Array_A */			/* [2] */
-		BTF_TYPE_ARRAY_ENC(3, 1, 8),
+		BTF_TYPE_ARRAY_ENC(3, 4, 8),
 		/* Array_B */			/* [3] */
-		BTF_TYPE_ARRAY_ENC(2, 1, 8),
+		BTF_TYPE_ARRAY_ENC(2, 4, 8),
+		/* unsigned int */		/* [4] */
+		BTF_TYPE_INT_ENC(0, 0, 0, 32, 4),
 		BTF_END_RAW,
 	},
 	.str_sec = "",
@@ -523,10 +545,11 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "loop_test1_map",
 	.key_size = sizeof(int),
 	.value_size = sizeof(sizeof(int) * 8),
-	.key_id = 1,
-	.value_id = 2,
+	.key_type_id = 1,
+	.value_type_id = 2,
 	.max_entries = 4,
 	.btf_load_err = true,
+	.err_str = "Loop detected",
 },
 
 /* typedef is _before_ the BTF type of Array_A and Array_B
@@ -548,10 +571,11 @@ static struct btf_raw_test raw_tests[] = {
 		/* typedef Array_B int_array */
 		BTF_TYPEDEF_ENC(1, 4),				/* [2] */
 		/* Array_A */
-		BTF_TYPE_ARRAY_ENC(2, 1, 8),			/* [3] */
+		BTF_TYPE_ARRAY_ENC(2, 5, 8),			/* [3] */
 		/* Array_B */
-		BTF_TYPE_ARRAY_ENC(3, 1, 8),			/* [4] */
-
+		BTF_TYPE_ARRAY_ENC(3, 5, 8),			/* [4] */
+		/* unsigned int */				/* [5] */
+		BTF_TYPE_INT_ENC(0, 0, 0, 32, 4),
 		BTF_END_RAW,
 	},
 	.str_sec = "\0int_array\0",
@@ -560,10 +584,11 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "loop_test2_map",
 	.key_size = sizeof(int),
 	.value_size = sizeof(sizeof(int) * 8),
-	.key_id = 1,
-	.value_id = 2,
+	.key_type_id = 1,
+	.value_type_id = 2,
 	.max_entries = 4,
 	.btf_load_err = true,
+	.err_str = "Loop detected",
 },
 
 /* Array_A  <------------------+
@@ -579,10 +604,11 @@ static struct btf_raw_test raw_tests[] = {
 		/* int */				/* [1] */
 		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
 		/* Array_A */				/* [2] */
-		BTF_TYPE_ARRAY_ENC(3, 1, 8),
+		BTF_TYPE_ARRAY_ENC(3, 4, 8),
 		/* Array_B */				/* [3] */
-		BTF_TYPE_ARRAY_ENC(2, 1, 8),
-
+		BTF_TYPE_ARRAY_ENC(2, 4, 8),
+		/* unsigned int */			/* [4] */
+		BTF_TYPE_INT_ENC(0, 0, 0, 32, 4),
 		BTF_END_RAW,
 	},
 	.str_sec = "",
@@ -591,10 +617,11 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "loop_test3_map",
 	.key_size = sizeof(int),
 	.value_size = sizeof(sizeof(int) * 8),
-	.key_id = 1,
-	.value_id = 2,
+	.key_type_id = 1,
+	.value_type_id = 2,
 	.max_entries = 4,
 	.btf_load_err = true,
+	.err_str = "Loop detected",
 },
 
 /* typedef is _between_ the BTF type of Array_A and Array_B
@@ -614,11 +641,13 @@ static struct btf_raw_test raw_tests[] = {
 		/* int */				/* [1] */
 		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
 		/* Array_A */				/* [2] */
-		BTF_TYPE_ARRAY_ENC(3, 1, 8),
+		BTF_TYPE_ARRAY_ENC(3, 5, 8),
 		/* typedef Array_B int_array */		/* [3] */
 		BTF_TYPEDEF_ENC(NAME_TBD, 4),
 		/* Array_B */				/* [4] */
-		BTF_TYPE_ARRAY_ENC(2, 1, 8),
+		BTF_TYPE_ARRAY_ENC(2, 5, 8),
+		/* unsigned int */			/* [5] */
+		BTF_TYPE_INT_ENC(0, 0, 0, 32, 4),
 		BTF_END_RAW,
 	},
 	.str_sec = "\0int_array\0",
@@ -627,10 +656,11 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "loop_test4_map",
 	.key_size = sizeof(int),
 	.value_size = sizeof(sizeof(int) * 8),
-	.key_id = 1,
-	.value_id = 2,
+	.key_type_id = 1,
+	.value_type_id = 2,
 	.max_entries = 4,
 	.btf_load_err = true,
+	.err_str = "Loop detected",
 },
 
 /* typedef struct B Struct_B
@@ -668,10 +698,11 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "loop_test5_map",
 	.key_size = sizeof(int),
 	.value_size = 8,
-	.key_id = 1,
-	.value_id = 2,
+	.key_type_id = 1,
+	.value_type_id = 2,
 	.max_entries = 4,
 	.btf_load_err = true,
+	.err_str = "Loop detected",
 },
 
 /* struct A {
@@ -684,11 +715,13 @@ static struct btf_raw_test raw_tests[] = {
 	.raw_types = {
 		/* int */
 		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),	/* [1] */
-		BTF_TYPE_ARRAY_ENC(3, 1, 4),			/* [2] */
+		BTF_TYPE_ARRAY_ENC(3, 4, 4),			/* [2] */
 		/* struct A */					/* [3] */
 		BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 2), 8),
 		BTF_MEMBER_ENC(NAME_TBD, 1, 0),	/* int x;		*/
 		BTF_MEMBER_ENC(NAME_TBD, 2, 32),/* struct A array_a[4];	*/
+		/* unsigned int */				/* [4] */
+		BTF_TYPE_INT_ENC(0, 0, 0, 32, 4),
 		BTF_END_RAW,
 	},
 	.str_sec = "\0A\0x\0y",
@@ -697,10 +730,11 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "loop_test6_map",
 	.key_size = sizeof(int),
 	.value_size = 8,
-	.key_id = 1,
-	.value_id = 2,
+	.key_type_id = 1,
+	.value_type_id = 2,
 	.max_entries = 4,
 	.btf_load_err = true,
+	.err_str = "Loop detected",
 },
 
 {
@@ -724,10 +758,11 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "loop_test7_map",
 	.key_size = sizeof(int),
 	.value_size = sizeof(void *),
-	.key_id = 1,
-	.value_id = 2,
+	.key_type_id = 1,
+	.value_type_id = 2,
 	.max_entries = 4,
 	.btf_load_err = true,
+	.err_str = "Loop detected",
 },
 
 {
@@ -759,14 +794,73 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "loop_test8_map",
 	.key_size = sizeof(int),
 	.value_size = sizeof(void *),
-	.key_id = 1,
-	.value_id = 2,
+	.key_type_id = 1,
+	.value_type_id = 2,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Loop detected",
+},
+
+{
+	.descr = "string section does not end with null",
+	.raw_types = {
+		/* int */				/* [1] */
+		BTF_TYPE_INT_ENC(NAME_TBD, BTF_INT_SIGNED, 0, 32, 4),
+		BTF_END_RAW,
+	},
+	.str_sec = "\0int",
+	.str_sec_size = sizeof("\0int") - 1,
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "hdr_test_map",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid string section",
+},
+
+{
+	.descr = "empty string section",
+	.raw_types = {
+		/* int */				/* [1] */
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
+		BTF_END_RAW,
+	},
+	.str_sec = "",
+	.str_sec_size = 0,
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "hdr_test_map",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid string section",
+},
+
+{
+	.descr = "empty type section",
+	.raw_types = {
+		BTF_END_RAW,
+	},
+	.str_sec = "\0int",
+	.str_sec_size = sizeof("\0int"),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "hdr_test_map",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
 	.max_entries = 4,
 	.btf_load_err = true,
+	.err_str = "No type found",
 },
 
 {
-	.descr = "type_off == str_off",
+	.descr = "btf_header test #1. Longer hdr_len",
 	.raw_types = {
 		/* int */				/* [1] */
 		BTF_TYPE_INT_ENC(NAME_TBD, BTF_INT_SIGNED, 0, 32, 4),
@@ -778,15 +872,16 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "hdr_test_map",
 	.key_size = sizeof(int),
 	.value_size = sizeof(int),
-	.key_id = 1,
-	.value_id = 1,
+	.key_type_id = 1,
+	.value_type_id = 1,
 	.max_entries = 4,
 	.btf_load_err = true,
-	.type_off_delta = sizeof(struct btf_type) + sizeof(int) + sizeof("\0int"),
+	.hdr_len_delta = 4,
+	.err_str = "Unsupported btf_header",
 },
 
 {
-	.descr = "Unaligned type_off",
+	.descr = "btf_header test #2. Gap between hdr and type",
 	.raw_types = {
 		/* int */				/* [1] */
 		BTF_TYPE_INT_ENC(NAME_TBD, BTF_INT_SIGNED, 0, 32, 4),
@@ -798,15 +893,16 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "hdr_test_map",
 	.key_size = sizeof(int),
 	.value_size = sizeof(int),
-	.key_id = 1,
-	.value_id = 1,
+	.key_type_id = 1,
+	.value_type_id = 1,
 	.max_entries = 4,
 	.btf_load_err = true,
-	.type_off_delta = 1,
+	.type_off_delta = 4,
+	.err_str = "Unsupported section found",
 },
 
 {
-	.descr = "str_off beyonds btf size",
+	.descr = "btf_header test #3. Gap between type and str",
 	.raw_types = {
 		/* int */				/* [1] */
 		BTF_TYPE_INT_ENC(NAME_TBD, BTF_INT_SIGNED, 0, 32, 4),
@@ -818,15 +914,16 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "hdr_test_map",
 	.key_size = sizeof(int),
 	.value_size = sizeof(int),
-	.key_id = 1,
-	.value_id = 1,
+	.key_type_id = 1,
+	.value_type_id = 1,
 	.max_entries = 4,
 	.btf_load_err = true,
-	.str_off_delta = sizeof("\0int") + 1,
+	.str_off_delta = 4,
+	.err_str = "Unsupported section found",
 },
 
 {
-	.descr = "str_len beyonds btf size",
+	.descr = "btf_header test #4. Overlap between type and str",
 	.raw_types = {
 		/* int */				/* [1] */
 		BTF_TYPE_INT_ENC(NAME_TBD, BTF_INT_SIGNED, 0, 32, 4),
@@ -838,15 +935,16 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "hdr_test_map",
 	.key_size = sizeof(int),
 	.value_size = sizeof(int),
-	.key_id = 1,
-	.value_id = 1,
+	.key_type_id = 1,
+	.value_type_id = 1,
 	.max_entries = 4,
 	.btf_load_err = true,
-	.str_len_delta = 1,
+	.str_off_delta = -4,
+	.err_str = "Section overlap found",
 },
 
 {
-	.descr = "String section does not end with null",
+	.descr = "btf_header test #5. Larger BTF size",
 	.raw_types = {
 		/* int */				/* [1] */
 		BTF_TYPE_INT_ENC(NAME_TBD, BTF_INT_SIGNED, 0, 32, 4),
@@ -858,15 +956,16 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "hdr_test_map",
 	.key_size = sizeof(int),
 	.value_size = sizeof(int),
-	.key_id = 1,
-	.value_id = 1,
+	.key_type_id = 1,
+	.value_type_id = 1,
 	.max_entries = 4,
 	.btf_load_err = true,
-	.str_len_delta = -1,
+	.str_len_delta = -4,
+	.err_str = "Unsupported section found",
 },
 
 {
-	.descr = "Empty string section",
+	.descr = "btf_header test #6. Smaller BTF size",
 	.raw_types = {
 		/* int */				/* [1] */
 		BTF_TYPE_INT_ENC(NAME_TBD, BTF_INT_SIGNED, 0, 32, 4),
@@ -878,11 +977,223 @@ static struct btf_raw_test raw_tests[] = {
 	.map_name = "hdr_test_map",
 	.key_size = sizeof(int),
 	.value_size = sizeof(int),
-	.key_id = 1,
-	.value_id = 1,
+	.key_type_id = 1,
+	.value_type_id = 1,
 	.max_entries = 4,
 	.btf_load_err = true,
-	.str_len_delta = 0 - (int)sizeof("\0int"),
+	.str_len_delta = 4,
+	.err_str = "Total section length too long",
+},
+
+{
+	.descr = "array test #1. index_type \"unsigned int\"",
+	.raw_types = {
+		/* int */				/* [1] */
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
+		/* unsigned int */			/* [2] */
+		BTF_TYPE_INT_ENC(0, 0, 0, 32, 4),
+		/* unsigned int[16] */			/* [3] */
+		BTF_TYPE_ARRAY_ENC(1, 2, 16),
+		BTF_END_RAW,
+	},
+	.str_sec = "",
+	.str_sec_size = sizeof(""),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "array_test_map",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+},
+
+{
+	.descr = "array test #2. index_type \"const unsigned int\"",
+	.raw_types = {
+		/* int */				/* [1] */
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
+		/* unsigned int */			/* [2] */
+		BTF_TYPE_INT_ENC(0, 0, 0, 32, 4),
+		/* int[16] */				/* [3] */
+		BTF_TYPE_ARRAY_ENC(1, 4, 16),
+		/* CONST type_id=2 */			/* [4] */
+		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 2),
+		BTF_END_RAW,
+	},
+	.str_sec = "",
+	.str_sec_size = sizeof(""),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "array_test_map",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+},
+
+{
+	.descr = "array test #3. index_type \"int\"",
+	.raw_types = {
+		/* int */				/* [1] */
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
+		/* int[16] */				/* [2] */
+		BTF_TYPE_ARRAY_ENC(1, 1, 16),
+		BTF_END_RAW,
+	},
+	.str_sec = "",
+	.str_sec_size = sizeof(""),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "array_test_map",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid index",
+},
+
+{
+	.descr = "array test #3. index_type \"const int\"",
+	.raw_types = {
+		/* int */				/* [1] */
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
+		/* int[16] */				/* [2] */
+		BTF_TYPE_ARRAY_ENC(1, 3, 16),
+		/* CONST type_id=1 */			/* [3] */
+		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 1),
+		BTF_END_RAW,
+	},
+	.str_sec = "",
+	.str_sec_size = sizeof(""),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "array_test_map",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid index",
+},
+
+{
+	.descr = "array test #4. index_type \"void\"",
+	.raw_types = {
+		/* int */				/* [1] */
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
+		/* int[16] */				/* [2] */
+		BTF_TYPE_ARRAY_ENC(1, 0, 16),
+		BTF_END_RAW,
+	},
+	.str_sec = "",
+	.str_sec_size = sizeof(""),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "array_test_map",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid index",
+},
+
+{
+	.descr = "array test #5. index_type \"const void\"",
+	.raw_types = {
+		/* int */				/* [1] */
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
+		/* int[16] */				/* [2] */
+		BTF_TYPE_ARRAY_ENC(1, 3, 16),
+		/* CONST type_id=0 (void) */		/* [3] */
+		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 0),
+		BTF_END_RAW,
+	},
+	.str_sec = "",
+	.str_sec_size = sizeof(""),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "array_test_map",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid index",
+},
+
+{
+	.descr = "array test #6. elem_type \"const void *\"",
+	.raw_types = {
+		/* int */				/* [1] */
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
+		/* const void *[16] */			/* [2] */
+		BTF_TYPE_ARRAY_ENC(3, 5, 16),
+		/* CONST type_id=4 */			/* [3] */
+		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 4),
+		/* void* */				/* [4] */
+		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 0),
+		/* unsigned int */			/* [5] */
+		BTF_TYPE_INT_ENC(0, 0, 0, 32, 4),
+		BTF_END_RAW,
+	},
+	.str_sec = "",
+	.str_sec_size = sizeof(""),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "array_test_map",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+},
+
+{
+	.descr = "array test #7. index_type \"const void *\"",
+	.raw_types = {
+		/* int */				/* [1] */
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
+		/* const void *[16] */			/* [2] */
+		BTF_TYPE_ARRAY_ENC(3, 3, 16),
+		/* CONST type_id=4 */			/* [3] */
+		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 4),
+		/* void* */				/* [4] */
+		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 0),
+		BTF_END_RAW,
+	},
+	.str_sec = "",
+	.str_sec_size = sizeof(""),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "array_test_map",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid index",
+},
+
+{
+	.descr = "invalid BTF_INFO",
+	.raw_types = {
+		/* int */				/* [1] */
+		BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4),
+		BTF_TYPE_ENC(0, 0x10000000, 4),
+		BTF_END_RAW,
+	},
+	.str_sec = "",
+	.str_sec_size = sizeof(""),
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "array_test_map",
+	.key_size = sizeof(int),
+	.value_size = sizeof(int),
+	.key_type_id = 1,
+	.value_type_id = 1,
+	.max_entries = 4,
+	.btf_load_err = true,
+	.err_str = "Invalid btf_info",
 },
 
 }; /* struct btf_raw_test raw_tests[] */
@@ -951,6 +1262,7 @@ static void *btf_raw_create(const struct btf_header *hdr,
 	memcpy(raw_btf + offset, str, str_sec_size);
 
 	ret_hdr = (struct btf_header *)raw_btf;
+	ret_hdr->type_len = type_sec_size;
 	ret_hdr->str_off = type_sec_size;
 	ret_hdr->str_len = str_sec_size;
 
@@ -981,6 +1293,7 @@ static int do_test_raw(unsigned int test_num)
 
 	hdr = raw_btf;
 
+	hdr->hdr_len = (int)hdr->hdr_len + test->hdr_len_delta;
 	hdr->type_off = (int)hdr->type_off + test->type_off_delta;
 	hdr->str_off = (int)hdr->str_off + test->str_off_delta;
 	hdr->str_len = (int)hdr->str_len + test->str_len_delta;
@@ -992,8 +1305,13 @@ static int do_test_raw(unsigned int test_num)
 	free(raw_btf);
 
 	err = ((btf_fd == -1) != test->btf_load_err);
-	CHECK(err, "btf_fd:%d test->btf_load_err:%u",
-	      btf_fd, test->btf_load_err);
+	if (CHECK(err, "btf_fd:%d test->btf_load_err:%u",
+		  btf_fd, test->btf_load_err) ||
+	    CHECK(test->err_str && !strstr(btf_log_buf, test->err_str),
+		  "expected err_str:%s", test->err_str)) {
+		err = -1;
+		goto done;
+	}
 
 	if (err || btf_fd == -1)
 		goto done;
@@ -1004,8 +1322,8 @@ static int do_test_raw(unsigned int test_num)
 	create_attr.value_size = test->value_size;
 	create_attr.max_entries = test->max_entries;
 	create_attr.btf_fd = btf_fd;
-	create_attr.btf_key_id = test->key_id;
-	create_attr.btf_value_id = test->value_id;
+	create_attr.btf_key_type_id = test->key_type_id;
+	create_attr.btf_value_type_id = test->value_type_id;
 
 	map_fd = bpf_create_map_xattr(&create_attr);
 
@@ -1267,8 +1585,8 @@ static int test_btf_id(unsigned int test_num)
 	create_attr.value_size = sizeof(unsigned int);
 	create_attr.max_entries = 4;
 	create_attr.btf_fd = btf_fd[0];
-	create_attr.btf_key_id = 1;
-	create_attr.btf_value_id = 2;
+	create_attr.btf_key_type_id = 1;
+	create_attr.btf_value_type_id = 2;
 
 	map_fd = bpf_create_map_xattr(&create_attr);
 	if (CHECK(map_fd == -1, "errno:%d", errno)) {
@@ -1279,10 +1597,10 @@ static int test_btf_id(unsigned int test_num)
 	info_len = sizeof(map_info);
 	err = bpf_obj_get_info_by_fd(map_fd, &map_info, &info_len);
 	if (CHECK(err || map_info.btf_id != info[0].id ||
-		  map_info.btf_key_id != 1 || map_info.btf_value_id != 2,
-		  "err:%d errno:%d info.id:%u btf_id:%u btf_key_id:%u btf_value_id:%u",
-		  err, errno, info[0].id, map_info.btf_id, map_info.btf_key_id,
-		  map_info.btf_value_id)) {
+		  map_info.btf_key_type_id != 1 || map_info.btf_value_type_id != 2,
+		  "err:%d errno:%d info.id:%u btf_id:%u btf_key_type_id:%u btf_value_type_id:%u",
+		  err, errno, info[0].id, map_info.btf_id, map_info.btf_key_type_id,
+		  map_info.btf_value_type_id)) {
 		err = -1;
 		goto done;
 	}
@@ -1542,10 +1860,10 @@ static int do_test_file(unsigned int test_num)
 		goto done;
 	}
 
-	err = (bpf_map__btf_key_id(map) == 0 || bpf_map__btf_value_id(map) == 0)
+	err = (bpf_map__btf_key_type_id(map) == 0 || bpf_map__btf_value_type_id(map) == 0)
 		!= test->btf_kv_notfound;
-	if (CHECK(err, "btf_key_id:%u btf_value_id:%u test->btf_kv_notfound:%u",
-		  bpf_map__btf_key_id(map), bpf_map__btf_value_id(map),
+	if (CHECK(err, "btf_key_type_id:%u btf_value_type_id:%u test->btf_kv_notfound:%u",
+		  bpf_map__btf_key_type_id(map), bpf_map__btf_value_type_id(map),
 		  test->btf_kv_notfound))
 		goto done;
 
@@ -1654,8 +1972,8 @@ static struct btf_raw_test pprint_test = {
 	.map_name = "pprint_test",
 	.key_size = sizeof(unsigned int),
 	.value_size = sizeof(struct pprint_mapv),
-	.key_id = 3,	/* unsigned int */
-	.value_id = 16,	/* struct pprint_mapv */
+	.key_type_id = 3,	/* unsigned int */
+	.value_type_id = 16,	/* struct pprint_mapv */
 	.max_entries = 128 * 1024,
 };
 
@@ -1712,8 +2030,8 @@ static int test_pprint(void)
 	create_attr.value_size = test->value_size;
 	create_attr.max_entries = test->max_entries;
 	create_attr.btf_fd = btf_fd;
-	create_attr.btf_key_id = test->key_id;
-	create_attr.btf_value_id = test->value_id;
+	create_attr.btf_key_type_id = test->key_type_id;
+	create_attr.btf_value_type_id = test->value_type_id;
 
 	map_fd = bpf_create_map_xattr(&create_attr);
 	if (CHECK(map_fd == -1, "errno:%d", errno)) {
-- 
2.9.5

[PATCH bpf-next 6/7] bpf: btf: Sync bpf.h and btf.h to tools/include/uapi/linux/

[Martin KaFai Lau]

This patch sync the uapi's bpf.h and btf.h to tools/.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 tools/include/uapi/linux/bpf.h |  8 ++++----
 tools/include/uapi/linux/btf.h | 28 +++++++---------------------
 2 files changed, 11 insertions(+), 25 deletions(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index d94d333a8225..123ebe4b3662 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -284,8 +284,8 @@ union bpf_attr {
 		char	map_name[BPF_OBJ_NAME_LEN];
 		__u32	map_ifindex;	/* ifindex of netdev to create on */
 		__u32	btf_fd;		/* fd pointing to a BTF type data */
-		__u32	btf_key_id;	/* BTF type_id of the key */
-		__u32	btf_value_id;	/* BTF type_id of the value */
+		__u32	btf_key_type_id;	/* BTF type_id of the key */
+		__u32	btf_value_type_id;	/* BTF type_id of the value */
 	};
 
 	struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */
@@ -2211,8 +2211,8 @@ struct bpf_map_info {
 	__u64 netns_dev;
 	__u64 netns_ino;
 	__u32 btf_id;
-	__u32 btf_key_id;
-	__u32 btf_value_id;
+	__u32 btf_key_type_id;
+	__u32 btf_value_type_id;
 } __attribute__((aligned(8)));
 
 struct bpf_btf_info {
diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h
index bcb56ee47014..b89b56f2b099 100644
--- a/tools/include/uapi/linux/btf.h
+++ b/tools/include/uapi/linux/btf.h
@@ -12,42 +12,29 @@ struct btf_header {
 	__u16	magic;
 	__u8	version;
 	__u8	flags;
-
-	__u32	parent_label;
-	__u32	parent_name;
+	__u32	hdr_len;
 
 	/* All offsets are in bytes relative to the end of this header */
-	__u32	label_off;	/* offset of label section	*/
-	__u32	object_off;	/* offset of data object section*/
-	__u32	func_off;	/* offset of function section	*/
 	__u32	type_off;	/* offset of type section	*/
+	__u32	type_len;	/* length of type section	*/
 	__u32	str_off;	/* offset of string section	*/
 	__u32	str_len;	/* length of string section	*/
 };
 
 /* Max # of type identifier */
-#define BTF_MAX_TYPE	0x7fffffff
+#define BTF_MAX_TYPE	0x0000ffff
 /* Max offset into the string section */
-#define BTF_MAX_NAME_OFFSET	0x7fffffff
+#define BTF_MAX_NAME_OFFSET	0x0000ffff
 /* Max # of struct/union/enum members or func args */
 #define BTF_MAX_VLEN	0xffff
 
-/* The type id is referring to a parent BTF */
-#define BTF_TYPE_PARENT(id)	(((id) >> 31) & 0x1)
-#define BTF_TYPE_ID(id)		((id) & BTF_MAX_TYPE)
-
-/* String is in the ELF string section */
-#define BTF_STR_TBL_ELF_ID(ref)	(((ref) >> 31) & 0x1)
-#define BTF_STR_OFFSET(ref)	((ref) & BTF_MAX_NAME_OFFSET)
-
 struct btf_type {
 	__u32 name_off;
 	/* "info" bits arrangement
 	 * bits  0-15: vlen (e.g. # of struct's members)
 	 * bits 16-23: unused
-	 * bits 24-28: kind (e.g. int, ptr, array...etc)
-	 * bits 29-30: unused
-	 * bits    31: root
+	 * bits 24-27: kind (e.g. int, ptr, array...etc)
+	 * bits 28-31: unused
 	 */
 	__u32 info;
 	/* "size" is used by INT, ENUM, STRUCT and UNION.
@@ -62,8 +49,7 @@ struct btf_type {
 	};
 };
 
-#define BTF_INFO_KIND(info)	(((info) >> 24) & 0x1f)
-#define BTF_INFO_ISROOT(info)	(!!(((info) >> 24) & 0x80))
+#define BTF_INFO_KIND(info)	(((info) >> 24) & 0x0f)
 #define BTF_INFO_VLEN(info)	((info) & 0xffff)
 
 #define BTF_KIND_UNKN		0	/* Unknown	*/
-- 
2.9.5

Re: [PATCH bpf-next 1/7] bpf: Expose check_uarg_tail_zero()

[Yonghong Song]

On 5/18/18 5:16 PM, Martin KaFai Lau wrote:
> This patch exposes check_uarg_tail_zero() which will
> be reused by a later BTF patch.  Its name is changed to
> bpf_check_uarg_tail_zero().
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>

Acked-by: Yonghong Song <yhs@fb.com>

Re: [PATCH bpf-next 2/7] bpf: btf: Change how section is supported in btf_header

[Yonghong Song]

On 5/18/18 5:16 PM, Martin KaFai Lau wrote:
> There are currently unused section descriptions in the btf_header.  Those
> sections are here to support future BTF use cases.  For example, the
> func section (func_off) is to support function signature (e.g. the BPF
> prog function signature).
> 
> Instead of spelling out all potential sections up-front in the btf_header.
> This patch makes changes to btf_header such that extending it (e.g. adding
> a section) is possible later.  The unused ones can be removed for now and
> they can be added back later.
> 
> This patch:
> 1. adds a hdr_len to the btf_header.  It will allow adding
> sections (and other info like parent_label and parent_name)
> later.  The check is similar to the existing bpf_attr.
> If a user passes in a longer hdr_len, the kernel
> ensures the extra tailing bytes are 0.
> 
> 2. allows the section order in the BTF object to be
> different from its sec_off order in btf_header.
> 
> 3. each sec_off is followed by a sec_len.  It must not have gap or
> overlapping among sections.
> 
> The string section is ensured to be at the end due to the 4 bytes
> alignment requirement of the type section.
> 
> The above changes will allow enough flexibility to
> add new sections (and other info) to the btf_header later.
> 
> This patch also removes an unnecessary !err check
> at the end of btf_parse().
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
>   include/uapi/linux/btf.h |   8 +-
>   kernel/bpf/btf.c         | 207 +++++++++++++++++++++++++++++++++++------------
>   2 files changed, 158 insertions(+), 57 deletions(-)
> 
> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
> index bcb56ee47014..4fa479741a02 100644
> --- a/include/uapi/linux/btf.h
> +++ b/include/uapi/linux/btf.h
> @@ -12,15 +12,11 @@ struct btf_header {
>   	__u16	magic;
>   	__u8	version;
>   	__u8	flags;
> -
> -	__u32	parent_label;
> -	__u32	parent_name;
> +	__u32	hdr_len;
>   
>   	/* All offsets are in bytes relative to the end of this header */
> -	__u32	label_off;	/* offset of label section	*/
> -	__u32	object_off;	/* offset of data object section*/
> -	__u32	func_off;	/* offset of function section	*/
>   	__u32	type_off;	/* offset of type section	*/
> +	__u32	type_len;	/* length of type section	*/
>   	__u32	str_off;	/* offset of string section	*/
>   	__u32	str_len;	/* length of string section	*/
>   };
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index ded10ab47b8a..536e5981ad8c 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -12,6 +12,7 @@
>   #include <linux/uaccess.h>
>   #include <linux/kernel.h>
>   #include <linux/idr.h>
> +#include <linux/sort.h>
>   #include <linux/bpf_verifier.h>
>   #include <linux/btf.h>
>   
> @@ -184,15 +185,13 @@ static DEFINE_IDR(btf_idr);
>   static DEFINE_SPINLOCK(btf_idr_lock);
>   
>   struct btf {
> -	union {
> -		struct btf_header *hdr;
> -		void *data;
> -	};
> +	void *data;
>   	struct btf_type **types;
>   	u32 *resolved_ids;
>   	u32 *resolved_sizes;
>   	const char *strings;
>   	void *nohdr_data;
> +	struct btf_header hdr;
>   	u32 nr_types;
>   	u32 types_size;
>   	u32 data_size;
> @@ -227,6 +226,12 @@ enum resolve_mode {
>   };
>   
>   #define MAX_RESOLVE_DEPTH 32
> +#define NR_SECS 2

Not sure whether it is necessary to define NR_SECS 2 here or not.
See below.

> +
> +struct btf_sec_info {
> +	u32 off;
> +	u32 len;
> +};
>   
>   struct btf_verifier_env {
>   	struct btf *btf;
> @@ -418,14 +423,14 @@ static const struct btf_kind_operations *btf_type_ops(const struct btf_type *t)
>   static bool btf_name_offset_valid(const struct btf *btf, u32 offset)
>   {
>   	return !BTF_STR_TBL_ELF_ID(offset) &&
> -		BTF_STR_OFFSET(offset) < btf->hdr->str_len;
> +		BTF_STR_OFFSET(offset) < btf->hdr.str_len;
>   }
>   
>   static const char *btf_name_by_offset(const struct btf *btf, u32 offset)
>   {
>   	if (!BTF_STR_OFFSET(offset))
>   		return "(anon)";
> -	else if (BTF_STR_OFFSET(offset) < btf->hdr->str_len)
> +	else if (BTF_STR_OFFSET(offset) < btf->hdr.str_len)
>   		return &btf->strings[BTF_STR_OFFSET(offset)];
>   	else
>   		return "(invalid-name-offset)";
> @@ -536,7 +541,8 @@ static void btf_verifier_log_member(struct btf_verifier_env *env,
>   	__btf_verifier_log(log, "\n");
>   }
>   
> -static void btf_verifier_log_hdr(struct btf_verifier_env *env)
> +static void btf_verifier_log_hdr(struct btf_verifier_env *env,
> +				 u32 btf_data_size)
>   {
>   	struct bpf_verifier_log *log = &env->log;
>   	const struct btf *btf = env->btf;
> @@ -545,19 +551,16 @@ static void btf_verifier_log_hdr(struct btf_verifier_env *env)
>   	if (!bpf_verifier_log_needed(log))
>   		return;
>   
> -	hdr = btf->hdr;
> +	hdr = &btf->hdr;
>   	__btf_verifier_log(log, "magic: 0x%x\n", hdr->magic);
>   	__btf_verifier_log(log, "version: %u\n", hdr->version);
>   	__btf_verifier_log(log, "flags: 0x%x\n", hdr->flags);
> -	__btf_verifier_log(log, "parent_label: %u\n", hdr->parent_label);
> -	__btf_verifier_log(log, "parent_name: %u\n", hdr->parent_name);
> -	__btf_verifier_log(log, "label_off: %u\n", hdr->label_off);
> -	__btf_verifier_log(log, "object_off: %u\n", hdr->object_off);
> -	__btf_verifier_log(log, "func_off: %u\n", hdr->func_off);
> +	__btf_verifier_log(log, "hdr_len: %u\n", hdr->hdr_len);
>   	__btf_verifier_log(log, "type_off: %u\n", hdr->type_off);
> +	__btf_verifier_log(log, "type_len: %u\n", hdr->type_len);
>   	__btf_verifier_log(log, "str_off: %u\n", hdr->str_off);
>   	__btf_verifier_log(log, "str_len: %u\n", hdr->str_len);
> -	__btf_verifier_log(log, "btf_total_size: %u\n", btf->data_size);
> +	__btf_verifier_log(log, "btf_total_size: %u\n", btf_data_size);
>   }
>   
>   static int btf_add_type(struct btf_verifier_env *env, struct btf_type *t)
> @@ -1754,9 +1757,9 @@ static int btf_check_all_metas(struct btf_verifier_env *env)
>   	struct btf_header *hdr;
>   	void *cur, *end;
>   
> -	hdr = btf->hdr;
> +	hdr = &btf->hdr;
>   	cur = btf->nohdr_data + hdr->type_off;
> -	end = btf->nohdr_data + hdr->str_off;
> +	end = btf->nohdr_data + hdr->type_len;
>   
>   	env->log_type_id = 1;
>   	while (cur < end) {
> @@ -1866,8 +1869,20 @@ static int btf_check_all_types(struct btf_verifier_env *env)
>   
>   static int btf_parse_type_sec(struct btf_verifier_env *env)
>   {
> +	const struct btf_header *hdr = &env->btf->hdr;
>   	int err;
>   
> +	/* Type section must align to 4 bytes */
> +	if (hdr->type_off & (sizeof(u32) - 1)) {
> +		btf_verifier_log(env, "Unaligned type_off");
> +		return -EINVAL;
> +	}
> +
> +	if (!hdr->type_len) {
> +		btf_verifier_log(env, "No type found");
> +		return -EINVAL;
> +	}
> +
>   	err = btf_check_all_metas(env);
>   	if (err)
>   		return err;
> @@ -1881,10 +1896,15 @@ static int btf_parse_str_sec(struct btf_verifier_env *env)
>   	struct btf *btf = env->btf;
>   	const char *start, *end;
>   
> -	hdr = btf->hdr;
> +	hdr = &btf->hdr;
>   	start = btf->nohdr_data + hdr->str_off;
>   	end = start + hdr->str_len;
>   
> +	if (end != btf->data + btf->data_size) {
> +		btf_verifier_log(env, "String section is not at the end");
> +		return -EINVAL;
> +	}
> +
>   	if (!hdr->str_len || hdr->str_len - 1 > BTF_MAX_NAME_OFFSET ||
>   	    start[0] || end[-1]) {
>   		btf_verifier_log(env, "Invalid string section");
> @@ -1896,20 +1916,119 @@ static int btf_parse_str_sec(struct btf_verifier_env *env)
>   	return 0;
>   }
>   
> -static int btf_parse_hdr(struct btf_verifier_env *env)
> +static const size_t btf_sec_info_offset[] = {
> +	offsetof(struct btf_header, type_off),
> +	offsetof(struct btf_header, str_off),
> +};

Maybe we can define NR_SECS is 
ARRAY_SIZE(btf_sec_info_offset)/sizeof(size_t)?

> +
> +static int btf_sec_info_cmp(const void *a, const void *b)
>   {
> +	const struct btf_sec_info *x = a;
> +	const struct btf_sec_info *y = b;
> +
> +	return (int)(x->off - y->off) ? : (int)(x->len - y->len);
> +}
> +
> +static int btf_check_sec_info(struct btf_verifier_env *env,
> +			      u32 btf_data_size)
> +{
> +	struct btf_sec_info secs[NR_SECS];
> +	u32 total, expected_total, i;
>   	const struct btf_header *hdr;
> -	struct btf *btf = env->btf;
> -	u32 meta_left;
> +	const struct btf *btf;
> +
> +	BUILD_BUG_ON(ARRAY_SIZE(btf_sec_info_offset) != NR_SECS);

If we define NR_SECS depending on the size of btf_sec_info_offset, this
BUILD_BUG_ON is not needed.

> +
> +	btf = env->btf;
> +	hdr = &btf->hdr;
> +
> +	/* Populate the secs from hdr */
> +	for (i = 0; i < NR_SECS; i++)
> +		secs[i] = *(struct btf_sec_info *)((void *)hdr +
> +						   btf_sec_info_offset[i]);
> +
> +	sort(secs, NR_SECS, sizeof(struct btf_sec_info),
> +	     btf_sec_info_cmp, NULL);
> +
> +	/* Check for gaps and overlap among sections */
> +	total = 0;
> +	expected_total = btf_data_size - hdr->hdr_len;
> +	for (i = 0; i < NR_SECS; i++) {
> +		if (expected_total < secs[i].off) {
> +			btf_verifier_log(env, "Invalid section offset");
> +			return -EINVAL;
> +		}
> +		if (total < secs[i].off) {
> +			/* gap */
> +			btf_verifier_log(env, "Unsupported section found");
> +			return -EINVAL;
> +		}
> +		if (total > secs[i].off) {
> +			btf_verifier_log(env, "Section overlap found");
> +			return -EINVAL;
> +		}
> +		if (expected_total - total < secs[i].len) {
> +			btf_verifier_log(env,
> +					 "Total section length too long");
> +			return -EINVAL;
> +		}
> +		total += secs[i].len;
> +	}
> +
> +	/* There is data other than hdr and known sections */
> +	if (expected_total != total) {
> +		btf_verifier_log(env, "Unsupported section found");
> +		return -EINVAL;
> +	}

Does this patch try to address compatibility issue? That is, the old btf 
format with 2 sections should still work with future kernel with more
than 2 sections? The above comparision seems suggesting that newer 
kernel will not support non-compatible number of sections?

> +
> +	return 0;
> +}
> +
> +static int btf_parse_hdr(struct btf_verifier_env *env, void __user *btf_data,
> +			 u32 btf_data_size)
> +{
> +	const struct btf_header *hdr;
> +	u32 hdr_len, hdr_copy;
> +	struct btf_min_header {
> +		u16	magic;
> +		u8	version;
> +		u8	flags;
> +		u32	hdr_len;
> +	} __user *min_hdr;

This is the partial structure with the first four fields
for struct btf_header.
It is okay, but I feel a little bit duplication here.

Looks like only sizeof(*min_hdr) and min_hdr->hdr_len is used.
Maybe we can just cast bpf_data to bpf_header and
sizeof(*min_hdr) being offsetof(bpf_data, type_off), and
min_hdr->hdr_len being bpf_data->hdr_len.

Do this work?

> +	struct btf *btf;
> +	int err;
> +
> +	btf = env->btf;
> +	min_hdr = btf_data;
> +
> +	if (btf_data_size < sizeof(*min_hdr)) {
> +		btf_verifier_log(env, "hdr_len not found");
> +		return -EINVAL;
> +	}
>   
> -	if (btf->data_size < sizeof(*hdr)) {
> +	if (get_user(hdr_len, &min_hdr->hdr_len))
> +		return -EFAULT;
> +
> +	if (btf_data_size < hdr_len) {
>   		btf_verifier_log(env, "btf_header not found");
>   		return -EINVAL;
>   	}
>   
> -	btf_verifier_log_hdr(env);
> +	err = bpf_check_uarg_tail_zero(btf_data, sizeof(btf->hdr), hdr_len);
> +	if (err) {
> +		if (err == -E2BIG)
> +			btf_verifier_log(env, "Unsupported btf_header");
> +		return err;
> +	}
> +
> +	hdr_copy = min_t(u32, hdr_len, sizeof(btf->hdr));
> +	if (copy_from_user(&btf->hdr, btf_data, hdr_copy))
> +		return -EFAULT;
> +
> +	hdr = &btf->hdr;
> +
> +	btf_verifier_log_hdr(env, btf_data_size);
>   
> -	hdr = btf->hdr;
>   	if (hdr->magic != BTF_MAGIC) {
>   		btf_verifier_log(env, "Invalid magic");
>   		return -EINVAL;
> @@ -1925,26 +2044,14 @@ static int btf_parse_hdr(struct btf_verifier_env *env)
>   		return -ENOTSUPP;
>   	}
>   
> -	meta_left = btf->data_size - sizeof(*hdr);
> -	if (!meta_left) {
> +	if (btf_data_size == hdr->hdr_len) {
>   		btf_verifier_log(env, "No data");
>   		return -EINVAL;
>   	}
>   
> -	if (meta_left < hdr->type_off || hdr->str_off <= hdr->type_off ||
> -	    /* Type section must align to 4 bytes */
> -	    hdr->type_off & (sizeof(u32) - 1)) {
> -		btf_verifier_log(env, "Invalid type_off");
> -		return -EINVAL;
> -	}
> -
> -	if (meta_left < hdr->str_off ||
> -	    meta_left - hdr->str_off < hdr->str_len) {
> -		btf_verifier_log(env, "Invalid str_off or str_len");
> -		return -EINVAL;
> -	}
> -
> -	btf->nohdr_data = btf->hdr + 1;
> +	err = btf_check_sec_info(env, btf_data_size);
> +	if (err)
> +		return err;
>   
>   	return 0;
>   }
> @@ -1987,6 +2094,11 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
>   		err = -ENOMEM;
>   		goto errout;
>   	}
> +	env->btf = btf;
> +
> +	err = btf_parse_hdr(env, btf_data, btf_data_size);
> +	if (err)
> +		goto errout;
>   
>   	data = kvmalloc(btf_data_size, GFP_KERNEL | __GFP_NOWARN);
>   	if (!data) {
> @@ -1996,18 +2108,13 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
>   
>   	btf->data = data;
>   	btf->data_size = btf_data_size;
> +	btf->nohdr_data = btf->data + btf->hdr.hdr_len;
>   
>   	if (copy_from_user(data, btf_data, btf_data_size)) {
>   		err = -EFAULT;
>   		goto errout;
>   	}
>   
> -	env->btf = btf;
> -
> -	err = btf_parse_hdr(env);
> -	if (err)
> -		goto errout;
> -
>   	err = btf_parse_str_sec(env);
>   	if (err)
>   		goto errout;
> @@ -2016,16 +2123,14 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
>   	if (err)
>   		goto errout;
>   
> -	if (!err && log->level && bpf_verifier_log_full(log)) {
> +	if (log->level && bpf_verifier_log_full(log)) {
>   		err = -ENOSPC;
>   		goto errout;
>   	}
>   
> -	if (!err) {
> -		btf_verifier_env_free(env);
> -		refcount_set(&btf->refcnt, 1);
> -		return btf;
> -	}
> +	btf_verifier_env_free(env);
> +	refcount_set(&btf->refcnt, 1);
> +	return btf;
>   
>   errout:
>   	btf_verifier_env_free(env);
> 

Re: [PATCH bpf-next 3/7] bpf: btf: Check array->index_type

[Yonghong Song]

On 5/18/18 5:16 PM, Martin KaFai Lau wrote:
> Instead of ingoring the array->index_type field.  Enforce that
> it must be an unsigned BTF_KIND_INT.
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
>   kernel/bpf/btf.c | 83 ++++++++++++++++++++++++++++++++++++++++----------------
>   1 file changed, 59 insertions(+), 24 deletions(-)
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 536e5981ad8c..b4e48dae2240 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -444,6 +444,28 @@ static const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id)
>   	return btf->types[type_id];
>   }
>   
> +/*
> + * Regular int is not a bit field and it must be either
> + * u8/u16/u32/u64.
> + */
> +static bool btf_type_int_is_regular(const struct btf_type *t)
> +{
> +	u16 nr_bits, nr_bytes;
> +	u32 int_data;
> +
> +	int_data = btf_type_int(t);
> +	nr_bits = BTF_INT_BITS(int_data);
> +	nr_bytes = BITS_ROUNDUP_BYTES(nr_bits);
> +	if (BITS_PER_BYTE_MASKED(nr_bits) ||
> +	    BTF_INT_OFFSET(int_data) ||
> +	    (nr_bytes != sizeof(u8) && nr_bytes != sizeof(u16) &&
> +	     nr_bytes != sizeof(u32) && nr_bytes != sizeof(u64))) {
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>   __printf(2, 3) static void __btf_verifier_log(struct bpf_verifier_log *log,
>   					      const char *fmt, ...)
>   {
> @@ -1309,14 +1331,16 @@ static s32 btf_array_check_meta(struct btf_verifier_env *env,
>   		return -EINVAL;
>   	}
>   
> -	/* We are a little forgiving on array->index_type since
> -	 * the kernel is not using it.
> -	 */
> -	/* Array elem cannot be in type void,
> -	 * so !array->type is not allowed.
> +	/* Array elem type and index type cannot be in type void,
> +	 * so !array->type and !array->index_type are not allowed.
>   	 */
>   	if (!array->type || BTF_TYPE_PARENT(array->type)) {
> -		btf_verifier_log_type(env, t, "Invalid type_id");
> +		btf_verifier_log_type(env, t, "Invalid elem");
> +		return -EINVAL;
> +	}
> +
> +	if (!array->index_type || BTF_TYPE_PARENT(array->index_type)) {
> +		btf_verifier_log_type(env, t, "Invalid index");
>   		return -EINVAL;
>   	}
>   
> @@ -1329,11 +1353,35 @@ static int btf_array_resolve(struct btf_verifier_env *env,
>   			     const struct resolve_vertex *v)
>   {
>   	const struct btf_array *array = btf_type_array(v->t);
> -	const struct btf_type *elem_type;
> -	u32 elem_type_id = array->type;
> +	const struct btf_type *elem_type, *index_type;
> +	u32 elem_type_id, index_type_id;
>   	struct btf *btf = env->btf;
>   	u32 elem_size;
>   
> +	/* 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)) {
> +		btf_verifier_log_type(env, v->t, "Invalid index");
> +		return -EINVAL;
> +	}
> +
> +	if (!env_type_is_resolve_sink(env, index_type) &&
> +	    !env_type_is_resolved(env, index_type_id))
> +		return env_stack_push(env, index_type, index_type_id);
> +
> +	index_type = btf_type_id_size(btf, &index_type_id, NULL);
> +	if (!index_type || !btf_type_is_int(index_type) ||
> +	    /* bit field int is not allowed */
> +	    !btf_type_int_is_regular(index_type) ||
> +	    /* unsigned only */
> +	    BTF_INT_ENCODING(btf_type_int(index_type))) {
> +		btf_verifier_log_type(env, v->t, "Invalid index");
> +		return -EINVAL;
> +	}

Currently, in uapi/linux/btf.h, we have
/* Attributes stored in the BTF_INT_ENCODING */
#define BTF_INT_SIGNED  0x1
#define BTF_INT_CHAR    0x2
#define BTF_INT_BOOL    0x4
#define BTF_INT_VARARGS 0x8

The BPF_INT_ENCODING value 0 stands for UNSIGNED.
Do we want to explicitly document this in uapi/linux/bpf.h?

> +
> +	/* 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)) {
>   		btf_verifier_log_type(env, v->t,
> @@ -1351,22 +1399,9 @@ static int btf_array_resolve(struct btf_verifier_env *env,
>   		return -EINVAL;
>   	}
>   
> -	if (btf_type_is_int(elem_type)) {
> -		int int_type_data = btf_type_int(elem_type);
> -		u16 nr_bits = BTF_INT_BITS(int_type_data);
> -		u16 nr_bytes = BITS_ROUNDUP_BYTES(nr_bits);
> -
> -		/* Put more restriction on array of int.  The int cannot
> -		 * be a bit field and it must be either u8/u16/u32/u64.
> -		 */
> -		if (BITS_PER_BYTE_MASKED(nr_bits) ||
> -		    BTF_INT_OFFSET(int_type_data) ||
> -		    (nr_bytes != sizeof(u8) && nr_bytes != sizeof(u16) &&
> -		     nr_bytes != sizeof(u32) && nr_bytes != sizeof(u64))) {
> -			btf_verifier_log_type(env, v->t,
> -					      "Invalid array of int");
> -			return -EINVAL;
> -		}
> +	if (btf_type_is_int(elem_type) && !btf_type_int_is_regular(elem_type)) {
> +		btf_verifier_log_type(env, v->t, "Invalid array of int");
> +		return -EINVAL;
>   	}
>   
>   	if (array->nelems && elem_size > U32_MAX / array->nelems) {
> 

Re: [PATCH bpf-next 4/7] bpf: btf: Remove unused bits from uapi/linux/btf.h

[Yonghong Song]

On 5/18/18 5:16 PM, Martin KaFai Lau wrote:
> This patch does the followings:
> 1. Limit BTF_MAX_TYPES and BTF_MAX_NAME_OFFSET to 64k.  We can
>     raise it later.
> 
> 2. Remove the BTF_TYPE_PARENT and BTF_STR_TBL_ELF_ID.  They are
>     currently encoded at the highest bit of a u32.
>     It is because the current use case does not require supporting
>     parent type (i.e type_id referring to a type in another BTF file).
>     It also does not support referring to a string in ELF.
> 
>     The BTF_TYPE_PARENT and BTF_STR_TBL_ELF_ID checks are replaced
>     by BTF_TYPE_ID_CHECK and BTF_STR_OFFSET_CHECK which are
>     defined in btf.c instead of uapi/linux/btf.h.
> 
> 3. Limit the BTF_INFO_KIND from 5 bits to 4 bits which is enough.
>     There is unused bits headroom if we ever needed it later.
> 
> 4. The root bit in BTF_INFO is also removed because it is not
>     used in the current use case.
> 
> The above can be added back later because the verifier
> ensures the unused bits are zeros.
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>

Acked-by: Yonghong Song <yhs@fb.com>

Re: [PATCH bpf-next 5/7] bpf: btf: Rename btf_key_id and btf_value_id in bpf_map_info

[Yonghong Song]

On 5/18/18 5:16 PM, Martin KaFai Lau wrote:
> In "struct bpf_map_info", the name "btf_id", "btf_key_id" and "btf_value_id"
> could cause confusion because the "id" of "btf_id" means the BPF obj id
> given to the BTF object while
> "btf_key_id" and "btf_value_id" means the BTF type id within
> that BTF object.
> 
> To make it clear, btf_key_id and btf_value_id are
> renamed to btf_key_type_id and btf_value_type_id.
> 
> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>

Re: [PATCH bpf-next 2/7] bpf: btf: Change how section is supported in btf_header

[Martin KaFai Lau]

On Mon, May 21, 2018 at 01:15:24PM -0700, Yonghong Song wrote:
> 
> 
> On 5/18/18 5:16 PM, Martin KaFai Lau wrote:
> > There are currently unused section descriptions in the btf_header.  Those
> > sections are here to support future BTF use cases.  For example, the
> > func section (func_off) is to support function signature (e.g. the BPF
> > prog function signature).
> > 
> > Instead of spelling out all potential sections up-front in the btf_header.
> > This patch makes changes to btf_header such that extending it (e.g. adding
> > a section) is possible later.  The unused ones can be removed for now and
> > they can be added back later.
> > 
> > This patch:
> > 1. adds a hdr_len to the btf_header.  It will allow adding
> > sections (and other info like parent_label and parent_name)
> > later.  The check is similar to the existing bpf_attr.
> > If a user passes in a longer hdr_len, the kernel
> > ensures the extra tailing bytes are 0.
> > 
> > 2. allows the section order in the BTF object to be
> > different from its sec_off order in btf_header.
> > 
> > 3. each sec_off is followed by a sec_len.  It must not have gap or
> > overlapping among sections.
> > 
> > The string section is ensured to be at the end due to the 4 bytes
> > alignment requirement of the type section.
> > 
> > The above changes will allow enough flexibility to
> > add new sections (and other info) to the btf_header later.
> > 
> > This patch also removes an unnecessary !err check
> > at the end of btf_parse().
> > 
> > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> > ---
> >   include/uapi/linux/btf.h |   8 +-
> >   kernel/bpf/btf.c         | 207 +++++++++++++++++++++++++++++++++++------------
> >   2 files changed, 158 insertions(+), 57 deletions(-)
> > 
> > diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
> > index bcb56ee47014..4fa479741a02 100644
> > --- a/include/uapi/linux/btf.h
> > +++ b/include/uapi/linux/btf.h
> > @@ -12,15 +12,11 @@ struct btf_header {
> >   	__u16	magic;
> >   	__u8	version;
> >   	__u8	flags;
> > -
> > -	__u32	parent_label;
> > -	__u32	parent_name;
> > +	__u32	hdr_len;
> >   	/* All offsets are in bytes relative to the end of this header */
> > -	__u32	label_off;	/* offset of label section	*/
> > -	__u32	object_off;	/* offset of data object section*/
> > -	__u32	func_off;	/* offset of function section	*/
> >   	__u32	type_off;	/* offset of type section	*/
> > +	__u32	type_len;	/* length of type section	*/
> >   	__u32	str_off;	/* offset of string section	*/
> >   	__u32	str_len;	/* length of string section	*/
> >   };
> > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > index ded10ab47b8a..536e5981ad8c 100644
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -12,6 +12,7 @@
> >   #include <linux/uaccess.h>
> >   #include <linux/kernel.h>
> >   #include <linux/idr.h>
> > +#include <linux/sort.h>
> >   #include <linux/bpf_verifier.h>
> >   #include <linux/btf.h>
> > @@ -184,15 +185,13 @@ static DEFINE_IDR(btf_idr);
> >   static DEFINE_SPINLOCK(btf_idr_lock);
> >   struct btf {
> > -	union {
> > -		struct btf_header *hdr;
> > -		void *data;
> > -	};
> > +	void *data;
> >   	struct btf_type **types;
> >   	u32 *resolved_ids;
> >   	u32 *resolved_sizes;
> >   	const char *strings;
> >   	void *nohdr_data;
> > +	struct btf_header hdr;
> >   	u32 nr_types;
> >   	u32 types_size;
> >   	u32 data_size;
> > @@ -227,6 +226,12 @@ enum resolve_mode {
> >   };
> >   #define MAX_RESOLVE_DEPTH 32
> > +#define NR_SECS 2
> 
> Not sure whether it is necessary to define NR_SECS 2 here or not.
> See below.
> 
> > +
> > +struct btf_sec_info {
> > +	u32 off;
> > +	u32 len;
> > +};
> >   struct btf_verifier_env {
> >   	struct btf *btf;
> > @@ -418,14 +423,14 @@ static const struct btf_kind_operations *btf_type_ops(const struct btf_type *t)
> >   static bool btf_name_offset_valid(const struct btf *btf, u32 offset)
> >   {
> >   	return !BTF_STR_TBL_ELF_ID(offset) &&
> > -		BTF_STR_OFFSET(offset) < btf->hdr->str_len;
> > +		BTF_STR_OFFSET(offset) < btf->hdr.str_len;
> >   }
> >   static const char *btf_name_by_offset(const struct btf *btf, u32 offset)
> >   {
> >   	if (!BTF_STR_OFFSET(offset))
> >   		return "(anon)";
> > -	else if (BTF_STR_OFFSET(offset) < btf->hdr->str_len)
> > +	else if (BTF_STR_OFFSET(offset) < btf->hdr.str_len)
> >   		return &btf->strings[BTF_STR_OFFSET(offset)];
> >   	else
> >   		return "(invalid-name-offset)";
> > @@ -536,7 +541,8 @@ static void btf_verifier_log_member(struct btf_verifier_env *env,
> >   	__btf_verifier_log(log, "\n");
> >   }
> > -static void btf_verifier_log_hdr(struct btf_verifier_env *env)
> > +static void btf_verifier_log_hdr(struct btf_verifier_env *env,
> > +				 u32 btf_data_size)
> >   {
> >   	struct bpf_verifier_log *log = &env->log;
> >   	const struct btf *btf = env->btf;
> > @@ -545,19 +551,16 @@ static void btf_verifier_log_hdr(struct btf_verifier_env *env)
> >   	if (!bpf_verifier_log_needed(log))
> >   		return;
> > -	hdr = btf->hdr;
> > +	hdr = &btf->hdr;
> >   	__btf_verifier_log(log, "magic: 0x%x\n", hdr->magic);
> >   	__btf_verifier_log(log, "version: %u\n", hdr->version);
> >   	__btf_verifier_log(log, "flags: 0x%x\n", hdr->flags);
> > -	__btf_verifier_log(log, "parent_label: %u\n", hdr->parent_label);
> > -	__btf_verifier_log(log, "parent_name: %u\n", hdr->parent_name);
> > -	__btf_verifier_log(log, "label_off: %u\n", hdr->label_off);
> > -	__btf_verifier_log(log, "object_off: %u\n", hdr->object_off);
> > -	__btf_verifier_log(log, "func_off: %u\n", hdr->func_off);
> > +	__btf_verifier_log(log, "hdr_len: %u\n", hdr->hdr_len);
> >   	__btf_verifier_log(log, "type_off: %u\n", hdr->type_off);
> > +	__btf_verifier_log(log, "type_len: %u\n", hdr->type_len);
> >   	__btf_verifier_log(log, "str_off: %u\n", hdr->str_off);
> >   	__btf_verifier_log(log, "str_len: %u\n", hdr->str_len);
> > -	__btf_verifier_log(log, "btf_total_size: %u\n", btf->data_size);
> > +	__btf_verifier_log(log, "btf_total_size: %u\n", btf_data_size);
> >   }
> >   static int btf_add_type(struct btf_verifier_env *env, struct btf_type *t)
> > @@ -1754,9 +1757,9 @@ static int btf_check_all_metas(struct btf_verifier_env *env)
> >   	struct btf_header *hdr;
> >   	void *cur, *end;
> > -	hdr = btf->hdr;
> > +	hdr = &btf->hdr;
> >   	cur = btf->nohdr_data + hdr->type_off;
> > -	end = btf->nohdr_data + hdr->str_off;
> > +	end = btf->nohdr_data + hdr->type_len;
> >   	env->log_type_id = 1;
> >   	while (cur < end) {
> > @@ -1866,8 +1869,20 @@ static int btf_check_all_types(struct btf_verifier_env *env)
> >   static int btf_parse_type_sec(struct btf_verifier_env *env)
> >   {
> > +	const struct btf_header *hdr = &env->btf->hdr;
> >   	int err;
> > +	/* Type section must align to 4 bytes */
> > +	if (hdr->type_off & (sizeof(u32) - 1)) {
> > +		btf_verifier_log(env, "Unaligned type_off");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (!hdr->type_len) {
> > +		btf_verifier_log(env, "No type found");
> > +		return -EINVAL;
> > +	}
> > +
> >   	err = btf_check_all_metas(env);
> >   	if (err)
> >   		return err;
> > @@ -1881,10 +1896,15 @@ static int btf_parse_str_sec(struct btf_verifier_env *env)
> >   	struct btf *btf = env->btf;
> >   	const char *start, *end;
> > -	hdr = btf->hdr;
> > +	hdr = &btf->hdr;
> >   	start = btf->nohdr_data + hdr->str_off;
> >   	end = start + hdr->str_len;
> > +	if (end != btf->data + btf->data_size) {
> > +		btf_verifier_log(env, "String section is not at the end");
> > +		return -EINVAL;
> > +	}
> > +
> >   	if (!hdr->str_len || hdr->str_len - 1 > BTF_MAX_NAME_OFFSET ||
> >   	    start[0] || end[-1]) {
> >   		btf_verifier_log(env, "Invalid string section");
> > @@ -1896,20 +1916,119 @@ static int btf_parse_str_sec(struct btf_verifier_env *env)
> >   	return 0;
> >   }
> > -static int btf_parse_hdr(struct btf_verifier_env *env)
> > +static const size_t btf_sec_info_offset[] = {
> > +	offsetof(struct btf_header, type_off),
> > +	offsetof(struct btf_header, str_off),
> > +};
> 
> Maybe we can define NR_SECS is
> ARRAY_SIZE(btf_sec_info_offset)/sizeof(size_t)?
> 
> > +
> > +static int btf_sec_info_cmp(const void *a, const void *b)
> >   {
> > +	const struct btf_sec_info *x = a;
> > +	const struct btf_sec_info *y = b;
> > +
> > +	return (int)(x->off - y->off) ? : (int)(x->len - y->len);
> > +}
> > +
> > +static int btf_check_sec_info(struct btf_verifier_env *env,
> > +			      u32 btf_data_size)
> > +{
> > +	struct btf_sec_info secs[NR_SECS];
> > +	u32 total, expected_total, i;
> >   	const struct btf_header *hdr;
> > -	struct btf *btf = env->btf;
> > -	u32 meta_left;
> > +	const struct btf *btf;
> > +
> > +	BUILD_BUG_ON(ARRAY_SIZE(btf_sec_info_offset) != NR_SECS);
> 
> If we define NR_SECS depending on the size of btf_sec_info_offset, this
> BUILD_BUG_ON is not needed.
Agree, BUILD_BUG_ON() can be avoided.  Will change.

> 
> > +
> > +	btf = env->btf;
> > +	hdr = &btf->hdr;
> > +
> > +	/* Populate the secs from hdr */
> > +	for (i = 0; i < NR_SECS; i++)
> > +		secs[i] = *(struct btf_sec_info *)((void *)hdr +
> > +						   btf_sec_info_offset[i]);
> > +
> > +	sort(secs, NR_SECS, sizeof(struct btf_sec_info),
> > +	     btf_sec_info_cmp, NULL);
> > +
> > +	/* Check for gaps and overlap among sections */
> > +	total = 0;
> > +	expected_total = btf_data_size - hdr->hdr_len;
> > +	for (i = 0; i < NR_SECS; i++) {
> > +		if (expected_total < secs[i].off) {
> > +			btf_verifier_log(env, "Invalid section offset");
> > +			return -EINVAL;
> > +		}
> > +		if (total < secs[i].off) {
> > +			/* gap */
> > +			btf_verifier_log(env, "Unsupported section found");
> > +			return -EINVAL;
> > +		}
> > +		if (total > secs[i].off) {
> > +			btf_verifier_log(env, "Section overlap found");
> > +			return -EINVAL;
> > +		}
> > +		if (expected_total - total < secs[i].len) {
> > +			btf_verifier_log(env,
> > +					 "Total section length too long");
> > +			return -EINVAL;
> > +		}
> > +		total += secs[i].len;
> > +	}
> > +
> > +	/* There is data other than hdr and known sections */
> > +	if (expected_total != total) {
> > +		btf_verifier_log(env, "Unsupported section found");
> > +		return -EINVAL;
> > +	}
> 
> Does this patch try to address compatibility issue? That is, the old btf
> format with 2 sections should still work with future kernel with more
> than 2 sections? The above comparision seems suggesting that newer kernel
> will not support non-compatible number of sections?
Yes, it should.  When an older btf passed in, the newer sec_off
and sec_len in btf->hdr will be zeros.  They will still pass the
for loop.

> 
> > +
> > +	return 0;
> > +}
> > +
> > +static int btf_parse_hdr(struct btf_verifier_env *env, void __user *btf_data,
> > +			 u32 btf_data_size)
> > +{
> > +	const struct btf_header *hdr;
> > +	u32 hdr_len, hdr_copy;
> > +	struct btf_min_header {
> > +		u16	magic;
> > +		u8	version;
> > +		u8	flags;
> > +		u32	hdr_len;
> > +	} __user *min_hdr;
> 
> This is the partial structure with the first four fields
> for struct btf_header.
> It is okay, but I feel a little bit duplication here.
> 
> Looks like only sizeof(*min_hdr) and min_hdr->hdr_len is used.
> Maybe we can just cast bpf_data to bpf_header and
> sizeof(*min_hdr) being offsetof(bpf_data, type_off), and
> min_hdr->hdr_len being bpf_data->hdr_len.
> 
> Do this work?
offsetof() was what I had.  I found a few lines of offsetof() become
so long that is actually hard to read.  Also, I think spelling out the
'btf_min_header' name here is helpful, so I did not leave it as
anon struct.

Agree that there is duplication but it will never be changed once we
release it to uapi.  I can make some comments here instead.

> 
> > +	struct btf *btf;
> > +	int err;
> > +
> > +	btf = env->btf;
> > +	min_hdr = btf_data;
> > +
> > +	if (btf_data_size < sizeof(*min_hdr)) {
> > +		btf_verifier_log(env, "hdr_len not found");
> > +		return -EINVAL;
> > +	}
> > -	if (btf->data_size < sizeof(*hdr)) {
> > +	if (get_user(hdr_len, &min_hdr->hdr_len))
> > +		return -EFAULT;
> > +
> > +	if (btf_data_size < hdr_len) {
> >   		btf_verifier_log(env, "btf_header not found");
> >   		return -EINVAL;
> >   	}
> > -	btf_verifier_log_hdr(env);
> > +	err = bpf_check_uarg_tail_zero(btf_data, sizeof(btf->hdr), hdr_len);
> > +	if (err) {
> > +		if (err == -E2BIG)
> > +			btf_verifier_log(env, "Unsupported btf_header");
> > +		return err;
> > +	}
> > +
> > +	hdr_copy = min_t(u32, hdr_len, sizeof(btf->hdr));
> > +	if (copy_from_user(&btf->hdr, btf_data, hdr_copy))
> > +		return -EFAULT;
> > +
> > +	hdr = &btf->hdr;
> > +
> > +	btf_verifier_log_hdr(env, btf_data_size);
> > -	hdr = btf->hdr;
> >   	if (hdr->magic != BTF_MAGIC) {
> >   		btf_verifier_log(env, "Invalid magic");
> >   		return -EINVAL;
> > @@ -1925,26 +2044,14 @@ static int btf_parse_hdr(struct btf_verifier_env *env)
> >   		return -ENOTSUPP;
> >   	}
> > -	meta_left = btf->data_size - sizeof(*hdr);
> > -	if (!meta_left) {
> > +	if (btf_data_size == hdr->hdr_len) {
> >   		btf_verifier_log(env, "No data");
> >   		return -EINVAL;
> >   	}
> > -	if (meta_left < hdr->type_off || hdr->str_off <= hdr->type_off ||
> > -	    /* Type section must align to 4 bytes */
> > -	    hdr->type_off & (sizeof(u32) - 1)) {
> > -		btf_verifier_log(env, "Invalid type_off");
> > -		return -EINVAL;
> > -	}
> > -
> > -	if (meta_left < hdr->str_off ||
> > -	    meta_left - hdr->str_off < hdr->str_len) {
> > -		btf_verifier_log(env, "Invalid str_off or str_len");
> > -		return -EINVAL;
> > -	}
> > -
> > -	btf->nohdr_data = btf->hdr + 1;
> > +	err = btf_check_sec_info(env, btf_data_size);
> > +	if (err)
> > +		return err;
> >   	return 0;
> >   }
> > @@ -1987,6 +2094,11 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
> >   		err = -ENOMEM;
> >   		goto errout;
> >   	}
> > +	env->btf = btf;
> > +
> > +	err = btf_parse_hdr(env, btf_data, btf_data_size);
> > +	if (err)
> > +		goto errout;
> >   	data = kvmalloc(btf_data_size, GFP_KERNEL | __GFP_NOWARN);
> >   	if (!data) {
> > @@ -1996,18 +2108,13 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
> >   	btf->data = data;
> >   	btf->data_size = btf_data_size;
> > +	btf->nohdr_data = btf->data + btf->hdr.hdr_len;
> >   	if (copy_from_user(data, btf_data, btf_data_size)) {
> >   		err = -EFAULT;
> >   		goto errout;
> >   	}
> > -	env->btf = btf;
> > -
> > -	err = btf_parse_hdr(env);
> > -	if (err)
> > -		goto errout;
> > -
> >   	err = btf_parse_str_sec(env);
> >   	if (err)
> >   		goto errout;
> > @@ -2016,16 +2123,14 @@ static struct btf *btf_parse(void __user *btf_data, u32 btf_data_size,
> >   	if (err)
> >   		goto errout;
> > -	if (!err && log->level && bpf_verifier_log_full(log)) {
> > +	if (log->level && bpf_verifier_log_full(log)) {
> >   		err = -ENOSPC;
> >   		goto errout;
> >   	}
> > -	if (!err) {
> > -		btf_verifier_env_free(env);
> > -		refcount_set(&btf->refcnt, 1);
> > -		return btf;
> > -	}
> > +	btf_verifier_env_free(env);
> > +	refcount_set(&btf->refcnt, 1);
> > +	return btf;
> >   errout:
> >   	btf_verifier_env_free(env);
> > 

Re: [PATCH bpf-next 3/7] bpf: btf: Check array->index_type

[Yonghong Song]

On 5/18/18 5:16 PM, Martin KaFai Lau wrote:
> Instead of ingoring the array->index_type field.  Enforce that
> it must be an unsigned BTF_KIND_INT.
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
>   kernel/bpf/btf.c | 83 ++++++++++++++++++++++++++++++++++++++++----------------
>   1 file changed, 59 insertions(+), 24 deletions(-)
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 536e5981ad8c..b4e48dae2240 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -444,6 +444,28 @@ static const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id)
>   	return btf->types[type_id];
>   }
>   
> +/*
> + * Regular int is not a bit field and it must be either
> + * u8/u16/u32/u64.
> + */
> +static bool btf_type_int_is_regular(const struct btf_type *t)
> +{
> +	u16 nr_bits, nr_bytes;
> +	u32 int_data;
> +
> +	int_data = btf_type_int(t);
> +	nr_bits = BTF_INT_BITS(int_data);
> +	nr_bytes = BITS_ROUNDUP_BYTES(nr_bits);
> +	if (BITS_PER_BYTE_MASKED(nr_bits) ||
> +	    BTF_INT_OFFSET(int_data) ||
> +	    (nr_bytes != sizeof(u8) && nr_bytes != sizeof(u16) &&
> +	     nr_bytes != sizeof(u32) && nr_bytes != sizeof(u64))) {
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>   __printf(2, 3) static void __btf_verifier_log(struct bpf_verifier_log *log,
>   					      const char *fmt, ...)
>   {
> @@ -1309,14 +1331,16 @@ static s32 btf_array_check_meta(struct btf_verifier_env *env,
>   		return -EINVAL;
>   	}
>   
> -	/* We are a little forgiving on array->index_type since
> -	 * the kernel is not using it.
> -	 */
> -	/* Array elem cannot be in type void,
> -	 * so !array->type is not allowed.
> +	/* Array elem type and index type cannot be in type void,
> +	 * so !array->type and !array->index_type are not allowed.
>   	 */
>   	if (!array->type || BTF_TYPE_PARENT(array->type)) {
> -		btf_verifier_log_type(env, t, "Invalid type_id");
> +		btf_verifier_log_type(env, t, "Invalid elem");
> +		return -EINVAL;
> +	}
> +
> +	if (!array->index_type || BTF_TYPE_PARENT(array->index_type)) {
> +		btf_verifier_log_type(env, t, "Invalid index");
>   		return -EINVAL;
>   	}
>   
> @@ -1329,11 +1353,35 @@ static int btf_array_resolve(struct btf_verifier_env *env,
>   			     const struct resolve_vertex *v)
>   {
>   	const struct btf_array *array = btf_type_array(v->t);
> -	const struct btf_type *elem_type;
> -	u32 elem_type_id = array->type;
> +	const struct btf_type *elem_type, *index_type;
> +	u32 elem_type_id, index_type_id;
>   	struct btf *btf = env->btf;
>   	u32 elem_size;
>   
> +	/* 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)) {
> +		btf_verifier_log_type(env, v->t, "Invalid index");
> +		return -EINVAL;
> +	}
> +
> +	if (!env_type_is_resolve_sink(env, index_type) &&
> +	    !env_type_is_resolved(env, index_type_id))
> +		return env_stack_push(env, index_type, index_type_id);
> +
> +	index_type = btf_type_id_size(btf, &index_type_id, NULL);
> +	if (!index_type || !btf_type_is_int(index_type) ||
> +	    /* bit field int is not allowed */
> +	    !btf_type_int_is_regular(index_type) ||
> +	    /* unsigned only */
> +	    BTF_INT_ENCODING(btf_type_int(index_type))) {

Could you explain why you only support array index type to be
unsigned? A lot of test cases  in Patch #7 are amended with unsigned 
types. In C, signed integers can surely be index, e.g., a[-1].

> +		btf_verifier_log_type(env, v->t, "Invalid index");
> +		return -EINVAL;
> +	}
> +
> +	/* 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)) {
>   		btf_verifier_log_type(env, v->t,
> @@ -1351,22 +1399,9 @@ static int btf_array_resolve(struct btf_verifier_env *env,
>   		return -EINVAL;
>   	}
>   
> -	if (btf_type_is_int(elem_type)) {
> -		int int_type_data = btf_type_int(elem_type);
> -		u16 nr_bits = BTF_INT_BITS(int_type_data);
> -		u16 nr_bytes = BITS_ROUNDUP_BYTES(nr_bits);
> -
> -		/* Put more restriction on array of int.  The int cannot
> -		 * be a bit field and it must be either u8/u16/u32/u64.
> -		 */
> -		if (BITS_PER_BYTE_MASKED(nr_bits) ||
> -		    BTF_INT_OFFSET(int_type_data) ||
> -		    (nr_bytes != sizeof(u8) && nr_bytes != sizeof(u16) &&
> -		     nr_bytes != sizeof(u32) && nr_bytes != sizeof(u64))) {
> -			btf_verifier_log_type(env, v->t,
> -					      "Invalid array of int");
> -			return -EINVAL;
> -		}
> +	if (btf_type_is_int(elem_type) && !btf_type_int_is_regular(elem_type)) {
> +		btf_verifier_log_type(env, v->t, "Invalid array of int");
> +		return -EINVAL;
>   	}
>   
>   	if (array->nelems && elem_size > U32_MAX / array->nelems) {
> 

Re: [PATCH bpf-next 3/7] bpf: btf: Check array->index_type

[Martin KaFai Lau]

On Mon, May 21, 2018 at 09:41:11PM -0700, Yonghong Song wrote:
> 
> 
> On 5/18/18 5:16 PM, Martin KaFai Lau wrote:
> > Instead of ingoring the array->index_type field.  Enforce that
> > it must be an unsigned BTF_KIND_INT.
> > 
> > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> > ---
> >   kernel/bpf/btf.c | 83 ++++++++++++++++++++++++++++++++++++++++----------------
> >   1 file changed, 59 insertions(+), 24 deletions(-)
> > 
> > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > index 536e5981ad8c..b4e48dae2240 100644
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -444,6 +444,28 @@ static const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id)
> >   	return btf->types[type_id];
> >   }
> > +/*
> > + * Regular int is not a bit field and it must be either
> > + * u8/u16/u32/u64.
> > + */
> > +static bool btf_type_int_is_regular(const struct btf_type *t)
> > +{
> > +	u16 nr_bits, nr_bytes;
> > +	u32 int_data;
> > +
> > +	int_data = btf_type_int(t);
> > +	nr_bits = BTF_INT_BITS(int_data);
> > +	nr_bytes = BITS_ROUNDUP_BYTES(nr_bits);
> > +	if (BITS_PER_BYTE_MASKED(nr_bits) ||
> > +	    BTF_INT_OFFSET(int_data) ||
> > +	    (nr_bytes != sizeof(u8) && nr_bytes != sizeof(u16) &&
> > +	     nr_bytes != sizeof(u32) && nr_bytes != sizeof(u64))) {
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> >   __printf(2, 3) static void __btf_verifier_log(struct bpf_verifier_log *log,
> >   					      const char *fmt, ...)
> >   {
> > @@ -1309,14 +1331,16 @@ static s32 btf_array_check_meta(struct btf_verifier_env *env,
> >   		return -EINVAL;
> >   	}
> > -	/* We are a little forgiving on array->index_type since
> > -	 * the kernel is not using it.
> > -	 */
> > -	/* Array elem cannot be in type void,
> > -	 * so !array->type is not allowed.
> > +	/* Array elem type and index type cannot be in type void,
> > +	 * so !array->type and !array->index_type are not allowed.
> >   	 */
> >   	if (!array->type || BTF_TYPE_PARENT(array->type)) {
> > -		btf_verifier_log_type(env, t, "Invalid type_id");
> > +		btf_verifier_log_type(env, t, "Invalid elem");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (!array->index_type || BTF_TYPE_PARENT(array->index_type)) {
> > +		btf_verifier_log_type(env, t, "Invalid index");
> >   		return -EINVAL;
> >   	}
> > @@ -1329,11 +1353,35 @@ static int btf_array_resolve(struct btf_verifier_env *env,
> >   			     const struct resolve_vertex *v)
> >   {
> >   	const struct btf_array *array = btf_type_array(v->t);
> > -	const struct btf_type *elem_type;
> > -	u32 elem_type_id = array->type;
> > +	const struct btf_type *elem_type, *index_type;
> > +	u32 elem_type_id, index_type_id;
> >   	struct btf *btf = env->btf;
> >   	u32 elem_size;
> > +	/* 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)) {
> > +		btf_verifier_log_type(env, v->t, "Invalid index");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (!env_type_is_resolve_sink(env, index_type) &&
> > +	    !env_type_is_resolved(env, index_type_id))
> > +		return env_stack_push(env, index_type, index_type_id);
> > +
> > +	index_type = btf_type_id_size(btf, &index_type_id, NULL);
> > +	if (!index_type || !btf_type_is_int(index_type) ||
> > +	    /* bit field int is not allowed */
> > +	    !btf_type_int_is_regular(index_type) ||
> > +	    /* unsigned only */
> > +	    BTF_INT_ENCODING(btf_type_int(index_type))) {
> 
> Could you explain why you only support array index type to be
> unsigned? A lot of test cases  in Patch #7 are amended with unsigned types.
> In C, signed integers can surely be index, e.g., a[-1].
I will allow the signed in v2.

> 
> > +		btf_verifier_log_type(env, v->t, "Invalid index");
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* 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)) {
> >   		btf_verifier_log_type(env, v->t,
> > @@ -1351,22 +1399,9 @@ static int btf_array_resolve(struct btf_verifier_env *env,
> >   		return -EINVAL;
> >   	}
> > -	if (btf_type_is_int(elem_type)) {
> > -		int int_type_data = btf_type_int(elem_type);
> > -		u16 nr_bits = BTF_INT_BITS(int_type_data);
> > -		u16 nr_bytes = BITS_ROUNDUP_BYTES(nr_bits);
> > -
> > -		/* Put more restriction on array of int.  The int cannot
> > -		 * be a bit field and it must be either u8/u16/u32/u64.
> > -		 */
> > -		if (BITS_PER_BYTE_MASKED(nr_bits) ||
> > -		    BTF_INT_OFFSET(int_type_data) ||
> > -		    (nr_bytes != sizeof(u8) && nr_bytes != sizeof(u16) &&
> > -		     nr_bytes != sizeof(u32) && nr_bytes != sizeof(u64))) {
> > -			btf_verifier_log_type(env, v->t,
> > -					      "Invalid array of int");
> > -			return -EINVAL;
> > -		}
> > +	if (btf_type_is_int(elem_type) && !btf_type_int_is_regular(elem_type)) {
> > +		btf_verifier_log_type(env, v->t, "Invalid array of int");
> > +		return -EINVAL;
> >   	}
> >   	if (array->nelems && elem_size > U32_MAX / array->nelems) {
> > 

Re: [PATCH bpf-next 3/7] bpf: btf: Check array->index_type

[Martin KaFai Lau]

On Mon, May 21, 2018 at 02:04:51PM -0700, Yonghong Song wrote:
> 
> 
> On 5/18/18 5:16 PM, Martin KaFai Lau wrote:
> > Instead of ingoring the array->index_type field.  Enforce that
> > it must be an unsigned BTF_KIND_INT.
> > 
> > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> > ---
> >   kernel/bpf/btf.c | 83 ++++++++++++++++++++++++++++++++++++++++----------------
> >   1 file changed, 59 insertions(+), 24 deletions(-)
> > 
> > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> > index 536e5981ad8c..b4e48dae2240 100644
> > --- a/kernel/bpf/btf.c
> > +++ b/kernel/bpf/btf.c
> > @@ -444,6 +444,28 @@ static const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id)
> >   	return btf->types[type_id];
> >   }
> > +/*
> > + * Regular int is not a bit field and it must be either
> > + * u8/u16/u32/u64.
> > + */
> > +static bool btf_type_int_is_regular(const struct btf_type *t)
> > +{
> > +	u16 nr_bits, nr_bytes;
> > +	u32 int_data;
> > +
> > +	int_data = btf_type_int(t);
> > +	nr_bits = BTF_INT_BITS(int_data);
> > +	nr_bytes = BITS_ROUNDUP_BYTES(nr_bits);
> > +	if (BITS_PER_BYTE_MASKED(nr_bits) ||
> > +	    BTF_INT_OFFSET(int_data) ||
> > +	    (nr_bytes != sizeof(u8) && nr_bytes != sizeof(u16) &&
> > +	     nr_bytes != sizeof(u32) && nr_bytes != sizeof(u64))) {
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> >   __printf(2, 3) static void __btf_verifier_log(struct bpf_verifier_log *log,
> >   					      const char *fmt, ...)
> >   {
> > @@ -1309,14 +1331,16 @@ static s32 btf_array_check_meta(struct btf_verifier_env *env,
> >   		return -EINVAL;
> >   	}
> > -	/* We are a little forgiving on array->index_type since
> > -	 * the kernel is not using it.
> > -	 */
> > -	/* Array elem cannot be in type void,
> > -	 * so !array->type is not allowed.
> > +	/* Array elem type and index type cannot be in type void,
> > +	 * so !array->type and !array->index_type are not allowed.
> >   	 */
> >   	if (!array->type || BTF_TYPE_PARENT(array->type)) {
> > -		btf_verifier_log_type(env, t, "Invalid type_id");
> > +		btf_verifier_log_type(env, t, "Invalid elem");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (!array->index_type || BTF_TYPE_PARENT(array->index_type)) {
> > +		btf_verifier_log_type(env, t, "Invalid index");
> >   		return -EINVAL;
> >   	}
> > @@ -1329,11 +1353,35 @@ static int btf_array_resolve(struct btf_verifier_env *env,
> >   			     const struct resolve_vertex *v)
> >   {
> >   	const struct btf_array *array = btf_type_array(v->t);
> > -	const struct btf_type *elem_type;
> > -	u32 elem_type_id = array->type;
> > +	const struct btf_type *elem_type, *index_type;
> > +	u32 elem_type_id, index_type_id;
> >   	struct btf *btf = env->btf;
> >   	u32 elem_size;
> > +	/* 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)) {
> > +		btf_verifier_log_type(env, v->t, "Invalid index");
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (!env_type_is_resolve_sink(env, index_type) &&
> > +	    !env_type_is_resolved(env, index_type_id))
> > +		return env_stack_push(env, index_type, index_type_id);
> > +
> > +	index_type = btf_type_id_size(btf, &index_type_id, NULL);
> > +	if (!index_type || !btf_type_is_int(index_type) ||
> > +	    /* bit field int is not allowed */
> > +	    !btf_type_int_is_regular(index_type) ||
> > +	    /* unsigned only */
> > +	    BTF_INT_ENCODING(btf_type_int(index_type))) {
> > +		btf_verifier_log_type(env, v->t, "Invalid index");
> > +		return -EINVAL;
> > +	}
> 
> Currently, in uapi/linux/btf.h, we have
> /* Attributes stored in the BTF_INT_ENCODING */
> #define BTF_INT_SIGNED  0x1
> #define BTF_INT_CHAR    0x2
> #define BTF_INT_BOOL    0x4
> #define BTF_INT_VARARGS 0x8
> 
> The BPF_INT_ENCODING value 0 stands for UNSIGNED.
It is a bit field, so getting 0 defined would be confusing.
If BTF_INT_SIGNED bit is not set, then it is not signed.

I think it will help to define them as (1 << x) to make
the bit field nature more obvious.

> Do we want to explicitly document this in uapi/linux/bpf.h?
> 
> > +
> > +	/* 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)) {
> >   		btf_verifier_log_type(env, v->t,
> > @@ -1351,22 +1399,9 @@ static int btf_array_resolve(struct btf_verifier_env *env,
> >   		return -EINVAL;
> >   	}
> > -	if (btf_type_is_int(elem_type)) {
> > -		int int_type_data = btf_type_int(elem_type);
> > -		u16 nr_bits = BTF_INT_BITS(int_type_data);
> > -		u16 nr_bytes = BITS_ROUNDUP_BYTES(nr_bits);
> > -
> > -		/* Put more restriction on array of int.  The int cannot
> > -		 * be a bit field and it must be either u8/u16/u32/u64.
> > -		 */
> > -		if (BITS_PER_BYTE_MASKED(nr_bits) ||
> > -		    BTF_INT_OFFSET(int_type_data) ||
> > -		    (nr_bytes != sizeof(u8) && nr_bytes != sizeof(u16) &&
> > -		     nr_bytes != sizeof(u32) && nr_bytes != sizeof(u64))) {
> > -			btf_verifier_log_type(env, v->t,
> > -					      "Invalid array of int");
> > -			return -EINVAL;
> > -		}
> > +	if (btf_type_is_int(elem_type) && !btf_type_int_is_regular(elem_type)) {
> > +		btf_verifier_log_type(env, v->t, "Invalid array of int");
> > +		return -EINVAL;
> >   	}
> >   	if (array->nelems && elem_size > U32_MAX / array->nelems) {
> >