Re: [PATCH v7 4/5] bpf: allow using bpf_kptr_xchg even if the MEM_RCU flag is set

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 3181 bytes --]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -9300,6 +9300,7 @@
>  		PTR_TO_BTF_ID | MEM_PERCPU,
>  		PTR_TO_BTF_ID | MEM_PERCPU | MEM_RCU,
>  		PTR_TO_BTF_ID | MEM_PERCPU | PTR_TRUSTED,
> +		PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF | MEM_RCU,
>  	}
>  };

Was the new type added to the correct array?  This hunk adds the entry
to percpu_btf_ptr_types, which backs ARG_PTR_TO_PERCPU_BTF_ID in
compatible_reg_types[].  That array is used by bpf_per_cpu_ptr and
bpf_this_cpu_ptr, not by bpf_kptr_xchg.

For bpf_kptr_xchg arg1 (ARG_KPTR_XCHG_DEST), the verifier looks up
kptr_xchg_dest_types instead:

kernel/bpf/verifier.c:check_reg_type() {
    ...
    compatible = compatible_reg_types[base_type(arg_type)];
    ...
    for (i = 0; i < ARRAY_SIZE(compatible->types); i++) {
        expected = compatible->types[i];
        ...
        if (type == expected)
            goto found;
    }
    ...
    return -EACCES;

kptr_xchg_dest_types currently contains only:

static const struct bpf_reg_types kptr_xchg_dest_types = {
    .types = {
        PTR_TO_MAP_VALUE,
        PTR_TO_BTF_ID | MEM_ALLOC,
        PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF,
    }
};

A register with PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF | MEM_RCU will
not match any of these entries, so check_reg_type() returns -EACCES
before ever reaching the found: label and the switch statement below.

It looks like the entry should be added to kptr_xchg_dest_types rather
than percpu_btf_ptr_types.  Alexei made the same suggestion on v6 of
this series:

https://lore.kernel.org/bpf/CAADnVQJdwqeoqi+mPjQjmNbUqz2RxAjq-dVsGb7c_MCqpqmwhA@mail.gmail.com/

> @@ -9475,6 +9476,7 @@
>  	case PTR_TO_BTF_ID | MEM_ALLOC:
>  	case PTR_TO_BTF_ID | MEM_PERCPU | MEM_ALLOC:
>  	case PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF:
> +	case PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF | MEM_RCU:
>  		if (meta->func_id != BPF_FUNC_spin_lock && meta->func_id != BPF_FUNC_spin_unlock &&
>  		    meta->func_id != BPF_FUNC_kptr_xchg) {

This switch case is correct, but it can only be reached via the found:
label after the type-matching loop succeeds.  Since the first hunk
adds the type to the wrong array, the loop never matches for
bpf_kptr_xchg and this case is unreachable for its intended purpose.


---
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/22011467184

AI-authorship-score: low
AI-authorship-explanation: The patch went through 7 revisions on the mailing list with human reviewer interaction, and the type of mistake (adding to the wrong array) is characteristic of human error in a codebase with many similarly structured type arrays.
issues-found: 1
issue-severity-score: medium
issue-severity-explanation: The new type was added to percpu_btf_ptr_types instead of kptr_xchg_dest_types, so the patch does not achieve its stated goal of allowing bpf_kptr_xchg on MEM_RCU pointers; there is no user-visible crash or security issue, but the feature is silently broken.