Currently, many BPF_MAP_CREATE failures return -EINVAL without providing any explanation to userspace.
With extended BPF syscall support, detailed error messages can now be reported via the log buffer, allowing users to understand the specific reason for a failed map creation.
Signed-off-by: Leon Hwang leon.hwang@linux.dev --- include/linux/bpf_verifier.h | 2 ++ kernel/bpf/log.c | 30 +++++++++++++++++ kernel/bpf/syscall.c | 65 ++++++++++++++++++++++++++++++------ 3 files changed, 87 insertions(+), 10 deletions(-)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 9022e4f515f9..280beca480ea 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -644,6 +644,8 @@ int bpf_prog_load_log_attr_init(struct bpf_log_attr *log_attr, struct bpf_attrs struct bpf_attrs *attrs_common); int bpf_btf_load_log_attr_init(struct bpf_log_attr *log_attr, struct bpf_attrs *attrs, struct bpf_attrs *attrs_common); +struct bpf_verifier_log *bpf_log_attr_create_vlog(struct bpf_log_attr *log_attr, + struct bpf_attrs *attrs_common); int bpf_log_attr_finalize(struct bpf_log_attr *log_attr, struct bpf_verifier_log *log);
#define BPF_MAX_SUBPROGS 256 diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c index 0dba014ca055..6586d752970f 100644 --- a/kernel/bpf/log.c +++ b/kernel/bpf/log.c @@ -912,6 +912,36 @@ int bpf_btf_load_log_attr_init(struct bpf_log_attr *log_attr, struct bpf_attrs * attrs_common); }
+struct bpf_verifier_log *bpf_log_attr_create_vlog(struct bpf_log_attr *log_attr, + struct bpf_attrs *attrs_common) +{ + const struct bpf_common_attr *common_attr = attrs_common->attr; + struct bpf_verifier_log *log; + int err; + + memset(log_attr, 0, sizeof(*log_attr)); + log_attr->log_buf = common_attr->log_buf; + log_attr->log_size = common_attr->log_size; + log_attr->log_level = common_attr->log_level; + log_attr->attrs_common = attrs_common; + + if (!log_attr->log_buf) + return NULL; + + log = kzalloc(sizeof(*log), GFP_KERNEL); + if (!log) + return ERR_PTR(-ENOMEM); + + err = bpf_vlog_init(log, log_attr->log_level, u64_to_user_ptr(log_attr->log_buf), + log_attr->log_size); + if (err) { + kfree(log); + return ERR_PTR(err); + } + + return log; +} + int bpf_log_attr_finalize(struct bpf_log_attr *log_attr, struct bpf_verifier_log *log) { u32 log_true_size, off; diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index d0440e640e40..52e1ab142da9 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -1370,7 +1370,7 @@ static bool bpf_net_capable(void)
#define BPF_MAP_CREATE_LAST_FIELD excl_prog_hash_size /* called via syscall */ -static int map_create(union bpf_attr *attr, bpfptr_t uattr) +static int __map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_verifier_log *log) { const struct bpf_map_ops *ops; struct bpf_token *token = NULL; @@ -1382,8 +1382,10 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr) int err;
err = CHECK_ATTR(BPF_MAP_CREATE); - if (err) + if (err) { + bpf_log(log, "Invalid attr.\n"); return -EINVAL; + }
/* check BPF_F_TOKEN_FD flag, remember if it's set, and then clear it * to avoid per-map type checks tripping on unknown flag @@ -1392,17 +1394,25 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr) attr->map_flags &= ~BPF_F_TOKEN_FD;
if (attr->btf_vmlinux_value_type_id) { - if (attr->map_type != BPF_MAP_TYPE_STRUCT_OPS || - attr->btf_key_type_id || attr->btf_value_type_id) + if (attr->map_type != BPF_MAP_TYPE_STRUCT_OPS) { + bpf_log(log, "btf_vmlinux_value_type_id can only be used with struct_ops maps.\n"); return -EINVAL; + } + if (attr->btf_key_type_id || attr->btf_value_type_id) { + bpf_log(log, "btf_vmlinux_value_type_id is mutually exclusive with btf_key_type_id and btf_value_type_id.\n"); + return -EINVAL; + } } else if (attr->btf_key_type_id && !attr->btf_value_type_id) { + bpf_log(log, "Invalid btf_value_type_id.\n"); return -EINVAL; }
if (attr->map_type != BPF_MAP_TYPE_BLOOM_FILTER && attr->map_type != BPF_MAP_TYPE_ARENA && - attr->map_extra != 0) + attr->map_extra != 0) { + bpf_log(log, "Invalid map_extra.\n"); return -EINVAL; + }
f_flags = bpf_get_file_flag(attr->map_flags); if (f_flags < 0) @@ -1410,13 +1420,17 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr)
if (numa_node != NUMA_NO_NODE && ((unsigned int)numa_node >= nr_node_ids || - !node_online(numa_node))) + !node_online(numa_node))) { + bpf_log(log, "Invalid numa_node.\n"); return -EINVAL; + }
/* find map type and init map: hashtable vs rbtree vs bloom vs ... */ map_type = attr->map_type; - if (map_type >= ARRAY_SIZE(bpf_map_types)) + if (map_type >= ARRAY_SIZE(bpf_map_types)) { + bpf_log(log, "Invalid map_type.\n"); return -EINVAL; + } map_type = array_index_nospec(map_type, ARRAY_SIZE(bpf_map_types)); ops = bpf_map_types[map_type]; if (!ops) @@ -1434,8 +1448,10 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr)
if (token_flag) { token = bpf_token_get_from_fd(attr->map_token_fd); - if (IS_ERR(token)) + if (IS_ERR(token)) { + bpf_log(log, "Invalid map_token_fd.\n"); return PTR_ERR(token); + }
/* if current token doesn't grant map creation permissions, * then we can't use this token, so ignore it and rely on @@ -1518,8 +1534,10 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr)
err = bpf_obj_name_cpy(map->name, attr->map_name, sizeof(attr->map_name)); - if (err < 0) + if (err < 0) { + bpf_log(log, "Invalid map_name.\n"); goto free_map; + }
preempt_disable(); map->cookie = gen_cookie_next(&bpf_map_cookie); @@ -1542,6 +1560,7 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr)
btf = btf_get_by_fd(attr->btf_fd); if (IS_ERR(btf)) { + bpf_log(log, "Invalid btf_fd.\n"); err = PTR_ERR(btf); goto free_map; } @@ -1569,6 +1588,7 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr) bpfptr_t uprog_hash = make_bpfptr(attr->excl_prog_hash, uattr.is_kernel);
if (attr->excl_prog_hash_size != SHA256_DIGEST_SIZE) { + bpf_log(log, "Invalid excl_prog_hash_size.\n"); err = -EINVAL; goto free_map; } @@ -1584,6 +1604,7 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr) goto free_map; } } else if (attr->excl_prog_hash_size) { + bpf_log(log, "Invalid excl_prog_hash_size.\n"); err = -EINVAL; goto free_map; } @@ -1622,6 +1643,29 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr) return err; }
+static int map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_attrs *common_attrs) +{ + struct bpf_verifier_log *log; + struct bpf_log_attr log_attr; + int err, ret; + + log = bpf_log_attr_create_vlog(&log_attr, common_attrs); + if (IS_ERR(log)) + return PTR_ERR(log); + + err = __map_create(attr, uattr, log); + if (err >= 0) + goto free; + + ret = bpf_log_attr_finalize(&log_attr, log); + if (ret) + err = ret; + +free: + kfree(log); + return err; +} + void bpf_map_inc(struct bpf_map *map) { atomic64_inc(&map->refcnt); @@ -6218,7 +6262,8 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size,
switch (cmd) { case BPF_MAP_CREATE: - err = map_create(&attr, uattr); + bpf_attrs_init(&common_attrs, &common_attr, uattr_common, size_common); + err = map_create(&attr, uattr, &common_attrs); break; case BPF_MAP_LOOKUP_ELEM: err = map_lookup_elem(&attr);