From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Daney Subject: Re: [PATCH bpf-next 11/13] bpf, mips64: remove unneeded zero check from div/mod with k Date: Fri, 26 Jan 2018 14:36:24 -0800 Message-ID: References: <20180126223348.11250-1-daniel@iogearbox.net> <20180126223348.11250-12-daniel@iogearbox.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, David Daney To: Daniel Borkmann , ast@kernel.org Return-path: Received: from mail-cys01nam02on0045.outbound.protection.outlook.com ([104.47.37.45]:11421 "EHLO NAM02-CY1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751682AbeAZWgb (ORCPT ); Fri, 26 Jan 2018 17:36:31 -0500 In-Reply-To: <20180126223348.11250-12-daniel@iogearbox.net> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 01/26/2018 02:33 PM, Daniel Borkmann wrote: > The verifier in both cBPF and eBPF reject div/mod by 0 imm, > so this can never load. Remove emitting such test and reject > it from being JITed instead (the latter is actually also not > needed, but given practice in sparc64, ppc64 today, so > doesn't hurt to add it here either). > > Signed-off-by: Daniel Borkmann > Cc: David Daney This looks plausible, Reviewed-by: David Daney > --- > arch/mips/net/ebpf_jit.c | 19 ++++--------------- > 1 file changed, 4 insertions(+), 15 deletions(-) > > diff --git a/arch/mips/net/ebpf_jit.c b/arch/mips/net/ebpf_jit.c > index 296f1410..3e2798b 100644 > --- a/arch/mips/net/ebpf_jit.c > +++ b/arch/mips/net/ebpf_jit.c > @@ -741,16 +741,11 @@ static int build_one_insn(const struct bpf_insn *insn, struct jit_ctx *ctx, > break; > case BPF_ALU | BPF_DIV | BPF_K: /* ALU_IMM */ > case BPF_ALU | BPF_MOD | BPF_K: /* ALU_IMM */ > + if (insn->imm == 0) > + return -EINVAL; > dst = ebpf_to_mips_reg(ctx, insn, dst_reg); > if (dst < 0) > return dst; > - if (insn->imm == 0) { /* Div by zero */ > - b_off = b_imm(exit_idx, ctx); > - if (is_bad_offset(b_off)) > - return -E2BIG; > - emit_instr(ctx, beq, MIPS_R_ZERO, MIPS_R_ZERO, b_off); > - emit_instr(ctx, addu, MIPS_R_V0, MIPS_R_ZERO, MIPS_R_ZERO); > - } > td = get_reg_val_type(ctx, this_idx, insn->dst_reg); > if (td == REG_64BIT || td == REG_32BIT_ZERO_EX) > /* sign extend */ > @@ -770,19 +765,13 @@ static int build_one_insn(const struct bpf_insn *insn, struct jit_ctx *ctx, > break; > case BPF_ALU64 | BPF_DIV | BPF_K: /* ALU_IMM */ > case BPF_ALU64 | BPF_MOD | BPF_K: /* ALU_IMM */ > + if (insn->imm == 0) > + return -EINVAL; > dst = ebpf_to_mips_reg(ctx, insn, dst_reg); > if (dst < 0) > return dst; > - if (insn->imm == 0) { /* Div by zero */ > - b_off = b_imm(exit_idx, ctx); > - if (is_bad_offset(b_off)) > - return -E2BIG; > - emit_instr(ctx, beq, MIPS_R_ZERO, MIPS_R_ZERO, b_off); > - emit_instr(ctx, addu, MIPS_R_V0, MIPS_R_ZERO, MIPS_R_ZERO); > - } > if (get_reg_val_type(ctx, this_idx, insn->dst_reg) == REG_32BIT) > emit_instr(ctx, dinsu, dst, MIPS_R_ZERO, 32, 32); > - > if (insn->imm == 1) { > /* div by 1 is a nop, mod by 1 is zero */ > if (bpf_op == BPF_MOD) >