Re: [PATCH v7 bpf-next 02/10] libbpf: Support kind layout section handling in BTF

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 4623 bytes --]

> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index b136572e889a..531089a64adc 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c

[ ... ]

> @@ -214,7 +232,7 @@ static int btf_add_type_idx_entry(struct btf *btf, __u32 type_off)
>  	return 0;
>  }
>
> -static void btf_bswap_hdr(struct btf_header *h)
> +static void btf_bswap_hdr(struct btf_header *h, __u32 hdr_len)
>  {
>  	h->magic = bswap_16(h->magic);
>  	h->hdr_len = bswap_32(h->hdr_len);
> @@ -222,54 +240,89 @@ 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);
> +	/* May be operating on raw data with hdr_len that does not include below fields */
> +	if (hdr_len >= sizeof(struct btf_header)) {
> +		h->kind_layout_off = bswap_32(h->kind_layout_off);
> +		h->kind_layout_len = bswap_32(h->kind_layout_len);
> +	}
>  }
>
>  static int btf_parse_hdr(struct btf *btf)
>  {
> -	struct btf_header *hdr = btf->hdr;
> +	struct btf_header *hdr = btf->raw_data;
> +	__u32 hdr_len = hdr->hdr_len;
>  	__u32 meta_left;
>
> -	if (btf->raw_size < sizeof(struct btf_header)) {
> +	if (btf->raw_size < offsetofend(struct btf_header, str_len)) {
>  		pr_debug("BTF header not found\n");
>  		return -EINVAL;
>  	}
>
>  	if (hdr->magic == bswap_16(BTF_MAGIC)) {
>  		btf->swapped_endian = true;
> -		if (bswap_32(hdr->hdr_len) != sizeof(struct btf_header)) {
> +		hdr_len = bswap_32(hdr->hdr_len);
> +		if (hdr_len < offsetofend(struct btf_header, str_len)) {
>  			pr_warn("Can't load BTF with non-native endianness due to unsupported header length %u\n",
> -				bswap_32(hdr->hdr_len));
> +				hdr_len);
>  			return -ENOTSUP;
>  		}
> -		btf_bswap_hdr(hdr);
>  	} else if (hdr->magic != BTF_MAGIC) {
>  		pr_debug("Invalid BTF magic: %x\n", hdr->magic);
>  		return -EINVAL;
>  	}
>
> -	if (btf->raw_size < hdr->hdr_len) {
> +	if (btf->raw_size < hdr_len) {
>  		pr_debug("BTF header len %u larger than data size %u\n",
> -			 hdr->hdr_len, btf->raw_size);
> +			 hdr_len, btf->raw_size);
>  		return -EINVAL;
>  	}
>
> -	meta_left = btf->raw_size - hdr->hdr_len;
> -	if (meta_left < (long long)hdr->str_off + hdr->str_len) {
> +	/* At this point, we have basic header information, so allocate btf->hdr */
> +	btf->hdr = calloc(1, sizeof(struct btf_header));
> +	if (!btf->hdr) {
> +		pr_debug("BTF header allocation failed\n");
> +		return -ENOMEM;
> +	}
> +	if (btf->swapped_endian)
> +		btf_bswap_hdr(hdr, hdr_len);
                    ^^^
Can this segfault on mmap'd BTF data?  In btf_parse_raw_mmap(), the BTF
file is mmap'd with PROT_READ (read-only), then btf_new() is called with
is_mmap=true.  The hdr pointer here points to btf->raw_data which is the
mmap'd read-only memory.  When btf_bswap_hdr() attempts to write to hdr
(swapping the header fields in-place), it will write to read-only memory.

This differs from the old code which rejected swapped-endian BTF with
the old strict header size check.  Now that the header size check is
relaxed to support kind layout, swapped-endian BTF from mmap becomes
accessible but crashes.

Call chain: btf__parse_raw()->btf_parse_raw_mmap() mmaps PROT_READ at
mmap() call, then btf_new(..., true) sets btf->raw_data to mmap'd
memory, then btf_parse_hdr() attempts to write to it.

> +	memcpy(btf->hdr, hdr, hdr_len < sizeof(struct btf_header) ? hdr_len :
> +								    sizeof(struct btf_header));

[ ... ]

> @@ -1550,6 +1629,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->kind_layout) {
> +		data_sz = roundup(data_sz, 4);
> +		data_sz += hdr->kind_layout_len;
> +		hdr->kind_layout_off = roundup(hdr->type_len + hdr->str_len, 4);
                                ^^^^^^^^^^^^^^^^^^^^
Should btf_get_raw_data() modify btf->hdr->kind_layout_off?  The hdr
pointer is btf->hdr, so this modifies the BTF object's header.  The
function signature takes const struct btf *btf_ro, suggesting the object
shouldn't be modified.  If btf__raw_data() is called multiple times,
kind_layout_off could potentially be set to different values on each
call, or if the BTF was originally loaded with kind_layout at a
different offset, that information is lost.

> +	}
>  	data = calloc(1, data_sz);
>  	if (!data)
>  		return NULL;

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20140850025