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 <ka...@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);
