This patch extends the 'bpf()' syscall to support a set of common attributes shared across all BPF commands: 1. 'log_buf': User-provided buffer for storing logs 2. 'log_size': Size of the log buffer 3. 'log_level': Log verbosity level These common attributes are passed as the 4th argument to the 'bpf()' syscall, with the 5th argument specifying the size of this structure. To indicate the use of these common attributes from userspace, a new flag 'BPF_COMMON_ATTRS' ('1 << 16') is introduced. This flag is OR-ed into the 'cmd' field of the syscall. When 'cmd & BPF_COMMON_ATTRS' is set, the kernel will copy the common attributes from userspace into kernel space for use. Signed-off-by: Leon Hwang --- include/linux/syscalls.h | 3 ++- include/uapi/linux/bpf.h | 7 +++++++ kernel/bpf/syscall.c | 19 +++++++++++++++---- tools/include/uapi/linux/bpf.h | 7 +++++++ 4 files changed, 31 insertions(+), 5 deletions(-) diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h index 77f45e5d44139..94408575dc49b 100644 --- a/include/linux/syscalls.h +++ b/include/linux/syscalls.h @@ -933,7 +933,8 @@ asmlinkage long sys_seccomp(unsigned int op, unsigned int flags, asmlinkage long sys_getrandom(char __user *buf, size_t count, unsigned int flags); asmlinkage long sys_memfd_create(const char __user *uname_ptr, unsigned int flags); -asmlinkage long sys_bpf(int cmd, union bpf_attr __user *attr, unsigned int size); +asmlinkage long sys_bpf(int cmd, union bpf_attr __user *attr, unsigned int size, + struct bpf_common_attr __user *attr_common, unsigned int size_common); asmlinkage long sys_execveat(int dfd, const char __user *filename, const char __user *const __user *argv, const char __user *const __user *envp, int flags); diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 233de8677382e..5014baccf065f 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -1474,6 +1474,13 @@ struct bpf_stack_build_id { }; }; +struct bpf_common_attr { + __u64 log_buf; + __u32 log_size; + __u32 log_level; +}; + +#define BPF_COMMON_ATTRS (1 << 16) #define BPF_OBJ_NAME_LEN 16U enum { diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 3f178a0f8eb12..d49f822ceea12 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -5987,8 +5987,10 @@ static int prog_stream_read(union bpf_attr *attr) return ret; } -static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size) +static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size, + bpfptr_t uattr_common, unsigned int size_common) { + struct bpf_common_attr common_attrs; union bpf_attr attr; int err; @@ -6002,6 +6004,14 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size) if (copy_from_bpfptr(&attr, uattr, size) != 0) return -EFAULT; + memset(&common_attrs, 0, sizeof(common_attrs)); + if (cmd & BPF_COMMON_ATTRS) { + cmd &= ~BPF_COMMON_ATTRS; + size_common = min_t(u32, size_common, sizeof(common_attrs)); + if (uattr_common.user && copy_from_bpfptr(&common_attrs, uattr_common, size_common)) + return -EFAULT; + } + err = security_bpf(cmd, &attr, size, uattr.is_kernel); if (err < 0) return err; @@ -6134,9 +6144,10 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size) return err; } -SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size) +SYSCALL_DEFINE5(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size, + struct bpf_common_attr __user *, uattr_common, unsigned int, size_common) { - return __sys_bpf(cmd, USER_BPFPTR(uattr), size); + return __sys_bpf(cmd, USER_BPFPTR(uattr), size, USER_BPFPTR(uattr_common), size_common); } static bool syscall_prog_is_valid_access(int off, int size, @@ -6167,7 +6178,7 @@ BPF_CALL_3(bpf_sys_bpf, int, cmd, union bpf_attr *, attr, u32, attr_size) default: return -EINVAL; } - return __sys_bpf(cmd, KERNEL_BPFPTR(attr), attr_size); + return __sys_bpf(cmd, KERNEL_BPFPTR(attr), attr_size, KERNEL_BPFPTR(NULL), 0); } diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index 233de8677382e..5014baccf065f 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -1474,6 +1474,13 @@ struct bpf_stack_build_id { }; }; +struct bpf_common_attr { + __u64 log_buf; + __u32 log_size; + __u32 log_level; +}; + +#define BPF_COMMON_ATTRS (1 << 16) #define BPF_OBJ_NAME_LEN 16U enum { -- 2.50.1 To support the extended 'bpf()' syscall introduced in the previous commit, this patch adds the following APIs: 1. *Internal:* * 'sys_bpf_extended()' * 'sys_bpf_fd_extended()' These wrap the raw 'syscall()' interface to support passing extended attributes. 2. *Exported:* * 'probe_sys_bpf_extended()' This function checks whether the running kernel supports the extended 'bpf()' syscall with common attributes. Signed-off-by: Leon Hwang --- tools/lib/bpf/bpf.c | 45 +++++++++++++++++++++++++++++++++ tools/lib/bpf/bpf.h | 1 + tools/lib/bpf/features.c | 8 ++++++ tools/lib/bpf/libbpf.map | 2 ++ tools/lib/bpf/libbpf_internal.h | 2 ++ 5 files changed, 58 insertions(+) diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c index ab40dbf9f020f..27845e287dd5c 100644 --- a/tools/lib/bpf/bpf.c +++ b/tools/lib/bpf/bpf.c @@ -69,6 +69,51 @@ static inline __u64 ptr_to_u64(const void *ptr) return (__u64) (unsigned long) ptr; } +static inline int sys_bpf_extended(enum bpf_cmd cmd, union bpf_attr *attr, + unsigned int size, + struct bpf_common_attr *common_attrs, + unsigned int size_common) +{ + cmd = common_attrs ? cmd | BPF_COMMON_ATTRS : cmd & ~BPF_COMMON_ATTRS; + return syscall(__NR_bpf, cmd, attr, size, common_attrs, size_common); +} + +static inline int sys_bpf_fd_extended(enum bpf_cmd cmd, union bpf_attr *attr, + unsigned int size, + struct bpf_common_attr *common_attrs, + unsigned int size_common) +{ + int fd; + + fd = sys_bpf_extended(cmd, attr, size, common_attrs, size_common); + return ensure_good_fd(fd); +} + +int probe_sys_bpf_extended(int token_fd) +{ + const size_t attr_sz = offsetofend(union bpf_attr, prog_token_fd); + struct bpf_common_attr common_attrs; + struct bpf_insn insns[] = { + BPF_MOV64_IMM(BPF_REG_0, 0), + BPF_EXIT_INSN(), + }; + union bpf_attr attr; + + memset(&attr, 0, attr_sz); + attr.prog_type = BPF_PROG_TYPE_SOCKET_FILTER; + attr.license = ptr_to_u64("GPL"); + attr.insns = ptr_to_u64(insns); + attr.insn_cnt = (__u32)ARRAY_SIZE(insns); + attr.prog_token_fd = token_fd; + if (token_fd) + attr.prog_flags |= BPF_F_TOKEN_FD; + libbpf_strlcpy(attr.prog_name, "libbpf_sysbpftest", sizeof(attr.prog_name)); + memset(&common_attrs, 0, sizeof(common_attrs)); + + return sys_bpf_fd_extended(BPF_PROG_LOAD, &attr, attr_sz, &common_attrs, + sizeof(common_attrs)); +} + static inline int sys_bpf(enum bpf_cmd cmd, union bpf_attr *attr, unsigned int size) { diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h index 7252150e7ad35..38819071ecbe7 100644 --- a/tools/lib/bpf/bpf.h +++ b/tools/lib/bpf/bpf.h @@ -35,6 +35,7 @@ extern "C" { #endif +LIBBPF_API int probe_sys_bpf_extended(int token_fd); LIBBPF_API int libbpf_set_memlock_rlim(size_t memlock_bytes); struct bpf_map_create_opts { diff --git a/tools/lib/bpf/features.c b/tools/lib/bpf/features.c index 760657f5224c2..a63172c6343d0 100644 --- a/tools/lib/bpf/features.c +++ b/tools/lib/bpf/features.c @@ -507,6 +507,11 @@ static int probe_kern_arg_ctx_tag(int token_fd) return probe_fd(prog_fd); } +static int probe_kern_extended_syscall(int token_fd) +{ + return probe_fd(probe_sys_bpf_extended(token_fd)); +} + typedef int (*feature_probe_fn)(int /* token_fd */); static struct kern_feature_cache feature_cache; @@ -582,6 +587,9 @@ static struct kern_feature_desc { [FEAT_BTF_QMARK_DATASEC] = { "BTF DATASEC names starting from '?'", probe_kern_btf_qmark_datasec, }, + [FEAT_EXTENDED_SYSCALL] = { + "Kernel supports extended syscall", probe_kern_extended_syscall, + }, }; bool feat_supported(struct kern_feature_cache *cache, enum kern_feature_id feat_id) diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map index d7bd463e7017e..83d3d1af5ed1e 100644 --- a/tools/lib/bpf/libbpf.map +++ b/tools/lib/bpf/libbpf.map @@ -448,4 +448,6 @@ LIBBPF_1.6.0 { } LIBBPF_1.5.0; LIBBPF_1.7.0 { + global: + probe_sys_bpf_extended; } LIBBPF_1.6.0; diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h index 477a3b3389a09..497d845339a6b 100644 --- a/tools/lib/bpf/libbpf_internal.h +++ b/tools/lib/bpf/libbpf_internal.h @@ -380,6 +380,8 @@ enum kern_feature_id { FEAT_ARG_CTX_TAG, /* Kernel supports '?' at the front of datasec names */ FEAT_BTF_QMARK_DATASEC, + /* Kernel supports extended syscall */ + FEAT_EXTENDED_SYSCALL, __FEAT_CNT, }; -- 2.50.1 The log buffer of common attributes would be confusing with the one in 'union bpf_attr' for BPF_PROG_LOAD and BPF_BTF_LOAD. In order to clarify the usage of these two 'log_buf's, they both can be used for logging if: * They are same, including 'log_buf', 'log_level' and 'log_size'. * One of them is missing, then another one will be used for logging. If they both have 'log_buf' but they are not same, a log message will be written to the log buffer of 'union bpf_attr'. Signed-off-by: Leon Hwang --- include/linux/bpf.h | 3 ++- include/linux/bpf_verifier.h | 2 +- include/linux/btf.h | 3 ++- kernel/bpf/btf.c | 12 +++++++----- kernel/bpf/log.c | 23 ++++++++++++++++++++++- kernel/bpf/syscall.c | 14 ++++++++------ kernel/bpf/verifier.c | 8 ++++---- 7 files changed, 46 insertions(+), 19 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 8f6e87f0f3a89..adc0e68cb4e50 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -2717,7 +2717,8 @@ int bpf_check_uarg_tail_zero(bpfptr_t uaddr, size_t expected_size, size_t actual_size); /* verify correctness of eBPF program */ -int bpf_check(struct bpf_prog **fp, union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size); +int bpf_check(struct bpf_prog **fp, union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size, + struct bpf_common_attr *common_attrs); #ifndef CONFIG_BPF_JIT_ALWAYS_ON void bpf_patch_call_args(struct bpf_insn *insn, u32 stack_depth); diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 020de62bd09cd..2d61afec91c92 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -864,7 +864,7 @@ __printf(2, 3) void bpf_verifier_log_write(struct bpf_verifier_env *env, __printf(2, 3) void bpf_log(struct bpf_verifier_log *log, const char *fmt, ...); int bpf_vlog_init(struct bpf_verifier_log *log, u32 log_level, - char __user *log_buf, u32 log_size); + char __user *log_buf, u32 log_size, const struct bpf_common_attr *common_attrs); void bpf_vlog_reset(struct bpf_verifier_log *log, u64 new_pos); int bpf_vlog_finalize(struct bpf_verifier_log *log, u32 *log_size_actual); diff --git a/include/linux/btf.h b/include/linux/btf.h index 9eda6b113f9b4..c0acb46930bde 100644 --- a/include/linux/btf.h +++ b/include/linux/btf.h @@ -145,7 +145,8 @@ const char *btf_get_name(const struct btf *btf); void btf_get(struct btf *btf); void btf_put(struct btf *btf); const struct btf_header *btf_header(const struct btf *btf); -int btf_new_fd(const union bpf_attr *attr, bpfptr_t uattr, u32 uattr_sz); +int btf_new_fd(const union bpf_attr *attr, bpfptr_t uattr, u32 uattr_sz, + const struct bpf_common_attr *common_attrs); struct btf *btf_get_by_fd(int fd); int btf_get_info_by_fd(const struct btf *btf, const union bpf_attr *attr, diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 64739308902f7..4a17ae4842210 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -5771,7 +5771,8 @@ static int finalize_log(struct bpf_verifier_log *log, bpfptr_t uattr, u32 uattr_ return err; } -static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size) +static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size, + const struct bpf_common_attr *common_attrs) { bpfptr_t btf_data = make_bpfptr(attr->btf, uattr.is_kernel); char __user *log_ubuf = u64_to_user_ptr(attr->btf_log_buf); @@ -5791,8 +5792,8 @@ static struct btf *btf_parse(const union bpf_attr *attr, bpfptr_t uattr, u32 uat /* user could have requested verbose verifier output * and supplied buffer to store the verification trace */ - err = bpf_vlog_init(&env->log, attr->btf_log_level, - log_ubuf, attr->btf_log_size); + err = bpf_vlog_init(&env->log, attr->btf_log_level, log_ubuf, attr->btf_log_size, + common_attrs); if (err) goto errout_free; @@ -8028,12 +8029,13 @@ static int __btf_new_fd(struct btf *btf) return anon_inode_getfd("btf", &btf_fops, btf, O_RDONLY | O_CLOEXEC); } -int btf_new_fd(const union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size) +int btf_new_fd(const union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size, + const struct bpf_common_attr *common_attrs) { struct btf *btf; int ret; - btf = btf_parse(attr, uattr, uattr_size); + btf = btf_parse(attr, uattr, uattr_size, common_attrs); if (IS_ERR(btf)) return PTR_ERR(btf); diff --git a/kernel/bpf/log.c b/kernel/bpf/log.c index e4983c1303e76..a9a0834884eb9 100644 --- a/kernel/bpf/log.c +++ b/kernel/bpf/log.c @@ -29,12 +29,33 @@ static bool bpf_verifier_log_attr_valid(const struct bpf_verifier_log *log) } int bpf_vlog_init(struct bpf_verifier_log *log, u32 log_level, - char __user *log_buf, u32 log_size) + char __user *log_buf, u32 log_size, const struct bpf_common_attr *common_attrs) { + u32 log_true_size; + int err; + log->level = log_level; log->ubuf = log_buf; log->len_total = log_size; + if (log_buf && common_attrs && common_attrs->log_buf && + ((u64) log_buf != common_attrs->log_buf || log_level != common_attrs->log_level || + log_size != common_attrs->log_size)) { + if (!bpf_verifier_log_attr_valid(log)) + return -EINVAL; + bpf_log(log, "Conflict log configs between bpf_attr and common_attr.\n"); + err = bpf_vlog_finalize(log, &log_true_size); + if (err) + return err; + return -EINVAL; + } + + if (!log_buf && common_attrs && common_attrs->log_buf) { + log->level = common_attrs->log_level; + log->ubuf = u64_to_user_ptr(common_attrs->log_buf); + log->len_total = common_attrs->log_size; + } + /* log attributes have to be sane */ if (!bpf_verifier_log_attr_valid(log)) return -EINVAL; diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index d49f822ceea12..5e5cf0262a14e 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -2764,7 +2764,8 @@ static bool is_perfmon_prog_type(enum bpf_prog_type prog_type) /* last field in 'union bpf_attr' used by this command */ #define BPF_PROG_LOAD_LAST_FIELD fd_array_cnt -static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size) +static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size, + struct bpf_common_attr *common_attrs) { enum bpf_prog_type type = attr->prog_type; struct bpf_prog *prog, *dst_prog = NULL; @@ -2976,7 +2977,7 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size) goto free_prog_sec; /* run eBPF verifier */ - err = bpf_check(&prog, attr, uattr, uattr_size); + err = bpf_check(&prog, attr, uattr, uattr_size, common_attrs); if (err < 0) goto free_used_maps; @@ -5292,7 +5293,8 @@ static int bpf_obj_get_info_by_fd(const union bpf_attr *attr, #define BPF_BTF_LOAD_LAST_FIELD btf_token_fd -static int bpf_btf_load(const union bpf_attr *attr, bpfptr_t uattr, __u32 uattr_size) +static int bpf_btf_load(const union bpf_attr *attr, bpfptr_t uattr, __u32 uattr_size, + struct bpf_common_attr *common_attrs) { struct bpf_token *token = NULL; @@ -5319,7 +5321,7 @@ static int bpf_btf_load(const union bpf_attr *attr, bpfptr_t uattr, __u32 uattr_ bpf_token_put(token); - return btf_new_fd(attr, uattr, uattr_size); + return btf_new_fd(attr, uattr, uattr_size, common_attrs); } #define BPF_BTF_GET_FD_BY_ID_LAST_FIELD fd_by_id_token_fd @@ -6036,7 +6038,7 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size, err = map_freeze(&attr); break; case BPF_PROG_LOAD: - err = bpf_prog_load(&attr, uattr, size); + err = bpf_prog_load(&attr, uattr, size, &common_attrs); break; case BPF_OBJ_PIN: err = bpf_obj_pin(&attr); @@ -6081,7 +6083,7 @@ static int __sys_bpf(enum bpf_cmd cmd, bpfptr_t uattr, unsigned int size, err = bpf_raw_tracepoint_open(&attr); break; case BPF_BTF_LOAD: - err = bpf_btf_load(&attr, uattr, size); + err = bpf_btf_load(&attr, uattr, size, &common_attrs); break; case BPF_BTF_GET_FD_BY_ID: err = bpf_btf_get_fd_by_id(&attr); diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index b9394f8fac0ed..77b57289ec097 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -24584,7 +24584,8 @@ static int compute_scc(struct bpf_verifier_env *env) return err; } -int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u32 uattr_size) +int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u32 uattr_size, + struct bpf_common_attr *common_attrs) { u64 start_time = ktime_get_ns(); struct bpf_verifier_env *env; @@ -24633,9 +24634,8 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3 /* user could have requested verbose verifier output * and supplied buffer to store the verification trace */ - ret = bpf_vlog_init(&env->log, attr->log_level, - (char __user *) (unsigned long) attr->log_buf, - attr->log_size); + ret = bpf_vlog_init(&env->log, attr->log_level, u64_to_user_ptr(attr->log_buf), + attr->log_size, common_attrs); if (ret) goto err_unlock; -- 2.50.1 Currently, many 'BPF_MAP_CREATE' failures return '-EINVAL' without providing any explanation to user space. With the extended BPF syscall support introduced in the previous patch, detailed error messages can now be reported. This allows users to understand the specific reason for a failed map creation, rather than just receiving a generic '-EINVAL'. Signed-off-by: Leon Hwang --- kernel/bpf/syscall.c | 82 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 63 insertions(+), 19 deletions(-) diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 5e5cf0262a14e..2f5e6005671b5 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -1340,12 +1340,13 @@ static bool bpf_net_capable(void) #define BPF_MAP_CREATE_LAST_FIELD map_token_fd /* called via syscall */ -static int map_create(union bpf_attr *attr, bool kernel) +static int map_create(union bpf_attr *attr, bool kernel, struct bpf_common_attr *common_attrs) { + u32 map_type = attr->map_type, log_true_size; + struct bpf_verifier_log *log = NULL; const struct bpf_map_ops *ops; struct bpf_token *token = NULL; int numa_node = bpf_map_attr_numa_node(attr); - u32 map_type = attr->map_type; struct bpf_map *map; bool token_flag; int f_flags; @@ -1355,6 +1356,18 @@ static int map_create(union bpf_attr *attr, bool kernel) if (err) return -EINVAL; + if (common_attrs->log_buf) { + log = kvzalloc(sizeof(*log), GFP_KERNEL); + if (!log) + return -ENOMEM; + err = bpf_vlog_init(log, BPF_LOG_FIXED, u64_to_user_ptr(common_attrs->log_buf), + common_attrs->log_size, NULL); + if (err) { + kvfree(log); + return err; + } + } + /* 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 */ @@ -1363,16 +1376,24 @@ static int map_create(union bpf_attr *attr, bool kernel) 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) - return -EINVAL; + attr->btf_key_type_id || attr->btf_value_type_id) { + bpf_log(log, "Invalid use of btf_vmlinux_value_type_id.\n"); + err = -EINVAL; + goto put_token; + } } else if (attr->btf_key_type_id && !attr->btf_value_type_id) { - return -EINVAL; + bpf_log(log, "Invalid btf_value_type_id.\n"); + err = -EINVAL; + goto put_token; } if (attr->map_type != BPF_MAP_TYPE_BLOOM_FILTER && attr->map_type != BPF_MAP_TYPE_ARENA && - attr->map_extra != 0) - return -EINVAL; + attr->map_extra != 0) { + bpf_log(log, "Invalid map_extra.\n"); + err = -EINVAL; + goto put_token; + } f_flags = bpf_get_file_flag(attr->map_flags); if (f_flags < 0) @@ -1380,17 +1401,26 @@ static int map_create(union bpf_attr *attr, bool kernel) if (numa_node != NUMA_NO_NODE && ((unsigned int)numa_node >= nr_node_ids || - !node_online(numa_node))) - return -EINVAL; + !node_online(numa_node))) { + bpf_log(log, "Invalid or offline numa_node.\n"); + err = -EINVAL; + goto put_token; + } /* 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)) - return -EINVAL; + if (map_type >= ARRAY_SIZE(bpf_map_types)) { + bpf_log(log, "Invalid map_type.\n"); + err = -EINVAL; + goto put_token; + } map_type = array_index_nospec(map_type, ARRAY_SIZE(bpf_map_types)); ops = bpf_map_types[map_type]; - if (!ops) - return -EINVAL; + if (!ops) { + bpf_log(log, "Invalid map_type.\n"); + err = -EINVAL; + goto put_token; + } if (ops->map_alloc_check) { err = ops->map_alloc_check(attr); @@ -1399,13 +1429,20 @@ static int map_create(union bpf_attr *attr, bool kernel) } if (attr->map_ifindex) ops = &bpf_map_offload_ops; - if (!ops->map_mem_usage) - return -EINVAL; + if (!ops->map_mem_usage) { + bpf_log(log, "map_mem_usage is required.\n"); + err = -EINVAL; + goto put_token; + } if (token_flag) { token = bpf_token_get_from_fd(attr->map_token_fd); - if (IS_ERR(token)) - return PTR_ERR(token); + if (IS_ERR(token)) { + bpf_log(log, "Invalid map_token_fd.\n"); + err = PTR_ERR(token); + token = NULL; + goto put_token; + } /* if current token doesn't grant map creation permissions, * then we can't use this token, so ignore it and rely on @@ -1487,8 +1524,10 @@ static int map_create(union bpf_attr *attr, bool kernel) 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); @@ -1511,6 +1550,7 @@ static int map_create(union bpf_attr *attr, bool kernel) 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; } @@ -1565,6 +1605,10 @@ static int map_create(union bpf_attr *attr, bool kernel) bpf_map_free(map); put_token: bpf_token_put(token); + if (err && log) + (void) bpf_vlog_finalize(log, &log_true_size); + if (log) + kvfree(log); return err; } @@ -6020,7 +6064,7 @@ 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.is_kernel); + err = map_create(&attr, uattr.is_kernel, &common_attrs); break; case BPF_MAP_LOOKUP_ELEM: err = map_lookup_elem(&attr); -- 2.50.1 With the previous patch adding common attribute support for BPF_MAP_CREATE, it is now possible to retrieve detailed error messages when map creation fails by using the 'log_buf' field from the common attributes. This patch extends 'bpf_map_create_opts' with two new fields, 'log_buf' and 'log_size', allowing users to capture and inspect these log messages. Signed-off-by: Leon Hwang --- tools/lib/bpf/bpf.c | 16 +++++++++++++++- tools/lib/bpf/bpf.h | 5 ++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c index 27845e287dd5c..5b58e981a7669 100644 --- a/tools/lib/bpf/bpf.c +++ b/tools/lib/bpf/bpf.c @@ -218,7 +218,9 @@ int bpf_map_create(enum bpf_map_type map_type, const struct bpf_map_create_opts *opts) { const size_t attr_sz = offsetofend(union bpf_attr, map_token_fd); + struct bpf_common_attr common_attrs; union bpf_attr attr; + __u64 log_buf; int fd; bump_rlimit_memlock(); @@ -249,7 +251,19 @@ int bpf_map_create(enum bpf_map_type map_type, attr.map_token_fd = OPTS_GET(opts, token_fd, 0); - fd = sys_bpf_fd(BPF_MAP_CREATE, &attr, attr_sz); + log_buf = (__u64) OPTS_GET(opts, log_buf, NULL); + if (log_buf) { + if (!feat_supported(NULL, FEAT_EXTENDED_SYSCALL)) + return libbpf_err(-EOPNOTSUPP); + + memset(&common_attrs, 0, sizeof(common_attrs)); + common_attrs.log_buf = log_buf; + common_attrs.log_size = OPTS_GET(opts, log_size, 0); + fd = sys_bpf_extended(BPF_MAP_CREATE, &attr, attr_sz, &common_attrs, + sizeof(common_attrs)); + } else { + fd = sys_bpf_fd(BPF_MAP_CREATE, &attr, attr_sz); + } return libbpf_err_errno(fd); } diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h index 38819071ecbe7..3b54d6feb5842 100644 --- a/tools/lib/bpf/bpf.h +++ b/tools/lib/bpf/bpf.h @@ -55,9 +55,12 @@ struct bpf_map_create_opts { __s32 value_type_btf_obj_fd; __u32 token_fd; + + const char *log_buf; + __u32 log_size; size_t :0; }; -#define bpf_map_create_opts__last_field token_fd +#define bpf_map_create_opts__last_field log_size LIBBPF_API int bpf_map_create(enum bpf_map_type map_type, const char *map_name, -- 2.50.1 As kernel is able to report log when fail to create map, add test cases to check those logs. cd tools/testing/selftests/bpf ./test_progs -t map_create_failure 191/1 map_create_failure/invalid_vmlinux_value_type_id:OK 191/2 map_create_failure/invalid_value_type_id:OK 191/3 map_create_failure/invalid_map_extra:OK 191/4 map_create_failure/invalid_numa_node:OK 191/5 map_create_failure/invalid_map_type:OK 191/6 map_create_failure/invalid_token_fd:OK 191/7 map_create_failure/invalid_map_name:OK 191/8 map_create_failure/invalid_btf_fd:OK 191 map_create_failure:OK Summary: 1/8 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Leon Hwang --- .../selftests/bpf/prog_tests/map_init.c | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) diff --git a/tools/testing/selftests/bpf/prog_tests/map_init.c b/tools/testing/selftests/bpf/prog_tests/map_init.c index 14a31109dd0e0..ee7c73f5700c9 100644 --- a/tools/testing/selftests/bpf/prog_tests/map_init.c +++ b/tools/testing/selftests/bpf/prog_tests/map_init.c @@ -212,3 +212,127 @@ void test_map_init(void) if (test__start_subtest("pcpu_lru_map_init")) test_pcpu_lru_map_init(); } + +static void test_map_create(enum bpf_map_type map_type, const char *map_name, + struct bpf_map_create_opts *opts, const char *exp_msg) +{ + char log_buf[128]; + int fd; + + log_buf[0] = '\0'; + opts->log_buf = log_buf; + opts->log_size = sizeof(log_buf); + fd = bpf_map_create(map_type, map_name, 4, 4, 1, opts); + if (!ASSERT_LT(fd, 0, "bpf_map_create")) + goto out; + + ASSERT_STREQ(log_buf, exp_msg, "unexpected log_buf"); +out: + if (fd >= 0) + close(fd); +} + +static void test_map_create_simple(struct bpf_map_create_opts *opts, const char *exp_msg) +{ + test_map_create(BPF_MAP_TYPE_ARRAY, "test_map_create", opts, exp_msg); +} + +static void test_invalid_vmlinux_value_type_id(void) +{ + const char *msg = "Invalid use of btf_vmlinux_value_type_id.\n"; + LIBBPF_OPTS(bpf_map_create_opts, opts, + .btf_vmlinux_value_type_id = 1, + ); + + test_map_create_simple(&opts, msg); +} + +static void test_invalid_value_type_id(void) +{ + const char *msg = "Invalid btf_value_type_id.\n"; + LIBBPF_OPTS(bpf_map_create_opts, opts, + .btf_key_type_id = 1, + ); + + test_map_create_simple(&opts, msg); +} + +static void test_invalid_map_extra(void) +{ + const char *msg = "Invalid map_extra.\n"; + LIBBPF_OPTS(bpf_map_create_opts, opts, + .map_extra = 1, + ); + + test_map_create_simple(&opts, msg); +} + +static void test_invalid_numa_node(void) +{ + const char *msg = "Invalid or offline numa_node.\n"; + LIBBPF_OPTS(bpf_map_create_opts, opts, + .map_flags = BPF_F_NUMA_NODE, + .numa_node = 0xFF, + ); + + test_map_create_simple(&opts, msg); +} + +static void test_invalid_map_type(void) +{ + const char *msg = "Invalid map_type.\n"; + LIBBPF_OPTS(bpf_map_create_opts, opts); + + test_map_create(__MAX_BPF_MAP_TYPE, "test_map_create", &opts, msg); +} + +static void test_invalid_token_fd(void) +{ + const char *msg = "Invalid map_token_fd.\n"; + LIBBPF_OPTS(bpf_map_create_opts, opts, + .map_flags = BPF_F_TOKEN_FD, + .token_fd = 0xFF, + ); + + test_map_create_simple(&opts, msg); +} + +static void test_invalid_map_name(void) +{ + const char *msg = "Invalid map_name.\n"; + LIBBPF_OPTS(bpf_map_create_opts, opts); + + test_map_create(BPF_MAP_TYPE_ARRAY, "test-!@#", &opts, msg); +} + +static void test_invalid_btf_fd(void) +{ + const char *msg = "Invalid btf_fd.\n"; + LIBBPF_OPTS(bpf_map_create_opts, opts, + .btf_fd = -1, + .btf_key_type_id = 1, + .btf_value_type_id = 1, + ); + + test_map_create_simple(&opts, msg); +} + +void test_map_create_failure(void) +{ + if (test__start_subtest("invalid_vmlinux_value_type_id")) + test_invalid_vmlinux_value_type_id(); + if (test__start_subtest("invalid_value_type_id")) + test_invalid_value_type_id(); + if (test__start_subtest("invalid_map_extra")) + test_invalid_map_extra(); + if (test__start_subtest("invalid_numa_node")) + test_invalid_numa_node(); + if (test__start_subtest("invalid_map_type")) + test_invalid_map_type(); + if (test__start_subtest("invalid_token_fd")) + test_invalid_token_fd(); + if (test__start_subtest("invalid_map_name")) + test_invalid_map_name(); + if (test__start_subtest("invalid_btf_fd")) + test_invalid_btf_fd(); +} -- 2.50.1