Re: [PATCH bpf-next v2 5/6] selftests/bpf: Add tests for pointer type merge at a shared load
| Newsgroups | org.kernel.vger.bpf |
|---|---|
| Message-ID | <51afcc2b1c0704ce7cec1e6ff7aa6423fee6b3afad0ceebc4a118828ea87586b@mail.kernel.org> |
> diff --git a/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c b/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c
> index b91271d4caa4..ac8bba3e7d02 100644
> --- a/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c
> +++ b/tools/testing/selftests/bpf/progs/mem_rdonly_untrusted.c
[ ... ]
> +SEC("socket")
> +__success
> +__log_level(2)
> +__msg("= *(u64 *)(r{{[0-9]}} +0){{.*}}=untrusted_ptr_sock")
> +__msg("= *(u64 *)(r{{[0-9]}} +0){{.*}}=ringbuf_mem")
> +__retval(0)
> +int mixed_mem_untrusted_btf_id_type(void *ctx)
[ ... ]
> +SEC("socket")
> +__success
> +__log_level(2)
> +__msg("= *(u32 *)(r{{[0-9]}} +0){{.*}}=ptr_nameidata")
> +__msg("= *(u32 *)(r{{[0-9]}} +0){{.*}}=ringbuf_mem")
> +__retval(0)
> +int mixed_mem_btf_id_type(void *ctx)
[ ... ]
> +SEC("socket")
> +__success
> +__log_level(2)
> +__msg("r8 = *(u32 *)(r7 +0){{.*}}R7=ptr_nameidata")
> +__msg("r8 = *(u32 *)(r7 +0){{.*}}R7=rdonly_mem")
> +__retval(0)
> +int mixed_rdonly_mem_btf_id_type(void *ctx)
[ ... ]
> +SEC("socket")
> +__success
> +__log_level(2)
> +__msg("= *(u64 *)(r{{[0-9]}} +0){{.*}}=ringbuf_mem")
> +__msg("= *(u64 *)(r{{[0-9]}} +0){{.*}}=rdonly_untrusted_mem")
> +__retval(0)
> +int mixed_mem_mem_type(void *ctx)
[ ... ]
> +SEC("socket")
> +__failure
> +__msg("same insn cannot be used with different pointers")
> +int mixed_map_value_mem_type(void *ctx)
[ ... ]
> +SEC("socket")
> +__failure
> +__msg("same insn cannot be used with different pointers")
> +int mixed_stack_mem_type(void *ctx)
[ ... ]
A subsystem pattern flags this as potentially concerning: these six
tests cover only two of the four commits in the series. The three merge
shapes listed in dea41ba6e67d's changelog map 1:1 onto the
mixed_mem_untrusted_btf_id_type, mixed_mem_btf_id_type, and
mixed_rdonly_mem_btf_id_type tests, and the mixed_mem_mem_type,
mixed_map_value_mem_type, and mixed_stack_mem_type tests cover the new
PTR_TO_MEM mismatch rule from fbffa57329b8.
However, the other two commits appear untested:
bcfd0d6a160f changed check_ptr_to_btf_access() from checking
type_flag(reg->type) & PTR_UNTRUSTED to calling
bpf_may_fault_on_deref(), so a BPF_STX through a bare PTR_TO_BTF_ID is
now rejected with "only read is supported". The changelog documents a
reachable bpf_qdisc panic (next = sch->next_sched; next->limit = 1000;),
but no __failure / __msg("only read is supported") case is added here or
in progs/bpf_qdisc_*.c. Only verifier_global_ptr_args.c and
verifier_netfilter_ctx.c assert that message, and both do so for
PTR_UNTRUSTED pointers that the old check already caught, so the
newly-closed hole is untested.
bef47cf76444 states that PTR_TO_BTF_ID|PTR_UNTRUSTED|MEM_ALLOC|NON_OWN_REF
and PTR_TO_BTF_ID|PTR_UNTRUSTED|MEM_RCU were reachable and left as plain
loads (two Fixes: tags). The added tests only produce
ringbuf/rdonly/bare-BTF-ID types, all of which the old exact-match list
in bpf_convert_ctx_accesses() already handled or which the merge path
covers.
Would it make sense to add tests for the bcfd0d6a160f store-rejection path
and the bef47cf76444 MEM_ALLOC|NON_OWN_REF and MEM_RCU cases here?
---
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/31845722500