On Mon, Jun 23, 2014 at 2:38 AM, Markos Chandras <markos.chandras@xxxxxxxxxx> wrote: > Previously, the negative offset was not checked leading to failures > due to trying to load data beyond the skb struct boundaries. Until we > have proper asm helpers in place, it's best if we return ENOSUPP if K > is negative when trying to JIT the filter or 0 during runtime if we > do an indirect load where the value of X is unknown during build time. > > Cc: "David S. Miller" <davem@xxxxxxxxxxxxx> > Cc: Daniel Borkmann <dborkman@xxxxxxxxxx> > Cc: Alexei Starovoitov <ast@xxxxxxxxxxxx> > Cc: netdev@xxxxxxxxxxxxxxx > Signed-off-by: Markos Chandras <markos.chandras@xxxxxxxxxx> Hi Markos, thank you for addressing all of my earlier comments. Looks like test_bpf was quite useful in finding all of these bugs :) For the patches that reached netdev: Acked-by: Alexei Starovoitov <ast@xxxxxxxxxxxx> One minor nit below: > --- > arch/mips/net/bpf_jit.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/arch/mips/net/bpf_jit.c b/arch/mips/net/bpf_jit.c > index 5cc92c4590cb..95728ea6cb74 100644 > --- a/arch/mips/net/bpf_jit.c > +++ b/arch/mips/net/bpf_jit.c > @@ -331,6 +331,12 @@ static inline void emit_srl(unsigned int dst, unsigned int src, > emit_instr(ctx, srl, dst, src, sa); > } > > +static inline void emit_slt(unsigned int dst, unsigned int src1, > + unsigned int src2, struct jit_ctx *ctx) > +{ > + emit_instr(ctx, slt, dst, src1, src2); > +} > + > static inline void emit_sltu(unsigned int dst, unsigned int src1, > unsigned int src2, struct jit_ctx *ctx) > { > @@ -816,8 +822,21 @@ static int build_body(struct jit_ctx *ctx) > /* A <- P[k:1] */ > load_order = 0; > load: > + /* the interpreter will deal with the negative K */ > + if ((int)k < 0) should be a space after cast.