| Message ID | 20201105045140.2589346-3-andrii@kernel.org |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | Integrate kernel module BTF support | expand |
| Context | Check | Description |
|---|---|---|
| jkicinski/cover_letter | success | Link |
| jkicinski/fixes_present | success | Link |
| jkicinski/patch_count | success | Link |
| jkicinski/tree_selection | success | Clearly marked for bpf-next |
| jkicinski/subject_prefix | success | Link |
| jkicinski/source_inline | success | Was 0 now: 0 |
| jkicinski/verify_signedoff | success | Link |
| jkicinski/module_param | success | Was 0 now: 0 |
| jkicinski/build_32bit | fail | Errors and warnings before: 16006 this patch: 16006 |
| jkicinski/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/verify_fixes | success | Link |
| jkicinski/checkpatch | fail | Link |
| jkicinski/build_allmodconfig_warn | success | Errors and warnings before: 16038 this patch: 16038 |
| jkicinski/header_inline | success | Link |
| jkicinski/stable | success | Stable not CCed |
On Wed, Nov 04, 2020 at 08:51:37PM -0800, Andrii Nakryiko wrote: > @@ -215,6 +215,8 @@ struct btf { > struct btf *base_btf; > u32 start_id; /* first type ID in this BTF (0 for base BTF) */ > u32 start_str_off; /* first string offset (0 for base BTF) */ > + char name[MODULE_NAME_LEN]; > + bool kernel_btf; > }; > > enum verifier_phase { > @@ -4441,6 +4443,7 @@ struct btf *btf_parse_vmlinux(void) > > btf->data = __start_BTF; > btf->data_size = __stop_BTF - __start_BTF; > + btf->kernel_btf = true; imo it's a bit weird for vmlinux's BTF to be flagged as 'kernel_btf' and empty name, but kernel module's BTFs will not be marked as kernel, but will have a name. I think it's more natural to make vmlinux and module's BTF with kernel_btf=true flag and give "vmlinux" name to base kernel BTF. If somebody creates a kernel module with "vmlinux" name we will have a conflict, but that name is for human pretty printing only anyway, so I think it's fine.
On Thu, Nov 5, 2020 at 7:19 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Wed, Nov 04, 2020 at 08:51:37PM -0800, Andrii Nakryiko wrote: > > @@ -215,6 +215,8 @@ struct btf { > > struct btf *base_btf; > > u32 start_id; /* first type ID in this BTF (0 for base BTF) */ > > u32 start_str_off; /* first string offset (0 for base BTF) */ > > + char name[MODULE_NAME_LEN]; > > + bool kernel_btf; > > }; > > > > enum verifier_phase { > > @@ -4441,6 +4443,7 @@ struct btf *btf_parse_vmlinux(void) > > > > btf->data = __start_BTF; > > btf->data_size = __stop_BTF - __start_BTF; > > + btf->kernel_btf = true; > > imo it's a bit weird for vmlinux's BTF to be flagged as 'kernel_btf' > and empty name, but kernel module's BTFs will not be marked as kernel, > but will have a name. module's BTF also has kernel_btf = true, see patch 4; would be weird otherwise > I think it's more natural to make vmlinux and module's BTF with kernel_btf=true flag > and give "vmlinux" name to base kernel BTF. Yeah, I was wondering if I should name vmlinux BTF as "vmlinux" explicitly, for whatever reason I decided to go with an empty name. But I think it's not a bad idea to give it an explicit "vmlinux" name. I'll do that in the next version. Will make bpftool logic more straightforward as well. > If somebody creates a kernel module with "vmlinux" name we will have a conflict, > but that name is for human pretty printing only anyway, so I think it's fine. yep.
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index e6ceac3f7d62..7955b144f267 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -4418,6 +4418,9 @@ struct bpf_btf_info { __aligned_u64 btf; __u32 btf_size; __u32 id; + __aligned_u64 name; + __u32 name_len; + __u32 kernel_btf; } __attribute__((aligned(8))); struct bpf_link_info { diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index f61944a3873b..6a99677e57c0 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -215,6 +215,8 @@ struct btf { struct btf *base_btf; u32 start_id; /* first type ID in this BTF (0 for base BTF) */ u32 start_str_off; /* first string offset (0 for base BTF) */ + char name[MODULE_NAME_LEN]; + bool kernel_btf; }; enum verifier_phase { @@ -4441,6 +4443,7 @@ struct btf *btf_parse_vmlinux(void) btf->data = __start_BTF; btf->data_size = __stop_BTF - __start_BTF; + btf->kernel_btf = true; err = btf_parse_hdr(env); if (err) @@ -4466,8 +4469,13 @@ struct btf *btf_parse_vmlinux(void) bpf_struct_ops_init(btf, log); - btf_verifier_env_free(env); refcount_set(&btf->refcnt, 1); + + err = btf_alloc_id(btf); + if (err) + goto errout; + + btf_verifier_env_free(env); return btf; errout: @@ -5565,7 +5573,8 @@ int btf_get_info_by_fd(const struct btf *btf, struct bpf_btf_info info; u32 info_copy, btf_copy; void __user *ubtf; - u32 uinfo_len; + char __user *uname; + u32 uinfo_len, uname_len, name_len; uinfo = u64_to_user_ptr(attr->info.info); uinfo_len = attr->info.info_len; @@ -5582,6 +5591,31 @@ int btf_get_info_by_fd(const struct btf *btf, return -EFAULT; info.btf_size = btf->data_size; + info.kernel_btf = btf->kernel_btf; + + uname = u64_to_user_ptr(info.name); + uname_len = info.name_len; + if (!uname ^ !uname_len) + return -EINVAL; + + name_len = strlen(btf->name); + info.name_len = name_len; + + if (uname) { + if (uname_len >= name_len + 1) { + if (copy_to_user(uname, btf->name, name_len + 1)) + return -EFAULT; + } else { + char zero = '\0'; + + if (copy_to_user(uname, btf->name, uname_len - 1)) + return -EFAULT; + if (put_user(zero, uname + uname_len - 1)) + return -EFAULT; + return -ENOSPC; + } + } + if (copy_to_user(uinfo, &info, info_copy) || put_user(info_copy, &uattr->info.info_len)) return -EFAULT; diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h index e6ceac3f7d62..7955b144f267 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -4418,6 +4418,9 @@ struct bpf_btf_info { __aligned_u64 btf; __u32 btf_size; __u32 id; + __aligned_u64 name; + __u32 name_len; + __u32 kernel_btf; } __attribute__((aligned(8))); struct bpf_link_info {
Allocate ID for vmlinux BTF. This makes it visible when iterating over all BTF objects in the system. To allow distinguishing vmlinux BTF (and later kernel module BTF) from user-provided BTFs, expose extra kernel_btf flag, as well as BTF name (empty for vmlinux BTF, but will be used for kernel module BTF). We might want to later allow specifying BTF name for user-provided BTFs as well, if that makes sense. But currently this is reserved only for in-kernel BTFs. Having in-kernel BTFs exposed IDs will allow to extend BPF APIs that require in-kernel BTF type with ability to specify BTF types from kernel modules, not just vmlinux BTF. This will be implemented in a follow up patch set for fentry/fexit/fmod_ret/lsm/etc. Signed-off-by: Andrii Nakryiko <andrii@kernel.org> --- include/uapi/linux/bpf.h | 3 +++ kernel/bpf/btf.c | 38 ++++++++++++++++++++++++++++++++-- tools/include/uapi/linux/bpf.h | 3 +++ 3 files changed, 42 insertions(+), 2 deletions(-)