Re: [RFC bpf-next v3 08/11] bpf: pass attached BTF to find correct type info of struct_ops progs.

[Martin KaFai Lau <martin.lau@linux.dev>]

On 9/20/23 8:59 AM, thinker.li@gmail.com wrote:
> From: Kui-Feng Lee <thinker.li@gmail.com>
> 
> The type info of a struct_ops type may be in a module.  So, we need to know
> which module BTF to look for type information.  The later patches will make
> libbpf to attach module BTFs to programs. This patch passes attached BTF
> from syscall to bpf_struct_ops subsystem to make sure attached BTF is
> available when the bpf_struct_ops subsystem is ready to use it.
> 
> bpf_prog has attach_btf in aux from attach_btf_obj_fd, that is pass along
> with the bpf_attr loading the program. attach_btf is used to find the btf
> type of attach_btf_id. attach_btf_id is used to identify the traced
> function for a trace program.  For struct_ops programs, it is used to
> identify the struct_ops type of the struct_ops object a program attached
> to.
> 
> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
> ---
>   include/uapi/linux/bpf.h       |  4 ++++
>   kernel/bpf/bpf_struct_ops.c    | 12 +++++++++++-
>   kernel/bpf/syscall.c           |  2 +-
>   kernel/bpf/verifier.c          |  4 +++-
>   tools/include/uapi/linux/bpf.h |  4 ++++
>   5 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 73b155e52204..178d6fa45fa0 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1390,6 +1390,10 @@ union bpf_attr {
>   		 * to using 5 hash functions).
>   		 */
>   		__u64	map_extra;
> +
> +		__u32   mod_btf_fd;	/* fd pointing to a BTF type data
> +					 * for btf_vmlinux_value_type_id.
> +					 */
>   	};
>   
>   	struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */
> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index 8b5c859377e9..d5600d9ad302 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
> @@ -765,9 +765,19 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
>   	struct bpf_struct_ops_map *st_map;
>   	const struct btf_type *t, *vt;
>   	struct bpf_map *map;
> +	struct btf *btf;
>   	int ret;
>   
> -	st_ops = bpf_struct_ops_find_value(attr->btf_vmlinux_value_type_id, btf_vmlinux);
> +	/* XXX: We need a module name or ID to find a BTF type. */
> +	/* XXX: should use btf from attr->btf_fd */
> +	if (attr->mod_btf_fd) {
> +		btf = btf_get_by_fd(attr->mod_btf_fd);

The btf is leaked in all cases because it is not stored (and owned) in st_map 
during map_alloc. This circle back to the earlier comment in patch 4 about where 
btf is stored.

> +		if (IS_ERR(btf))
> +			return ERR_PTR(PTR_ERR(btf));
> +	} else {
> +		btf = btf_vmlinux;
> +	}
> +	st_ops = bpf_struct_ops_find_value(attr->btf_vmlinux_value_type_id, btf);
>   	if (!st_ops)
>   		return ERR_PTR(-ENOTSUPP);
>