Re: [PATCH bpf-next 1/4] bpf: Keep fault protection when merging pointer types

commit e49db69f3fc735b4b80a6b4b8a50b41285d42c00
Author: Daniel Borkmann <[email protected]>

bpf: Keep fault protection when merging pointer types

When the same BPF_LDX instruction is reached through paths that yield
different pointer types, save_aux_ptr_type() merges them into a single
type which is later used by bpf_convert_ctx_accesses() to decide whether
the load has to be rewritten into a BPF_PROBE_MEM one.

Before commit f2362a57aeff ("bpf: allow void* cast using bpf_rdonly_cast()")
the merge only accepted two PTR_TO_BTF_ID pointers and unconditionally
fell back to PTR_TO_BTF_ID | PTR_UNTRUSTED, so the merged type was always
one that gets the BPF_PROBE_MEM rewrite. However, the mentioned commit
widened the merge to also cover a PTR_TO_MEM base and replaced the
fallback by a union of the PTR_UNTRUSTED and MEM_RDONLY flags.

The union can produce types which bpf_convert_ctx_accesses() does not
rewrite, and the load then stays a plain one without an exception table
entry, e.g.:

  - PTR_TO_MEM merged with PTR_TO_BTF_ID | PTR_UNTRUSTED
   => PTR_TO_MEM | PTR_UNTRUSTED but only the MEM_RDONLY variant is valid
  - PTR_TO_MEM merged with a plain PTR_TO_BTF_ID
   => PTR_TO_MEM dropping the rewrite the latter type would have gotten
  - PTR_TO_MEM | MEM_RDONLY merged with a plain PTR_TO_BTF_ID
   => PTR_TO_MEM | MEM_RDONLY which is not rewritten either since only
    its PTR_UNTRUSTED variant is

In all three cases a program can take the unsafe path at runtime with a
NULL or otherwise bad pointer and panic the kernel on the faulting load.
Fix it by normalizing the merged type: if either side needs the rewrite,
pick the one rewritten form the merged base type has.

Reuse the may_fault_on_deref() helper in is_load_acq_unsafe() as well to
avoid open coding, and trim the overly verbose comment which is more of
implementation detail of bpf_convert_ctx_accesses() anyway.

This isn't a bug, but there are a few readability issues in the changelog.
The sentence "pick the one rewritten form the merged base type has" is
hard to parse - could it be reworded to something like "pick the rewritten
form that exists for the merged base type"? Also, "which is more of
implementation detail of" is missing an article. Finally, the continuation
lines in the ASCII list use 3-space indentation under 2-space bullets, so
the '=>' lines don't align with the bullet text above them.


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