| Message ID | 20201109210024.2024572-3-andrii@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | Integrate kernel module BTF support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/apply | fail | Patch does not apply to bpf-next |
| netdev/tree_selection | success | Clearly marked for bpf-next |
On Mon, Nov 09, 2020 at 01:00:21PM -0800, Andrii Nakryiko wrote: > 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 ("vmlinux" for vmlinux BTF, will equal to module's name for 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 | 39 ++++++++++++++++++++++++++++++++-- > tools/include/uapi/linux/bpf.h | 3 +++ > 3 files changed, 43 insertions(+), 2 deletions(-) > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index 9879d6793e90..162999b12790 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -4466,6 +4466,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 894ee33f4c84..663c3fb4e614 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 { > @@ -4430,6 +4432,8 @@ struct btf *btf_parse_vmlinux(void) > > btf->data = __start_BTF; > btf->data_size = __stop_BTF - __start_BTF; > + btf->kernel_btf = true; > + snprintf(btf->name, sizeof(btf->name), "vmlinux"); > > err = btf_parse_hdr(env); > if (err) > @@ -4455,8 +4459,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: > @@ -5554,7 +5563,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; > @@ -5571,6 +5581,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; It should still do copy_to_user() even it will return -ENOSPC. > + } > + } > + > if (copy_to_user(uinfo, &info, info_copy) || > put_user(info_copy, &uattr->info.info_len)) > return -EFAULT;
On Mon, Nov 9, 2020 at 1:55 PM Martin KaFai Lau <kafai@fb.com> wrote: > > On Mon, Nov 09, 2020 at 01:00:21PM -0800, Andrii Nakryiko wrote: > > 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 ("vmlinux" for vmlinux BTF, will equal to module's name for 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 | 39 ++++++++++++++++++++++++++++++++-- > > tools/include/uapi/linux/bpf.h | 3 +++ > > 3 files changed, 43 insertions(+), 2 deletions(-) > > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > index 9879d6793e90..162999b12790 100644 > > --- a/include/uapi/linux/bpf.h > > +++ b/include/uapi/linux/bpf.h > > @@ -4466,6 +4466,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 894ee33f4c84..663c3fb4e614 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 { > > @@ -4430,6 +4432,8 @@ struct btf *btf_parse_vmlinux(void) > > > > btf->data = __start_BTF; > > btf->data_size = __stop_BTF - __start_BTF; > > + btf->kernel_btf = true; > > + snprintf(btf->name, sizeof(btf->name), "vmlinux"); > > > > err = btf_parse_hdr(env); > > if (err) > > @@ -4455,8 +4459,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: > > @@ -5554,7 +5563,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; > > @@ -5571,6 +5581,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; > It should still do copy_to_user() even it will return -ENOSPC. > Good catch! I'll use "ret" variable, will set it to -ENOSPC here and will let the code fall through. > > + } > > + } > > + > > if (copy_to_user(uinfo, &info, info_copy) || > > put_user(info_copy, &uattr->info.info_len)) > > return -EFAULT;
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 9879d6793e90..162999b12790 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -4466,6 +4466,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 894ee33f4c84..663c3fb4e614 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 { @@ -4430,6 +4432,8 @@ struct btf *btf_parse_vmlinux(void) btf->data = __start_BTF; btf->data_size = __stop_BTF - __start_BTF; + btf->kernel_btf = true; + snprintf(btf->name, sizeof(btf->name), "vmlinux"); err = btf_parse_hdr(env); if (err) @@ -4455,8 +4459,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: @@ -5554,7 +5563,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; @@ -5571,6 +5581,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 9879d6793e90..162999b12790 100644 --- a/tools/include/uapi/linux/bpf.h +++ b/tools/include/uapi/linux/bpf.h @@ -4466,6 +4466,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 ("vmlinux" for vmlinux BTF, will equal to module's name for 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 | 39 ++++++++++++++++++++++++++++++++-- tools/include/uapi/linux/bpf.h | 3 +++ 3 files changed, 43 insertions(+), 2 deletions(-)