Re: [PATCH v14 bpf-next 06/10] libbpf: Support sanitization of BTF layout for older kernels

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Sanitization produces invalid or truncated BTF if `type_off` is non-zero due to an incorrect size calculation.
--

--- Patch [6]: [PATCH v14 bpf-next 06/10] libbpf: Support sanitization of BTF layout for older kernels ---
commit fb06581dbb776e3aedd4935e88818d62fe25f19f
Author: Alan Maguire <[email protected]>

libbpf: Support sanitization of BTF layout for older kernels

This patch introduces a FEAT_BTF_LAYOUT feature check to see if the kernel
supports BTF layout information. It also adds a BTF sanitization step
to remove the layout section for older kernels that don't support it.

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 0662d72bad207..5d392fff76bdd 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c

[ ... ]

> @@ -3153,9 +3155,55 @@ static int bpf_object__sanitize_btf(struct bpf_object *obj, struct btf *btf)
>  	bool has_type_tag = kernel_supports(obj, FEAT_BTF_TYPE_TAG);
>  	bool has_enum64 = kernel_supports(obj, FEAT_BTF_ENUM64);
>  	bool has_qmark_datasec = kernel_supports(obj, FEAT_BTF_QMARK_DATASEC);
> +	bool has_layout = kernel_supports(obj, FEAT_BTF_LAYOUT);
>  	int enum64_placeholder_id = 0;
> +	const struct btf_header *hdr;
> +	struct btf *btf = NULL;
> +	const void *raw_data;
>  	struct btf_type *t;
>  	int i, j, vlen;
> +	__u32 sz;
> +	int err;
> +
> +	/* clone BTF to sanitize a copy and leave the original intact */
> +	raw_data = btf__raw_data(orig_btf, &sz);
> +	hdr = raw_data;
> +
> +	if (!has_layout && hdr->hdr_len >= sizeof(struct btf_header) &&
> +	    (hdr->layout_len != 0 || hdr->layout_off != 0)) {
> +		struct btf_header *new_hdr;
> +		void *new_raw_data;
> +
> +		/*
> +		 * Need to rewrite BTF to exclude layout information and
> +		 * move string section to immediately after types.
> +		 */
> +		new_raw_data = malloc(sz);
> +		if (!new_raw_data)
> +			return ERR_PTR(-ENOMEM);
> +
> +		memcpy(new_raw_data, raw_data, sz);
> +		new_hdr = new_raw_data;
> +		new_hdr->layout_off = 0;
> +		new_hdr->layout_len = 0;
> +		new_hdr->str_off = hdr->type_off + hdr->type_len;
> +
> +		memmove(new_raw_data + hdr->hdr_len + new_hdr->str_off,
> +			new_raw_data + hdr->hdr_len + hdr->str_off,
> +			hdr->str_len);
> +		sz = hdr->hdr_len + hdr->type_len + hdr->str_len;

[Severity: Medium]
Since new_hdr->str_off correctly includes hdr->type_off, should the size
calculation for sz also include hdr->type_off?

If hdr->type_off is non-zero, this sz calculation would be smaller than the
required space (hdr->hdr_len + new_hdr->str_off + hdr->str_len). Could this
result in a truncated string section when loading valid BTF on older kernels?

> +
> +		btf = btf__new(new_raw_data, sz);
> +		free(new_raw_data);
> +	} else {
> +		btf = btf__new(raw_data, sz);
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6