Skip to content

bpf, x86: fix per-CPU address resolution into an extended register

Vineet Gupta bpf sashiko-reviews stable 3 versions Applied

Status from this message: This series was applied to bpf/bpf-next.git (master)

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v3 Latest2 patches 4 repliesApplied 2/2
  2. v22 patches 4 repliesSuperseded
  3. v11 patch 2 repliesSuperseded

Changes in v3 from the cover letter

- Fold the five per-register programs into one that loads every register, and drop the comment explaining the register choice (Eduard Zingerman).
- Move the percpu_data declaration inside the arch guard, so other targets no longer carry a .percpu section and an unused map (bpf-ci).
- Match the movabsq of each address as well as the add, so the matchers stay on consecutive lines and the pair is checked to use the same register.
- Restore the Reviewed-by on patch 1, dropped by mistake in v2.

Patches in v3 All 2 applied

#Subject and filesSizeRepliesApplied
1/2bpf, x86: Fix per-CPU address resolution into an extended registerarch/x86/net/bpf_jit_comp.c+1 -10 repliesApplied
2/2selftests/bpf: Check per-CPU address resolution per registertools/testing/selftests/bpf/prog_tests/verifier.c tools/testing/selftests/bpf/progs/verifier_percpu_addr.c+74 -03 repliesApplied

Reviews trailers per patch

1 of 2 patches has a Reviewed-by or Acked-by.

PatchEduard Zingerman
1/2R-b
2/2

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v3 thread

Cover letter [PATCH bpf v3 0/2] bpf, x86: fix per-CPU address resolution into an extended register

The JIT resolves a per-CPU address with

add <dst>, gs:[this_cpu_off]

but builds the REX prefix with add_1mod(), which sets REX.B. The destination is encoded in ModRM.reg, which REX.R extends, and the memory operand is disp32 with no base, so REX.B does nothing and the high register bit is dropped. Every extended destination therefore resolves into whichever register shares the low three bits:

R5 -> RAX R7 -> RBP R8 -> RSI R9 -> RDI

The address is left unadjusted and an unrelated register is clobbered. Patch 1 switches to add_2mod() so the bit goes through REX.R.

Clang reloads the address into R1 before each per-CPU access, so the destination is never an extended register and the bug has been dormant since v6.10. GCC keeps several per-CPU addresses live at once, which is how it turned up: test_progs-bpf_gcc panics the kernel in global_percpu_data/init, with the address of a .percpu variable in R5.

Patch 2 covers every register. A functional test only catches this if the address happens to land in an extended register, so the test matches the JITed add instead.

Changes in v3:
- Fold the five per-register programs into one that loads every register, and drop the comment explaining the register choice (Eduard Zingerman).
- Move the percpu_data declaration inside the arch guard, so other targets no longer carry a .percpu section and an unused map (bpf-ci).
- Match the movabsq of each address as well as the add, so the matchers stay on consecutive lines and the pair is checked to use the same register.
- Restore the Reviewed-by on patch 1, dropped by mistake in v2.

Changes in v2:
- Add the selftest, patch 2/2 (Eduard Zingerman). It uses __jited() rather than __xlated(): the xlated stream is identical for every register, and the wrong prefix is only visible in the native encoding.
- No functional change to patch 1.

Vineet Gupta (2):
bpf, x86: Fix per-CPU address resolution into an extended register selftests/bpf: Check per-CPU address resolution per register

 arch/x86/net/bpf_jit_comp.c                   |  2 +-
 .../selftests/bpf/prog_tests/verifier.c       |  2 +
 .../bpf/progs/verifier_percpu_addr.c          | 72 +++++++++++++++++++
 3 files changed, 75 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_percpu_addr.c
Signature · 1 line
2.53.0-Meta