Skip to content

Conversation

@kernel-patches-daemon-bpf
Copy link

Pull request for series with
subject: Add kind layout to BTF
version: 7
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1032375

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 759377d
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1032375
version: 7

@kernel-patches-review-bot
Copy link

commit 8c9fce49c07b8fd99b824ed1787d539dcf907ba8
Author: Alan Maguire <[email protected]>

libbpf: Support kind layout section handling in BTF

Support reading in kind layout fixing endian issues on reading; also
support writing kind layout section to raw BTF object. The BTF
representation is modified to always allocate a separate btf_header and
copy the valid portion from raw data to it.

> 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

In-Reply-To-Subject: libbpf: Support kind layout section handling in BTF
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20140850025

@kernel-patches-review-bot
Copy link

commit b76d9e18d2d3d3cd07a71fe66a3580d4bf942106
Author: Alan Maguire <[email protected]>

btf: support kernel parsing of BTF with kind layout

This commit adds kernel support for parsing the BTF kind_layout section,
which provides metadata about BTF kinds. The kernel validates the section
if present and rejects BTF containing unsupported kinds.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 0de8fc8a0e0b..d6221d3e7893 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c

[ ... ]

> @@ -5215,23 +5216,36 @@ static s32 btf_check_meta(struct btf_verifier_env *env,
>  		return -EINVAL;
>  	}
>
> -	if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX ||
> -	    BTF_INFO_KIND(t->info) == BTF_KIND_UNKN) {
> +	if (!btf_name_offset_valid(env->btf, t->name_off)) {
> +		btf_verifier_log(env, "[%u] Invalid name_offset:%u",
> +				 env->log_type_id, t->name_off);
> +		return -EINVAL;
> +	}
> +
> +	if (BTF_INFO_KIND(t->info) == BTF_KIND_UNKN) {
>  		btf_verifier_log(env, "[%u] Invalid kind:%u",
>  				 env->log_type_id, BTF_INFO_KIND(t->info));
>  		return -EINVAL;
>  	}
>
> -	if (!btf_name_offset_valid(env->btf, t->name_off)) {
> -		btf_verifier_log(env, "[%u] Invalid name_offset:%u",
> -				 env->log_type_id, t->name_off);
> +	if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX && env->btf->kind_layout &&
> +	    (BTF_INFO_KIND(t->info) * sizeof(struct btf_kind_layout)) <
> +	     env->btf->hdr.kind_layout_len) {
                 ^^^^

Is the bounds check correct in btf_check_meta()? The code checks whether
kind_layout[kind] exists by testing if (kind * elem_size < total_len).
This verifies the element starts within the buffer, but to actually read
the full element we need (kind + 1) * elem_size <= total_len.

For example, with kind_layout_len=41 bytes and elem_size=2 bytes:
- Check: kind=20 -> 20 * 2 < 41 -> 40 < 41 -> passes
- But reading kind_layout[20] needs bytes [40, 42) which exceeds the
  41-byte buffer

While both code paths still reject the BTF (just with different error
messages), the check allows kind_layout_len values that aren't proper
multiples of sizeof(struct btf_kind_layout) to be treated as if they
contain complete elements at the boundary.

The validation in btf_parse_kind_layout_sec() requires at least one
complete element (kind_layout_len >= sizeof(struct btf_kind_layout)) but
doesn't enforce that the length is a multiple of the element size, which
could lead to this off-by-one scenario.

> +		btf_verifier_log(env, "[%u] unknown but required kind %u",
> +				 env->log_type_id,
> +				 BTF_INFO_KIND(t->info));
>  		return -EINVAL;
> +	} else {
> +		if (BTF_INFO_KIND(t->info) > BTF_KIND_MAX) {
> +			btf_verifier_log(env, "[%u] Invalid kind:%u",
> +					 env->log_type_id, BTF_INFO_KIND(t->info));
> +			return -EINVAL;
> +		}
> +		var_meta_size = btf_type_ops(t)->check_meta(env, t, meta_left);
> +		if (var_meta_size < 0)
> +			return var_meta_size;
>  	}

[ ... ]



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

In-Reply-To-Subject: btf: support kernel parsing of BTF with kind layout
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20140850025

@kernel-patches-daemon-bpf
Copy link
Author

Forwarding comment 3642882607 via email
In-Reply-To: [email protected]
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

@kernel-patches-daemon-bpf
Copy link
Author

Forwarding comment 3642892492 via email
In-Reply-To: [email protected]
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

BTF kind layouts provide information to parse BTF kinds. By separating
parsing BTF from using all the information it provides, we allow BTF
to encode new features even if they cannot be used by readers. This
will be helpful in particular for cases where older tools are used
to parse newer BTF with kinds the older tools do not recognize;
the BTF can still be parsed in such cases using kind layout.

The intent is to support encoding of kind layouts optionally so that
tools like pahole can add this information. For each kind, we record

- length of singular element following struct btf_type
- length of each of the btf_vlen() elements following

The ideas here were discussed at [1], [2]; hence

Suggested-by: Andrii Nakryiko <[email protected]>
Signed-off-by: Alan Maguire <[email protected]>

[1] https://lore.kernel.org/bpf/CAEf4BzYjWHRdNNw4B=eOXOs_ONrDwrgX4bn=Nuc1g8JPFC34MA@mail.gmail.com/
[2] https://lore.kernel.org/bpf/[email protected]/
Support reading in kind layout fixing endian issues on reading;
also support writing kind layout section to raw BTF object.
There is not yet an API to populate the kind layout with meaningful
information.

As part of this, we need to consider multiple valid BTF header
sizes; the original or the kind layout-extended headers.
So to support this, the "struct btf" representation is modified
to always allocate a "struct btf_header" and copy the valid
portion from the raw data to it; this means we can always safely
check fields like btf->hdr->kind_layout_len.

Signed-off-by: Alan Maguire <[email protected]>
This allows BTF parsing to proceed even if we do not know the
kind.

Signed-off-by: Alan Maguire <[email protected]>
Support encoding of BTF kind layout data via btf__new_empty_opts().

Current supported opts are base_btf and add_kind_layout.

Kind layout information is maintained in btf.c in the
kind_layouts[] array; when BTF is created with the
add_kind_layout option it represents the current view
of supported BTF kinds.

Signed-off-by: Alan Maguire <[email protected]>
BTF parsing can use kind layout to navigate unknown kinds, so
btf_validate_type() should take kind layout information into
account to avoid failure when an unrecognized kind is met.

Signed-off-by: Alan Maguire <[email protected]>
Validate kind layout if present, but because the kernel must be
strict in what it accepts, reject BTF with unsupported kinds,
even if they are in the kind layout information.

Signed-off-by: Alan Maguire <[email protected]>
verify btf__new_empty_opts() adds kind layouts for all kinds supported,
and after adding kind-related types for an unknown kind, ensure that
parsing uses this info when that kind is encountered rather than
giving up.

Signed-off-by: Alan Maguire <[email protected]>
Provide a way to dump BTF metadata info via bpftool; this
consists of BTF size, header fields and kind layout info
(if available); for example

$ bpftool btf dump file vmlinux format meta
size 5460436
magic 0xeb9f
version 1
flags 0x0
hdr_len 32
type_len 3194640
type_off 0
str_len 2265721
str_off 3194640
kind_layout_len 40
kind_layout_off 5460364
kind 0    UNKNOWN    info_sz 0    elem_sz 0
kind 1    INT        info_sz 4    elem_sz 0
kind 2    PTR        info_sz 0    elem_sz 0
kind 3    ARRAY      info_sz 12   elem_sz 0
kind 4    STRUCT     info_sz 0    elem_sz 12
kind 5    UNION      info_sz 0    elem_sz 12
kind 6    ENUM       info_sz 0    elem_sz 8
kind 7    FWD        info_sz 0    elem_sz 0
kind 8    TYPEDEF    info_sz 0    elem_sz 0
kind 9    VOLATILE   info_sz 0    elem_sz 0
kind 10   CONST      info_sz 0    elem_sz 0
kind 11   RESTRICT   info_sz 0    elem_sz 0
kind 12   FUNC       info_sz 0    elem_sz 0
kind 13   FUNC_PROTO info_sz 0    elem_sz 8
kind 14   VAR        info_sz 4    elem_sz 0
kind 15   DATASEC    info_sz 0    elem_sz 12
kind 16   FLOAT      info_sz 0    elem_sz 0
kind 17   DECL_TAG   info_sz 4    elem_sz 0
kind 18   TYPE_TAG   info_sz 0    elem_sz 0
kind 19   ENUM64     info_sz 0    elem_sz 12

JSON output is also supported:

$ $ bpftool -p btf dump file /var/tmp/vmlinux.kind_layout format meta
{
    "size": 5460436,
    "header": {
        "magic": 60319,
        "version": 1,
        "flags": 0,
        "hdr_len": 32,
        "type_len": 3194640,
        "type_off": 0,
        "str_len": 2265721,
        "str_off": 3194640,
        "kind_layout_len": 40,
        "kind_layout_offset": 5460364
    },
    "kind_layouts": [{
            "kind": 0,
            "name": "UNKNOWN",
            "info_sz": 0,
            "elem_sz": 0
        },{
            "kind": 1,
            "name": "INT",
            "info_sz": 4,
            "elem_sz": 0
        },{
...

Signed-off-by: Alan Maguire <[email protected]>
...and provide an example of output.

Signed-off-by: Alan Maguire <[email protected]>
The "kind_layout" feature will add metadata about BTF kinds to the
generated BTF; its absence in pahole will not trigger an error so it
is safe to add unconditionally as it will simply be ignored if pahole
does not support it.

Signed-off-by: Alan Maguire <[email protected]>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 6f0b824
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1032375
version: 7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants