Re: [PATCH bpf-next v4 4/8] libbpf: Support BTF.ext loading and output in either endianness

On Fri, 2024-08-30 at 00:29 -0700, Tony Ambardar wrote:

[...]

@@ -3050,11 +3127,42 @@ static int btf_ext_parse_hdr(__u8 *data, __u32 data_size) return -ENOTSUP; }

• if (data_size == hdr->hdr_len) {

• if (data_size < hdr_len) {

• pr_debug("BTF.ext header not found\n");
  

• return -EINVAL;
  

• } else if (data_size == hdr_len) { pr_debug("BTF.ext has no data\n"); return -EINVAL; }

• /* Verify mandatory hdr info details present */
• if (hdr_len < offsetofend(struct btf_ext_header, line_info_len)) {

• pr_warn("BTF.ext header missing func_info, line_info\n");
  

• return -EINVAL;
  

• }
• /* Keep hdr native byte-order in memory for introspection */
• if (btf_ext->swapped_endian)

• btf_ext_bswap_hdr(btf_ext, hdr_len);
  

• /* Basic info section consistency checks*/
• info_size = btf_ext->data_size - hdr_len;
• if (info_size & 0x03) {

• pr_warn("BTF.ext info size not 4-byte multiple\n");
  

• return -EINVAL;
  

• }
• info_size -= hdr->func_info_len + hdr->line_info_len;
• if (hdr_len >= offsetofend(struct btf_ext_header, core_relo_len))

• info_size -= hdr->core_relo_len;
  

nit: Since we are checking this, maybe also check that sections do not overlap? Also, why disallowing gaps between sections?

• if (info_size) {

• pr_warn("BTF.ext info size mismatch with header data\n");
  

• return -EINVAL;
  

• }
• /* Keep infos native byte-order in memory for introspection */
• if (btf_ext->swapped_endian)

• btf_ext_bswap_info(btf_ext, !btf_ext->swapped_endian);
  

• return 0;

}

[...]

@@ -3119,15 +3223,71 @@ struct btf_ext *btf_ext__new(const __u8 *data, __u32 size) return btf_ext; } +static void *btf_ext_raw_data(const struct btf_ext *btf_ext_ro, __u32 *size,

• 	      bool swap_endian)
  

+{

• struct btf_ext *btf_ext = (struct btf_ext *)btf_ext_ro;
• const __u32 data_sz = btf_ext->data_size;
• void *data;
• data = swap_endian ? btf_ext->data_swapped : btf_ext->data;
• if (data) {

• *size = data_sz;
  

• return data;
  

• }
• data = calloc(1, data_sz);
• if (!data)

• return NULL;
  

• memcpy(data, btf_ext->data, data_sz);
• if (swap_endian) {

• btf_ext_bswap_info(btf_ext, true);
  

• btf_ext_bswap_hdr(btf_ext, btf_ext->hdr->hdr_len);
  

• btf_ext->data_swapped = data;
  

• }

Nit: I don't like how this function is organized: - if btf_ext->data can't be NULL swap_endian == true at this point; - if btf_ext->data can be NULL and swap_endian == false pointer to `data` would be lost.

I assume that btf_ext->data can't be null, basing on the btf_ext__new(), but function body is a bit confusing.

• *size = data_sz;
• return data;

+}

[...]