> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index ddd0f4f32d24..a4412bc16688 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -559,7 +574,7 @@ struct btf_field_desc {
> /* member struct size, or zero, if no members */
> int m_sz;
> /* repeated per-member offsets */
> - int m_off_cnt, m_offs[1];
> + int m_off_cnt, m_offs[2];
> };
Eduard Zingerman noted in the RFC v1 review that this change to m_offs[2]
should be moved to patch #2, since that patch's commit message explains
why it is needed. The change was acknowledged but not addressed in v2.
See:
https://lore.kernel.org/bpf/[email protected]/
> diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
> index 618167cab4e6..6062c9958034 100644
> --- a/include/uapi/linux/btf.h
> +++ b/include/uapi/linux/btf.h
[ ... ]
> +/*
> + * The combination of size, vlen and flags gives us the means to interpret
> + * the following vlen-specified set of 4-byte values:
> + *
> + * - a BTF_LOC_PARAM_CONST is a constant value; combination
> + * of size, vlen and _SIGNED flag determines it. If the value requires
> + * 64 bits it is stored in {lo,hi} order.
> + * - a BTF_LOC_PARAM_ADDR is an address that will be normalized with
> + * respect to kernel base address.
> + * - a BTF_LOC_PARAM_REG with vlen 1 is a simple register number;
> + * with vlen 2 it is a multi-register parameter.
> + * - a _REG | DEREF with vlen 1 dereferences the value in the register
> + * number specified.
> + * - a REG | DEREF | OFFSET with vlen specifies the register value in
> + * the first 4-byte value and the offset in the remainder.
> + * - binary logical operators operate on a combination of register
> + * number and constant value, aside from _NOT which operates on
> + * a register
> + */
> +enum btf_loc_param_flags {
> + BTF_LOC_PARAM_SIGNED = 0x1,
> + BTF_LOC_PARAM_CONST = 0x2,
> + BTF_LOC_PARAM_ADDR = 0x4,
> + BTF_LOC_PARAM_REG = 0x8,
> + BTF_LOC_PARAM_DEREF = 0x10,
> + BTF_LOC_PARAM_OFFSET = 0x20,
> +};
Does the comment above the enum match the actual flags? The text mentions
"binary logical operators" and "_NOT which operates on a register," but
the enum defines only SIGNED, CONST, ADDR, REG, DEREF and OFFSET. Where
is BTF_LOC_PARAM_NOT and where are the logical operator flags?
This same mismatch appears in tools/include/uapi/linux/btf.h as well.
> +/*
> + * BTF_KIND_LOC_PROTO specifies location prototypes; i.e. how locations
> relate
> + * to parameters; a struct btf_type of BTF_KIND_LOC_PROTO is followed by a
> + * a vlen-specified number of __u32 BTF type ids which specify the associated
> + * BTF_KIND_LOC_PARAM for each function parameter associated with the
> + * location. The type should either be 0 (no location info) or point at
> + * a BTF_KIND_LOC_PARAM.
> + */
This isn't a bug, but there's a duplicated word: "followed by a / a
vlen-specified number". The same duplication appears in the tools copy at
tools/include/uapi/linux/btf.h.
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 9c2cab08bb79..d74c8668aa3f 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
[ ... ]
> +static s32 btf_loc_param_check_meta(struct btf_verifier_env *env,
> + const struct btf_type *t,
> + u32 meta_left)
> +{
> + const struct btf_loc_param *p = btf_loc_param(t);
> + u32 meta_needed;
> + u32 size;
> +
> + meta_needed = sizeof(*p) + sizeof(__u32) * btf_vlen(t);
> + if (meta_left < meta_needed) {
> + btf_verifier_log_basic(env, t,
> + "meta_left:%u meta_needed:%u",
> + meta_left, meta_needed);
> + return -EINVAL;
> + }
> +
> + if (t->name_off) {
> + btf_verifier_log_type(env, t, "Invalid name");
> + return -EINVAL;
> + }
> + size = t->size;
> + if (size > 16 || !is_power_of_2(size)) {
> + btf_verifier_log_type(env, t, "Unexpected size");
> + return -EINVAL;
> + }
> +
> + return meta_needed;
> +}
Does this handler miss the trailing btf_verifier_log_type(env, t, NULL)
call before returning successfully? Looking at __btf_verifier_log_type()
in this file, the comment states: "btf verifier prints all types it is
processing via btf_verifier_log_type(..., fmt = NULL)". Every other
check_meta handler in kernel/bpf/btf.c ends with that call on the
success path (int, ptr, array, struct, enum, enum64, fwd, typedef,
func_proto, func, var, datasec, decl_tag, float), but
btf_loc_param_check_meta, btf_loc_proto_check_meta, and
btf_locsec_check_meta all return successfully without it.
When userspace passes a log buffer to BPF_BTF_LOAD, wouldn't the per-type
"[id] KIND name ..." trace skip every LOC_PARAM/LOC_PROTO/LOCSEC type,
creating gaps in the [id] sequence? Since env->log_type_id advances
inside btf_verifier_log_type() and not on the successful path here, could
later error messages for subsequent types be attributed to the wrong id?
> +static void btf_locsec_log(struct btf_verifier_env *env,
> + const struct btf_type *t)
> +{
> + const struct btf_loc *loc = btf_type_loc_secinfo(t);
> + u32 nr_locs = btf_type_vlen(t), i;
> + const struct btf *btf = env->btf;
> +
> + btf_verifier_log(env, "locsec %s locs=(",
> + __btf_name_by_offset(btf, t->name_off));
> + for (i = 0; i < nr_locs; i++, loc++) {
> + btf_verifier_log(env, "\n\tfunc %u loc_proto %u offset 0x%x%s",
> + loc->func, loc->loc_proto, loc->offset,
> + i + 1 == nr_locs ? ")" : ", ");
> + }
> +}
A subsystem pattern flags this as potentially concerning: the log_details
callback re-prints the kind and name that __btf_verifier_log_type() has
already emitted, so the output would read "[7] LOCSEC name locsec name
locs=(...)". Every other log_details handler (btf_datasec_log,
btf_struct_log, etc.) prints only the kind-specific tail.
Also, if vlen is zero, does the loop body that would print the closing
parenthesis ever run? A zero-vlen LOCSEC would pass check_meta and leave
a dangling "(" in the log. The same pattern appears in btf_loc_proto_log
and btf_loc_param_log.
---
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/33537080133
