Messages in this thread | | | Subject | Re: [PATCH 4/4] bpf powerpc: Add addr > TASK_SIZE_MAX explicit check | From | Ravi Bangoria <> | Date | Wed, 7 Jul 2021 09:36:33 +0530 |
| |
>> @@ -763,6 +771,14 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * >> /* dst = *(u16 *)(ul) (src + off) */ >> case BPF_LDX | BPF_MEM | BPF_H: >> case BPF_LDX | BPF_PROBE_MEM | BPF_H: >> + if (BPF_MODE(code) == BPF_PROBE_MEM) { >> + EMIT(PPC_RAW_ADDI(b2p[TMP_REG_1], src_reg, off)); >> + PPC_LI64(b2p[TMP_REG_2], TASK_SIZE_MAX); >> + EMIT(PPC_RAW_CMPLD(b2p[TMP_REG_1], b2p[TMP_REG_2])); >> + PPC_BCC(COND_GT, (ctx->idx + 4) * 4); >> + EMIT(PPC_RAW_XOR(dst_reg, dst_reg, dst_reg)); >> + PPC_JMP((ctx->idx + 2) * 4); >> + } > > That code seems strictly identical to the previous one and the next one. > Can you refactor in a function ?
I'll check this.
> >> EMIT(PPC_RAW_LHZ(dst_reg, src_reg, off)); >> if (insn_is_zext(&insn[i + 1])) >> addrs[++i] = ctx->idx * 4; >> @@ -773,6 +789,14 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * >> /* dst = *(u32 *)(ul) (src + off) */ >> case BPF_LDX | BPF_MEM | BPF_W: >> case BPF_LDX | BPF_PROBE_MEM | BPF_W: >> + if (BPF_MODE(code) == BPF_PROBE_MEM) { >> + EMIT(PPC_RAW_ADDI(b2p[TMP_REG_1], src_reg, off)); >> + PPC_LI64(b2p[TMP_REG_2], TASK_SIZE_MAX); >> + EMIT(PPC_RAW_CMPLD(b2p[TMP_REG_1], b2p[TMP_REG_2])); >> + PPC_BCC(COND_GT, (ctx->idx + 4) * 4); >> + EMIT(PPC_RAW_XOR(dst_reg, dst_reg, dst_reg)); >> + PPC_JMP((ctx->idx + 2) * 4); >> + } >> EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off)); >> if (insn_is_zext(&insn[i + 1])) >> addrs[++i] = ctx->idx * 4; >> @@ -783,6 +807,20 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * >> /* dst = *(u64 *)(ul) (src + off) */ >> case BPF_LDX | BPF_MEM | BPF_DW: >> case BPF_LDX | BPF_PROBE_MEM | BPF_DW: >> + if (BPF_MODE(code) == BPF_PROBE_MEM) { >> + EMIT(PPC_RAW_ADDI(b2p[TMP_REG_1], src_reg, off)); >> + PPC_LI64(b2p[TMP_REG_2], TASK_SIZE_MAX); >> + EMIT(PPC_RAW_CMPLD(b2p[TMP_REG_1], b2p[TMP_REG_2])); >> + if (off % 4) > > That test is worth a comment.
(off % 4) test is based on how PPC_BPF_LL() emits instruction.
> > And I'd prefer > > if (off & 3) { > PPC_BCC(COND_GT, (ctx->idx + 5) * 4); > EMIT(PPC_RAW_XOR(dst_reg, dst_reg, dst_reg)); > PPC_JMP((ctx->idx + 3) * 4); > } else { > PPC_BCC(COND_GT, (ctx->idx + 4) * 4); > EMIT(PPC_RAW_XOR(dst_reg, dst_reg, dst_reg)); > PPC_JMP((ctx->idx + 2) * 4); > }
Yes this is neat.
Thanks for the review, Ravi
| |