Re: [PATCH bpf-next v1 1/2] bpf: Preserve unique-field state across nested structs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] BPF_REFCOUNT is incorrectly marked as non-unique, allowing users to trigger a WARN_ON_ONCE via duplicate fields.
- [High] Typo in BPF_RES_SPIN_LOCK parsing checks the wrong offset, allowing users to trigger a WARN_ON_ONCE.
--

--- Patch [1]: [PATCH bpf-next v1 1/2] bpf: Preserve unique-field state across nested structs ---
commit cc8eb6296aa42dface6bb303fbfdec9c1e0e7566
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

bpf: Preserve unique-field state across nested structs

This commit addresses an issue where btf_find_struct_field() initialized a
fresh seen mask for every recursive descent. By sharing the seen_mask pointer
across the traversal, it properly detects duplicate unique fields to prevent
an invariant warning during map creation.

> btf_find_struct_field() initializes a fresh seen mask for every recursive
> descent. Unique special fields in different levels of the same aggregate
> therefore do not see one another. The duplicate fields can reach
> btf_parse_fields(), where they trigger an invariant WARN_ON_ONCE(). A
> crafted user BTF can consequently trigger the warning before map creation
> checks capabilities.

[Severity: High]
This isn't a bug introduced by this patch, but looking at
btf_get_field_type() in kernel/bpf/btf.c, BPF_REFCOUNT is defined as
non-unique (is_unique = false).

If a user creates a BPF map using a BTF struct containing two bpf_refcount
fields, they bypass the uniqueness check. When btf_parse_fields() processes
them, the second field triggers the invariant check:

kernel/bpf/btf.c:btf_parse_fields() {
    ...
    case BPF_REFCOUNT:
        WARN_ON_ONCE(rec->refcount_off >= 0);
    ...
}

Can a crafted user BTF still trigger this WARN_ON_ONCE() using duplicate
refcount fields?


[Severity: High]
This is a pre-existing issue, but looking at the BPF_RES_SPIN_LOCK case in
btf_parse_fields(), it appears to check the wrong offset:

kernel/bpf/btf.c:btf_parse_fields() {
    ...
    case BPF_RES_SPIN_LOCK:
        WARN_ON_ONCE(rec->spin_lock_off >= 0);
        /* Cache offset for faster lookup at runtime */
        rec->res_spin_lock_off = rec->fields[i].offset;
        break;
    ...
}

If a user provides a BTF struct containing both a bpf_spin_lock and
a bpf_res_spin_lock, wouldn't this trigger the WARN_ON_ONCE() before
reaching the intended graceful rejection?

kernel/bpf/btf.c:btf_parse_fields() {
    ...
    if (rec->spin_lock_off >= 0 && rec->res_spin_lock_off >= 0) {
        ret = -EINVAL;
        goto end;
    }
    ...
}

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index cbb1e49b9bcbf..8d628552ef99e 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -3751,7 +3751,7 @@ static int btf_repeat_fields(struct btf_field_info *info, int info_cnt,
>  static int btf_find_struct_field(const struct btf *btf,
>  				 const struct btf_type *t, u32 field_mask,
>  				 struct btf_field_info *info, int info_cnt,
> -				 u32 level);
> +				 u32 level, u32 *seen_mask);
>  
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719142401.2420111-1-memxor@gmail.com?part=1