[PATCH bpf-next v2 3/6] bpf, x86: Fix exception table metadata for arena load-acquire

[Daniel Borkmann <daniel@iogearbox.net>]

A load-acquire from an arena pointer is converted to BPF_PROBE_ATOMIC and
gets an exception table entry, but the entry is filled in as if it were a
store, since populate_extable() decides based on instruction class alone
and a load-acquire is of BPF_STX class:

  if (BPF_CLASS(insn->code) == BPF_LDX) {
      arena_reg = reg2pt_regs[src_reg];
      fixup_reg = reg2pt_regs[dst_reg];
  } else {
      arena_reg = reg2pt_regs[dst_reg];
      fixup_reg = DONT_CLEAR;
  }

For a load-acquire dst_reg holds the loaded value and src_reg holds the
address, so both assignments in the else branch are wrong. On a fault
over an unmapped arena page ex_handler_bpf() then:

  - computes the reported address from the value register instead
    of the address register
  - reports the access as a WRITE, since it derives the direction
    from fixup_reg == DONT_CLEAR
  - leaves dst_reg untouched, so the program continues with a stale
    value instead of the 0 that BPF_PROBE_* loads deliver

The access itself is emitted correctly, emit_atomic_ld_st_index() uses
src_reg as the address, so this is a broken probe contract and a wrong
diagnostic rather than a memory safety issue.

Use bpf_atomic_is_load_acq() helper so a load-acquire takes the load path.

Fixes: 5341c9a4d833 ("bpf, x86: Support load-acquire and store-release instructions")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Peilin Ye <yepeilin@google.com>
---
 arch/x86/net/bpf_jit_comp.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 01e7ce569c1e..88ed95b2eaa7 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -2331,8 +2331,13 @@ st:			insn_off = insn->off;
 				 * BPF_PROBE_ATOMIC) before being used for the memory access. Pass
 				 * the reg holding the unmodified 32-bit address to
 				 * ex_handler_bpf().
+				 *
+				 * A load-acquire is of BPF_STX class, but reads from src_reg
+				 * into dst_reg like a BPF_LDX does, hence it must not be
+				 * treated as a store here.
 				 */
-				if (BPF_CLASS(insn->code) == BPF_LDX) {
+				if (BPF_CLASS(insn->code) == BPF_LDX ||
+				    bpf_atomic_is_load_acq(insn)) {
 					arena_reg = reg2pt_regs[src_reg];
 					fixup_reg = reg2pt_regs[dst_reg];
 				} else {
-- 
2.43.0