* Re: [PATCH v3 bpf-next 2/5] bpf: assign ID to vmlinux BTF and return extra info for BTF in GET_OBJ_INFO
[not found] ` <20201109210024.2024572-3-andrii@kernel.org>
@ 2020-11-09 21:55 ` Martin KaFai Lau
2020-11-10 0:09 ` Andrii Nakryiko
0 siblings, 1 reply; 2+ messages in thread
From: Martin KaFai Lau @ 2020-11-09 21:55 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, netdev, ast, daniel, kernel-team, linux-kernel, rafael,
jeyu, Arnaldo Carvalho de Melo, Greg Kroah-Hartman
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;
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v3 bpf-next 2/5] bpf: assign ID to vmlinux BTF and return extra info for BTF in GET_OBJ_INFO
2020-11-09 21:55 ` [PATCH v3 bpf-next 2/5] bpf: assign ID to vmlinux BTF and return extra info for BTF in GET_OBJ_INFO Martin KaFai Lau
@ 2020-11-10 0:09 ` Andrii Nakryiko
0 siblings, 0 replies; 2+ messages in thread
From: Andrii Nakryiko @ 2020-11-10 0:09 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
Daniel Borkmann, Kernel Team, open list, Rafael J. Wysocki,
Jessica Yu, Arnaldo Carvalho de Melo, Greg Kroah-Hartman
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;
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-11-10 0:09 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20201109210024.2024572-1-andrii@kernel.org>
[not found] ` <20201109210024.2024572-3-andrii@kernel.org>
2020-11-09 21:55 ` [PATCH v3 bpf-next 2/5] bpf: assign ID to vmlinux BTF and return extra info for BTF in GET_OBJ_INFO Martin KaFai Lau
2020-11-10 0:09 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).