Re: [PATCH v4 bpf-next 11/11] Documentation/bpf: Describe new location-related BTF kinds

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] bpf: LOCSEC info.vlen 16-bit limit overflows for large ELF sections
- [Low] bpf: contradiction between LOCSEC name_off rule and example
- [Low] bpf: omitted BTF_LOC_PARAM_FBREG and register/offset layout from LOC_PARAM spec
- [Low] bpf: omitted LOC_PROTO vlen strict matching requirement
--

commit 2cc26415b9bc105cbaaa995ced64eca8f5ea9509
Author: Alan Maguire <alan.maguire@oracle.com>

Documentation/bpf: Describe new location-related BTF kinds

Update BTF specification to describe encoding schemes for
BTF_KIND_LOC_PARAM, BTF_KIND_LOC_PROTO and BTF_KIND_LOCSEC.

> diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
> index 004aa1058d857..29de1222c3e73 100644
> --- a/Documentation/bpf/btf.rst
> +++ b/Documentation/bpf/btf.rst

[ ... ]

> @@ -563,6 +568,80 @@ The ``btf_enum64`` encoding:

[ ... ]

> +2.2.20 BTF_KIND_LOC_PARAM
> +~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +``struct btf_type`` encoding requirement:
> +  * ``name_off``: 0
> +  * ``info.kind_flag``: 0
> +  * ``info.kind``: BTF_KIND_LOC_PARAM
> +  * ``info.vlen``: number of 32-bit location value words
> +  * ``size``: size in bytes of the represented parameter: 1 through 16
> +
> +``btf_type`` is followed by a ``struct btf_loc_param`` and ``info.vlen``
> +number of 32-bit value words.::
> +
> +    struct btf_loc_param {
> +        __u32 flags;
> +        __u32 values[];
> +    };
> +
> +The ``flags`` field describes how to interpret ``values``:
> +
> +  * ``BTF_LOC_PARAM_CONST`` describes a constant; the value is stored in
> +    low-word, high-word order when it requires 64 bits.
> +  * ``BTF_LOC_PARAM_ADDR | BTF_LOC_PARAM_CONST`` describes an address offset
> +    relative to the runtime base address of the kernel or module image.
> +  * ``BTF_LOC_PARAM_REG`` with one word describes a register number; with two
> +    words it describes a multi-register parameter.
> +  * ``BTF_LOC_PARAM_REG | BTF_LOC_PARAM_OFFSET`` describes an address held in
> +    a register plus an offset. Adding ``BTF_LOC_PARAM_DEREF`` dereferences

[Severity: Low]
Does this documentation need to clarify the layout of the values array for
this flag combination? It isn't clear that the register number occupies the
first 4-byte value while the offset occupies the remainder.

Also, does this section need to document the BTF_LOC_PARAM_FBREG (0xffffffff)
reserved value defined in the UAPI header? Implementers will be unable to
correctly decode or encode these values without these details.

> +    that address. ``BTF_LOC_PARAM_REG | BTF_LOC_PARAM_DEREF`` with one word
> +    dereferences the value held in the register.
> +  * ``BTF_LOC_PARAM_SIGNED`` makes a constant or offset signed. A constant's
> +    signed width is ``size``. For a register-relative offset, its signed
> +    width is the number of offset value words times 32 bits.

[ ... ]

> +2.2.22 BTF_KIND_LOCSEC
> +~~~~~~~~~~~~~~~~~~~~~~
> +
> +``struct btf_type`` encoding requirement:
> +  * ``name_off``: offset to a valid ELF section name
> +  * ``info.kind_flag``: 0
> +  * ``info.kind``: BTF_KIND_LOCSEC
> +  * ``info.vlen``: number of inline sites in the section

[Severity: Medium]
Will this 16-bit info.vlen field overflow for large ELF sections?

The info.vlen field is structurally 16 bits, allowing a maximum of 65,535
inline sites per LOCSEC record. Large sections like the vmlinux .text section
contain significantly more inline sites, which will cause an overflow during
BTF generation.

Should the unused 32-bit size field be used for the site count instead, or
should the specification document how to split a large section across multiple
LOCSEC records?

> +  * ``type``: 0
> +
> +``btf_type`` is followed by ``info.vlen`` number of ``struct btf_loc``.::
> +
> +    struct btf_loc {
> +        __u32 func;
> +        __u32 loc_proto;
> +        __u32 offset;
> +    };
> +
> +The ``func`` field is the non-zero type ID of the ``BTF_KIND_FUNC`` being
> +described. ``loc_proto`` is the non-zero type ID of the associated
> +``BTF_KIND_LOC_PROTO``. ``offset`` is the location address offset relative to

[Severity: Low]
Should this document the strict structural length matching required by the
kernel verifier?

The kernel BTF verifier in btf_locsec_resolve() strictly requires that the
vlen of a BTF_KIND_LOC_PROTO matches the vlen of its associated
BTF_KIND_FUNC_PROTO. Missing this rule could lead tooling to generate
valid-looking BTF that the kernel will subsequently reject with -EINVAL.

> +the runtime base address of the ELF section associated with the LOCSEC.
> +For example, a LOCSEC named ``inline.text`` contains records for ``.text``
> +whose offsets are relative to the runtime base of that section.

[Severity: Low]
Is this example contradicting the name_off requirement?

The documentation above requires name_off to point to a valid ELF section
name, but this example uses "inline.text", creating ambiguity for
implementers parsing the BTF to map LOCSEC records to actual ELF sections.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924111428.75957-1-alan.maguire@oracle.com?part=11