Re: [RFC bpf-next 2/8] libbpf: support handling of metadata section in BTF

[Jiri Olsa <olsajiri@gmail.com>]

On Wed, May 31, 2023 at 09:19:29PM +0100, Alan Maguire wrote:
> support reading in metadata, fixing endian issues on reading;
> also support writing metadata section to raw BTF object.
> There is not yet an API to populate the metadata with meaningful
> information.
> 
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  tools/lib/bpf/btf.c | 141 ++++++++++++++++++++++++++++++++++----------
>  1 file changed, 111 insertions(+), 30 deletions(-)
> 
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 8484b563b53d..036dc1505969 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -16,6 +16,7 @@
>  #include <linux/err.h>
>  #include <linux/btf.h>
>  #include <gelf.h>
> +#include <zlib.h>
>  #include "btf.h"
>  #include "bpf.h"
>  #include "libbpf.h"
> @@ -39,36 +40,40 @@ struct btf {
>  
>  	/*
>  	 * When BTF is loaded from an ELF or raw memory it is stored
> -	 * in a contiguous memory block. The hdr, type_data, and, strs_data
> -	 * point inside that memory region to their respective parts of BTF
> -	 * representation:
> +	 * in a contiguous memory block. The hdr, type_data, strs_data,
> +	 * and optional meta_data point inside that memory region to their
> +	 * respective parts of BTF representation:
>  	 *
> -	 * +--------------------------------+
> -	 * |  Header  |  Types  |  Strings  |
> -	 * +--------------------------------+
> -	 * ^          ^         ^
> -	 * |          |         |
> -	 * hdr        |         |
> -	 * types_data-+         |
> -	 * strs_data------------+
> +	 * +--------------------------------+----------+
> +	 * |  Header  |  Types  |  Strings  | Metadata |
> +	 * +--------------------------------+----------+
> +	 * ^          ^         ^           ^
> +	 * |          |         |           |
> +	 * hdr        |         |           |
> +	 * types_data-+         |           |
> +	 * strs_data------------+           |
> +	 * meta_data------------------------+
> +	 *
> +	 * meta_data is optional.
>  	 *
>  	 * If BTF data is later modified, e.g., due to types added or
>  	 * removed, BTF deduplication performed, etc, this contiguous
> -	 * representation is broken up into three independently allocated
> -	 * memory regions to be able to modify them independently.
> +	 * representation is broken up into three or four independently
> +	 * allocated memory regions to be able to modify them independently.
>  	 * raw_data is nulled out at that point, but can be later allocated
>  	 * and cached again if user calls btf__raw_data(), at which point
> -	 * raw_data will contain a contiguous copy of header, types, and
> -	 * strings:
> +	 * raw_data will contain a contiguous copy of header, types, strings
> +	 * and (again optionally) metadata:
>  	 *
> -	 * +----------+  +---------+  +-----------+
> -	 * |  Header  |  |  Types  |  |  Strings  |
> -	 * +----------+  +---------+  +-----------+
> -	 * ^             ^            ^
> -	 * |             |            |
> -	 * hdr           |            |
> -	 * types_data----+            |
> -	 * strset__data(strs_set)-----+
> +	 * +----------+  +---------+  +-----------+  +----------+
> +	 * |  Header  |  |  Types  |  |  Strings  |  | Metadata |
> +	 * +----------+  +---------+  +-----------+  +---------_+
> +	 * ^             ^            ^              ^
> +	 * |             |            |              |
> +	 * hdr           |            |              |
> +	 * types_data----+            |              |
> +	 * strset__data(strs_set)-----+              |
> +	 * meta_data---------------------------------+
>  	 *
>  	 *               +----------+---------+-----------+
>  	 *               |  Header  |  Types  |  Strings  |
> @@ -116,6 +121,8 @@ struct btf {
>  	/* whether strings are already deduplicated */
>  	bool strs_deduped;
>  
> +	void *meta_data;
> +
>  	/* BTF object FD, if loaded into kernel */
>  	int fd;
>  
> @@ -215,6 +222,11 @@ static void btf_bswap_hdr(struct btf_header *h)
>  	h->type_len = bswap_32(h->type_len);
>  	h->str_off = bswap_32(h->str_off);
>  	h->str_len = bswap_32(h->str_len);
> +	if (h->hdr_len >= sizeof(struct btf_header)) {
> +		h->meta_header.meta_off = bswap_32(h->meta_header.meta_off);
> +		h->meta_header.meta_len = bswap_32(h->meta_header.meta_len);
> +	}
> +
>  }
>  
>  static int btf_parse_hdr(struct btf *btf)
> @@ -222,14 +234,17 @@ static int btf_parse_hdr(struct btf *btf)
>  	struct btf_header *hdr = btf->hdr;
>  	__u32 meta_left;
>  
> -	if (btf->raw_size < sizeof(struct btf_header)) {
> +	if (btf->raw_size < sizeof(struct btf_header) - sizeof(struct btf_meta_header)) {
>  		pr_debug("BTF header not found\n");
>  		return -EINVAL;
>  	}
>  
>  	if (hdr->magic == bswap_16(BTF_MAGIC)) {
> +		int swapped_len = bswap_32(hdr->hdr_len);
> +
>  		btf->swapped_endian = true;
> -		if (bswap_32(hdr->hdr_len) != sizeof(struct btf_header)) {
> +		if (swapped_len != sizeof(struct btf_header) &&
> +		    swapped_len != sizeof(struct btf_header) - sizeof(struct btf_meta_header)) {
>  			pr_warn("Can't load BTF with non-native endianness due to unsupported header length %u\n",
>  				bswap_32(hdr->hdr_len));
>  			return -ENOTSUP;
> @@ -285,6 +300,42 @@ static int btf_parse_str_sec(struct btf *btf)
>  	return 0;
>  }
>  
> +static void btf_bswap_meta(struct btf_metadata *meta, int len)
> +{
> +	struct btf_kind_meta *m = &meta->kind_meta[0];
> +	struct btf_kind_meta *end = (void *)meta + len;
> +
> +	meta->flags = bswap_32(meta->flags);
> +	meta->crc = bswap_32(meta->crc);
> +	meta->base_crc = bswap_32(meta->base_crc);
> +	meta->description_off = bswap_32(meta->description_off);
> +
> +	while (m < end) {
> +		m->name_off = bswap_32(m->name_off);
> +		m->flags = bswap_16(m->flags);
> +		m++;
> +	}
> +}
> +
> +static int btf_parse_meta_sec(struct btf *btf)
> +{
> +	const struct btf_header *hdr = btf->hdr;
> +
> +	if (hdr->hdr_len < sizeof(struct btf_header) ||
> +	    !hdr->meta_header.meta_off || !hdr->meta_header.meta_len)
> +		return 0;

I'm trying to figure out how is the meta data optional, and it seems to be
in here, right? but hdr->meta_header.meta_off or hdr->meta_header.meta_len
must be set NULL or zero

I'm getting crash when running btf test and it seems like correption when
parsing BTF generated from clang, which does not have this meta support

we do need clang support for this right? or bump version?

thanks,
jirka


> +	if (hdr->meta_header.meta_len < sizeof(struct btf_metadata)) {
> +		pr_debug("Invalid BTF metadata section\n");
> +		return -EINVAL;
> +	}
> +	btf->meta_data = btf->raw_data + btf->hdr->hdr_len + btf->hdr->meta_header.meta_off;
> +
> +	if (btf->swapped_endian)
> +		btf_bswap_meta(btf->meta_data, hdr->meta_header.meta_len);
> +
> +	return 0;
> +}
> +
>  static int btf_type_size(const struct btf_type *t)
>  {
>  	const int base_size = sizeof(struct btf_type);
> @@ -904,6 +955,7 @@ static struct btf *btf_new(const void *data, __u32 size, struct btf *base_btf)
>  	err = err ?: btf_parse_type_sec(btf);
>  	if (err)
>  		goto done;
> +	err = btf_parse_meta_sec(btf);
>  
>  done:
>  	if (err) {
> @@ -1267,6 +1319,11 @@ static void *btf_get_raw_data(const struct btf *btf, __u32 *size, bool swap_endi
>  	}
>  
>  	data_sz = hdr->hdr_len + hdr->type_len + hdr->str_len;
> +	if (btf->meta_data) {
> +		data_sz = roundup(data_sz, 8);
> +		data_sz += hdr->meta_header.meta_len;
> +		hdr->meta_header.meta_off = roundup(hdr->type_len + hdr->str_len, 8);
> +	}
>  	data = calloc(1, data_sz);
>  	if (!data)
>  		return NULL;
> @@ -1293,8 +1350,21 @@ static void *btf_get_raw_data(const struct btf *btf, __u32 *size, bool swap_endi
>  	p += hdr->type_len;
>  
>  	memcpy(p, btf_strs_data(btf), hdr->str_len);
> -	p += hdr->str_len;
> +	/* round up to 8 byte alignment to match offset above */
> +	p = data + hdr->hdr_len + roundup(hdr->type_len + hdr->str_len, 8);
> +
> +	if (btf->meta_data) {
> +		struct btf_metadata *meta = p;
>  
> +		memcpy(p, btf->meta_data, hdr->meta_header.meta_len);
> +		if (!swap_endian) {
> +			meta->crc = crc32(0L, (const Bytef *)&data, sizeof(data));
> +			meta->flags |= BTF_META_CRC_SET;
> +		}
> +		if (swap_endian)
> +			btf_bswap_meta(p, hdr->meta_header.meta_len);
> +		p += hdr->meta_header.meta_len;
> +	}
>  	*size = data_sz;
>  	return data;
>  err_out:
> @@ -1425,13 +1495,13 @@ static void btf_invalidate_raw_data(struct btf *btf)
>  	}
>  }
>  
> -/* Ensure BTF is ready to be modified (by splitting into a three memory
> - * regions for header, types, and strings). Also invalidate cached
> - * raw_data, if any.
> +/* Ensure BTF is ready to be modified (by splitting into a three or four memory
> + * regions for header, types, strings and optional metadata). Also invalidate
> + * cached raw_data, if any.
>   */
>  static int btf_ensure_modifiable(struct btf *btf)
>  {
> -	void *hdr, *types;
> +	void *hdr, *types, *meta = NULL;
>  	struct strset *set = NULL;
>  	int err = -ENOMEM;
>  
> @@ -1446,9 +1516,17 @@ static int btf_ensure_modifiable(struct btf *btf)
>  	types = malloc(btf->hdr->type_len);
>  	if (!hdr || !types)
>  		goto err_out;
> +	if (btf->hdr->hdr_len >= sizeof(struct btf_header)  &&
> +	    btf->hdr->meta_header.meta_off && btf->hdr->meta_header.meta_len) {
> +		meta = calloc(1, btf->hdr->meta_header.meta_len);
> +		if (!meta)
> +			goto err_out;
> +	}
>  
>  	memcpy(hdr, btf->hdr, btf->hdr->hdr_len);
>  	memcpy(types, btf->types_data, btf->hdr->type_len);
> +	if (meta)
> +		memcpy(meta, btf->meta_data, btf->hdr->meta_header.meta_len);
>  
>  	/* build lookup index for all strings */
>  	set = strset__new(BTF_MAX_STR_OFFSET, btf->strs_data, btf->hdr->str_len);
> @@ -1463,6 +1541,8 @@ static int btf_ensure_modifiable(struct btf *btf)
>  	btf->types_data_cap = btf->hdr->type_len;
>  	btf->strs_data = NULL;
>  	btf->strs_set = set;
> +	btf->meta_data = meta;
> +
>  	/* if BTF was created from scratch, all strings are guaranteed to be
>  	 * unique and deduplicated
>  	 */
> @@ -1480,6 +1560,7 @@ static int btf_ensure_modifiable(struct btf *btf)
>  	strset__free(set);
>  	free(hdr);
>  	free(types);
> +	free(meta);
>  	return err;
>  }
>  
> -- 
> 2.31.1
>