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;
> +}
> +
[...]
