[PATCH bpf-next v3 0/3] bpf: Preserve pointer state for commuted arithmetic

Yiyang Chen posted 3 patches 2 days, 17 hours ago
kernel/bpf/verifier.c                         | 35 +++++++++-------
.../testing/selftests/bpf/progs/dynptr_fail.c | 31 ++++++++++++++
.../bpf/progs/mem_rdonly_untrusted.c          | 17 ++++++++
.../bpf/progs/verifier_basic_stack.c          | 41 +++++++++++++++++++
4 files changed, 110 insertions(+), 14 deletions(-)
[PATCH bpf-next v3 0/3] bpf: Preserve pointer state for commuted arithmetic
Posted by Yiyang Chen 2 days, 17 hours ago
This series fixes pointer-state propagation for commuted scalar += pointer
arithmetic in the verifier.

Patch 1 keeps the full pointer register state when the pointer operand is
the source of the add, which preserves fields such as the stack frame
number and parent id instead of copying only type and id.

Patch 2 builds on that state propagation and moves the untrusted PTR_TO_MEM
early return after it, so scalar += untrusted_pointer is modeled as
PTR_TO_MEM and remains usable through the probe-read path.

Patch 3 adds verifier selftests for stack frame number preservation,
readonly-untrusted memory access, and dynptr data-slice invalidation.

Changes in v3:
- Preserve the complete pointer register state with verifier-env scratch
  storage, addressing Eduard's comment that copying selected fields is
  fragile and avoiding a temporary bpf_reg_state on the verifier stack.
- Keep the existing RUN(verifier_basic_stack) dispatch unchanged and add the
  stack regression directly to the existing verifier_basic_stack program.
- Keep the original operand direction inside adjust_ptr_min_max_vals() by
  saving the scalar operand in env->fake_reg[0].
- Move untrusted PTR_TO_MEM handling after the unified pointer-state copy so
  the commuted form remains PTR_TO_MEM before the early return.
- Add readonly-untrusted and dynptr selftest coverage, responding to the
  bpf-ci/static review finding that the untrusted pointer case needs a
  regression test.
- Clear the original dynptr data-slice register after deriving the commuted
  alias so the regression test isolates parent-id propagation.
- Make the readonly-untrusted return value endian-neutral by loading an int.
- Rebase to bpf-next base a23a71823352.

v2: https://lore.kernel.org/bpf/cover.1784563950.git.chenyy23@mails.tsinghua.edu.cn/
v1: https://lore.kernel.org/bpf/cover.1784563939.git.chenyy23@mails.tsinghua.edu.cn/

Yiyang Chen (3):
  bpf: Preserve pointer state for commuted arithmetic
  bpf: Propagate untrusted pointer state in commuted arithmetic
  selftests/bpf: Cover commuted pointer state propagation

 kernel/bpf/verifier.c                         | 35 +++++++++-------
 .../testing/selftests/bpf/progs/dynptr_fail.c | 31 ++++++++++++++
 .../bpf/progs/mem_rdonly_untrusted.c          | 17 ++++++++
 .../bpf/progs/verifier_basic_stack.c          | 41 +++++++++++++++++++
 4 files changed, 110 insertions(+), 14 deletions(-)


base-commit: a23a71823352e2d792dcaae25f1ebb744acbfc0b
-- 
2.34.1
Re: [PATCH bpf-next v3 0/3] bpf: Preserve pointer state for commuted arithmetic
Posted by Daniel Wade 2 days, 11 hours ago
Hi Eduard, Yiyang,

Thanks for the pointer. I independently tested Yiyang's v3 series on the
source-matched f105f3631d51 base with the same kernel config used for my
original report.

Before the series, my impact witness reached chosen-address kernel
read/write and changed UID/GID 65534 with CAP_BPF and CAP_PERFMON to
UID/GID 0 in 3/3 runs. With all three v3 patches applied, the intended
untrusted-pointer positive control still passed, while the impact program
was rejected at "R0 pointer += pointer prohibited" before any address leak
or privilege transition in 3/3 runs.

I also ran the three focused cases added by the v3 series. All passed:

  mem_rdonly_untrusted/ldx_is_ok_commuted_addr
  dynptr/dynptr_slice_commuted_invalidate
  verifier_basic_stack/stack pointer arithmetic preserves frame number

Given that the affected code has shipped and the issue reaches kernel
read/write from a CAP_BPF and CAP_PERFMON context, should this be routed
through bpf and marked for stable rather than bpf-next?

For the v3 series:

Tested-by: Daniel Wade <danjwade95@gmail.com>

Regards,
Daniel