| Message ID | fc4b1276eb10761fd7ce0814c8dd089da2815251.1633464148.git.naveen.n.rao@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | powerpc/bpf: Various fixes | expand |
| Related | show |
Le 05/10/2021 à 22:25, Naveen N. Rao a écrit : > We aren't handling subtraction involving an immediate value of > 0x80000000 properly. Fix the same. > > Fixes: 156d0e290e969c ("powerpc/ebpf/jit: Implement JIT compiler for extended BPF") > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> > --- > Changelog: > - Split up BPF_ADD and BPF_SUB cases per Christophe's comments > > arch/powerpc/net/bpf_jit_comp64.c | 27 +++++++++++++++++---------- > 1 file changed, 17 insertions(+), 10 deletions(-) > > diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c > index d67f6d62e2e1ff..6626e6c17d4ed2 100644 > --- a/arch/powerpc/net/bpf_jit_comp64.c > +++ b/arch/powerpc/net/bpf_jit_comp64.c > @@ -330,18 +330,25 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * > EMIT(PPC_RAW_SUB(dst_reg, dst_reg, src_reg)); > goto bpf_alu32_trunc; > case BPF_ALU | BPF_ADD | BPF_K: /* (u32) dst += (u32) imm */ > - case BPF_ALU | BPF_SUB | BPF_K: /* (u32) dst -= (u32) imm */ > case BPF_ALU64 | BPF_ADD | BPF_K: /* dst += imm */ > + if (!imm) { > + goto bpf_alu32_trunc; > + } else if (imm >= -32768 && imm < 32768) { > + EMIT(PPC_RAW_ADDI(dst_reg, dst_reg, IMM_L(imm))); > + } else { > + PPC_LI32(b2p[TMP_REG_1], imm); > + EMIT(PPC_RAW_ADD(dst_reg, dst_reg, b2p[TMP_REG_1])); > + } > + goto bpf_alu32_trunc; > + case BPF_ALU | BPF_SUB | BPF_K: /* (u32) dst -= (u32) imm */ > case BPF_ALU64 | BPF_SUB | BPF_K: /* dst -= imm */ > - if (BPF_OP(code) == BPF_SUB) > - imm = -imm; > - if (imm) { > - if (imm >= -32768 && imm < 32768) > - EMIT(PPC_RAW_ADDI(dst_reg, dst_reg, IMM_L(imm))); > - else { > - PPC_LI32(b2p[TMP_REG_1], imm); > - EMIT(PPC_RAW_ADD(dst_reg, dst_reg, b2p[TMP_REG_1])); > - } > + if (!imm) { > + goto bpf_alu32_trunc; > + } else if (imm > -32768 && imm < 32768) { Why do you exclude imm == 32768 ? Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu> > + EMIT(PPC_RAW_ADDI(dst_reg, dst_reg, IMM_L(-imm))); > + } else { > + PPC_LI32(b2p[TMP_REG_1], imm); > + EMIT(PPC_RAW_SUB(dst_reg, dst_reg, b2p[TMP_REG_1])); > } > goto bpf_alu32_trunc; > case BPF_ALU | BPF_MUL | BPF_X: /* (u32) dst *= (u32) src */ >
Christophe Leroy wrote: > > > Le 05/10/2021 à 22:25, Naveen N. Rao a écrit : >> We aren't handling subtraction involving an immediate value of >> 0x80000000 properly. Fix the same. >> >> Fixes: 156d0e290e969c ("powerpc/ebpf/jit: Implement JIT compiler for extended BPF") >> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> >> --- >> Changelog: >> - Split up BPF_ADD and BPF_SUB cases per Christophe's comments >> >> arch/powerpc/net/bpf_jit_comp64.c | 27 +++++++++++++++++---------- >> 1 file changed, 17 insertions(+), 10 deletions(-) >> >> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c >> index d67f6d62e2e1ff..6626e6c17d4ed2 100644 >> --- a/arch/powerpc/net/bpf_jit_comp64.c >> +++ b/arch/powerpc/net/bpf_jit_comp64.c >> @@ -330,18 +330,25 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * >> EMIT(PPC_RAW_SUB(dst_reg, dst_reg, src_reg)); >> goto bpf_alu32_trunc; >> case BPF_ALU | BPF_ADD | BPF_K: /* (u32) dst += (u32) imm */ >> - case BPF_ALU | BPF_SUB | BPF_K: /* (u32) dst -= (u32) imm */ >> case BPF_ALU64 | BPF_ADD | BPF_K: /* dst += imm */ >> + if (!imm) { >> + goto bpf_alu32_trunc; >> + } else if (imm >= -32768 && imm < 32768) { >> + EMIT(PPC_RAW_ADDI(dst_reg, dst_reg, IMM_L(imm))); >> + } else { >> + PPC_LI32(b2p[TMP_REG_1], imm); >> + EMIT(PPC_RAW_ADD(dst_reg, dst_reg, b2p[TMP_REG_1])); >> + } >> + goto bpf_alu32_trunc; >> + case BPF_ALU | BPF_SUB | BPF_K: /* (u32) dst -= (u32) imm */ >> case BPF_ALU64 | BPF_SUB | BPF_K: /* dst -= imm */ >> - if (BPF_OP(code) == BPF_SUB) >> - imm = -imm; >> - if (imm) { >> - if (imm >= -32768 && imm < 32768) >> - EMIT(PPC_RAW_ADDI(dst_reg, dst_reg, IMM_L(imm))); >> - else { >> - PPC_LI32(b2p[TMP_REG_1], imm); >> - EMIT(PPC_RAW_ADD(dst_reg, dst_reg, b2p[TMP_REG_1])); >> - } >> + if (!imm) { >> + goto bpf_alu32_trunc; >> + } else if (imm > -32768 && imm < 32768) { > > Why do you exclude imm == 32768 ? > > Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu> Good catch -- that was from an earlier version where this was shared across BPF_ADD and BPF_SUB. I missed updating this section before posting. Michael, please consider squashing in the below diff into this patch. Thanks! - Naveen --- diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c index f5a804d8c95bc1..0fdc1ff86e4f1c 100644 --- a/arch/powerpc/net/bpf_jit_comp64.c +++ b/arch/powerpc/net/bpf_jit_comp64.c @@ -368,7 +368,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * case BPF_ALU64 | BPF_SUB | BPF_K: /* dst -= imm */ if (!imm) { goto bpf_alu32_trunc; - } else if (imm > -32768 && imm < 32768) { + } else if (imm > -32768 && imm <= 32768) { EMIT(PPC_RAW_ADDI(dst_reg, dst_reg, IMM_L(-imm))); } else { PPC_LI32(b2p[TMP_REG_1], imm);
diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c index d67f6d62e2e1ff..6626e6c17d4ed2 100644 --- a/arch/powerpc/net/bpf_jit_comp64.c +++ b/arch/powerpc/net/bpf_jit_comp64.c @@ -330,18 +330,25 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context * EMIT(PPC_RAW_SUB(dst_reg, dst_reg, src_reg)); goto bpf_alu32_trunc; case BPF_ALU | BPF_ADD | BPF_K: /* (u32) dst += (u32) imm */ - case BPF_ALU | BPF_SUB | BPF_K: /* (u32) dst -= (u32) imm */ case BPF_ALU64 | BPF_ADD | BPF_K: /* dst += imm */ + if (!imm) { + goto bpf_alu32_trunc; + } else if (imm >= -32768 && imm < 32768) { + EMIT(PPC_RAW_ADDI(dst_reg, dst_reg, IMM_L(imm))); + } else { + PPC_LI32(b2p[TMP_REG_1], imm); + EMIT(PPC_RAW_ADD(dst_reg, dst_reg, b2p[TMP_REG_1])); + } + goto bpf_alu32_trunc; + case BPF_ALU | BPF_SUB | BPF_K: /* (u32) dst -= (u32) imm */ case BPF_ALU64 | BPF_SUB | BPF_K: /* dst -= imm */ - if (BPF_OP(code) == BPF_SUB) - imm = -imm; - if (imm) { - if (imm >= -32768 && imm < 32768) - EMIT(PPC_RAW_ADDI(dst_reg, dst_reg, IMM_L(imm))); - else { - PPC_LI32(b2p[TMP_REG_1], imm); - EMIT(PPC_RAW_ADD(dst_reg, dst_reg, b2p[TMP_REG_1])); - } + if (!imm) { + goto bpf_alu32_trunc; + } else if (imm > -32768 && imm < 32768) { + EMIT(PPC_RAW_ADDI(dst_reg, dst_reg, IMM_L(-imm))); + } else { + PPC_LI32(b2p[TMP_REG_1], imm); + EMIT(PPC_RAW_SUB(dst_reg, dst_reg, b2p[TMP_REG_1])); } goto bpf_alu32_trunc; case BPF_ALU | BPF_MUL | BPF_X: /* (u32) dst *= (u32) src */
We aren't handling subtraction involving an immediate value of 0x80000000 properly. Fix the same. Fixes: 156d0e290e969c ("powerpc/ebpf/jit: Implement JIT compiler for extended BPF") Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> --- Changelog: - Split up BPF_ADD and BPF_SUB cases per Christophe's comments arch/powerpc/net/bpf_jit_comp64.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-)