check_atomic_rmw() open codes the mapping from a BPF_ATOMIC to the register it reads the old value into, the BPF_STX case of insn_def_regno() open codes the very same mapping a second time, the const folding and the liveness transfer functions a third and a fourth time, and BPF JITs need it as well to know which register a faulting BPF_PROBE_ATOMIC has to clear. Add a small helper so that all of them can share it. No functional change. The BPF_LOAD_ACQ case is there for the JITs, which do walk all instruction classes. const_reg_xfer() loses its explicit BPF_ATOMIC mode test since the helper checks class and mode itself; the BPF_PROBE_ATOMIC it additionally accepts cannot be seen there as it is only set from bpf_do_misc_fixups(), that is, after const folding has run. arg_track_xfer() keeps its mode test since that also guards the stack clearing next to it.
Unified · Split
| @@ -414,6 +414,30 @@ static inline bool bpf_atomic_is_load_acq(const struct bpf_insn *insn) · 413 unmodified lines | ||
| 414 | 414 | insn->imm == BPF_LOAD_ACQ; |
| 415 | 415 | } |
| 416 | 416 | |
| 417 | +/* | |
| 418 | + * Given an instruction @insn, return the number of the BPF register that a | |
| 419 | + * BPF_ATOMIC reads the value at its memory operand into, or -1 if there is | |
| 420 | + * no such register. That is the register a BPF_PROBE_ATOMIC has to clear when | |
| 421 | + * the access faults. Like bpf_atomic_is_load_acq(), @insn is not assumed to | |
| 422 | + * be a BPF_ATOMIC here. | |
| 423 | + */ | |
| 424 | +static inline int bpf_atomic_load_reg(const struct bpf_insn *insn) | |
| 425 | +{ | |
| 426 | + if (BPF_CLASS(insn->code) != BPF_STX || | |
| 427 | + (BPF_MODE(insn->code) != BPF_ATOMIC && | |
| 428 | + BPF_MODE(insn->code) != BPF_PROBE_ATOMIC)) | |
| 429 | + return -1; | |
| 430 | + | |
| 431 | + switch (insn->imm) { | |
| 432 | + case BPF_LOAD_ACQ: | |
| 433 | + return insn->dst_reg; | |
| 434 | + case BPF_CMPXCHG: | |
| 435 | + return BPF_REG_0; | |
| 436 | + default: | |
| 437 | + return (insn->imm & BPF_FETCH) ? insn->src_reg : -1; | |
| 438 | + } | |
| 439 | +} | |
| 440 | + | |
| 417 | 441 | /* Memory store, *(uint *) (dst_reg + off16) = imm32 */ |
| 418 | 442 | |
| 419 | 443 | #define BPF_ST_MEM(SIZE, DST, OFF, IMM) \ |
| @@ -199,14 +199,9 @@ process_call: · 198 unmodified lines | ||
| 199 | 199 | ci_out[r] = unknown; |
| 200 | 200 | break; |
| 201 | 201 | case BPF_STX: |
| 202 | − if (mode != BPF_ATOMIC) | |
| 203 | − break; | |
| 204 | − if (insn->imm == BPF_CMPXCHG) | |
| 205 | − ci_out[BPF_REG_0] = unknown; | |
| 206 | − else if (insn->imm == BPF_LOAD_ACQ) | |
| 207 | − *dst = unknown; | |
| 208 | − else if (insn->imm & BPF_FETCH) | |
| 209 | − *src = unknown; | |
| 202 | + r = bpf_atomic_load_reg(insn); | |
| 203 | + if (r >= 0) | |
| 204 | + ci_out[r] = unknown; | |
| 210 | 205 | break; |
| 211 | 206 | } |
| 212 | 207 | } |
| @@ -49,16 +49,7 @@ static int insn_def_regno(const struct bpf_insn *insn) · 48 unmodified lines | ||
| 49 | 49 | case BPF_ST: |
| 50 | 50 | return -1; |
| 51 | 51 | case BPF_STX: |
| 52 | − if (BPF_MODE(insn->code) == BPF_ATOMIC || | |
| 53 | − BPF_MODE(insn->code) == BPF_PROBE_ATOMIC) { | |
| 54 | − if (insn->imm == BPF_CMPXCHG) | |
| 55 | − return BPF_REG_0; | |
| 56 | − else if (insn->imm == BPF_LOAD_ACQ) | |
| 57 | − return insn->dst_reg; | |
| 58 | − else if (insn->imm & BPF_FETCH) | |
| 59 | − return insn->src_reg; | |
| 60 | − } | |
| 61 | − return -1; | |
| 52 | + return bpf_atomic_load_reg(insn); | |
| 62 | 53 | default: |
| 63 | 54 | return insn->dst_reg; |
| 64 | 55 | } |
| @@ -1209,12 +1209,9 @@ static void arg_track_xfer(struct bpf_verifier_env *env, struct bpf_insn *insn, · 1208 unmodified lines | ||
| 1209 | 1209 | clear_stack_for_all_offs(insn, at_out, insn->dst_reg, |
| 1210 | 1210 | at_stack_out, sz); |
| 1211 | 1211 | |
| 1212 | − if (insn->imm == BPF_CMPXCHG) | |
| 1213 | − at_out[BPF_REG_0] = none; | |
| 1214 | − else if (insn->imm == BPF_LOAD_ACQ) | |
| 1215 | − *dst = none; | |
| 1216 | − else if (insn->imm & BPF_FETCH) | |
| 1217 | − *src = none; | |
| 1212 | + r = bpf_atomic_load_reg(insn); | |
| 1213 | + if (r >= 0) | |
| 1214 | + at_out[r] = none; | |
| 1218 | 1215 | } |
| 1219 | 1216 | } else if (class == BPF_ST && BPF_MODE(insn->code) == BPF_MEM) { |
| 1220 | 1217 | u32 sz = bpf_size_to_bytes(BPF_SIZE(insn->code)); |
| @@ -6485,21 +6485,12 @@ static int check_atomic_rmw(struct bpf_verifier_env *env, · 6484 unmodified lines | ||
| 6485 | 6485 | return -EACCES; |
| 6486 | 6486 | } |
| 6487 | 6487 | |
| 6488 | − if (insn->imm & BPF_FETCH) { | |
| 6489 | − if (insn->imm == BPF_CMPXCHG) | |
| 6490 | − load_reg = BPF_REG_0; | |
| 6491 | − else | |
| 6492 | − load_reg = insn->src_reg; | |
| 6493 | − | |
| 6488 | + load_reg = bpf_atomic_load_reg(insn); | |
| 6489 | + if (load_reg >= 0) { | |
| 6494 | 6490 | /* check and record load of old value */ |
| 6495 | 6491 | err = check_reg_arg(env, load_reg, DST_OP); |
| 6496 | 6492 | if (err) |
| 6497 | 6493 | return err; |
| 6498 | − } else { | |
| 6499 | − /* This instruction accesses a memory location but doesn't | |
| 6500 | − * actually load it into a register. | |
| 6501 | − */ | |
| 6502 | − load_reg = -1; | |
| 6503 | 6494 | } |
| 6504 | 6495 | |
| 6505 | 6496 | dst_reg = cur_regs(env) + insn->dst_reg; |