Re: [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]

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@xxxxxxxxxx>
> ---
>  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;