[REGRESSION,v1] bpf jit: Let the arm jit handle negative memory references

The arm jit has the same problem as the other two jits.
It only tests for negative absolute memory references, and if it sees
one bails out to let the interpreter handle the filter program.

But this fails if the X register contains a negative memory reference
and is used for an indirect load.
This is only caught at runtime, where the filter will always drop
the packet.
Filter which work fine with the interpreter do not work with the jit.

So this patch tries to fix it for the arm jit.

First we add the prototype of bpf_internal_load_pointer_neg_helper to
filter.h, since the arm jit has no assembler component, instead it has
to be used from C.

Then the helper functions are prepared to either handle known positive
offsets, known negative offsets, or any offset and test at runtime.

Finally the compiler is modified to emit calls to the right function,
depending if either the offset is known, or not.

This fixes the case of a negative X register and allows to lift
the restriction that bpf programs with negative offsets can't
be jited.

Signed-off-by: Jan Seiffert <kaffeemonster@googlemail.com>
---
The arm jit structure is a little bit different then the other jits, esp.
it does not use assembler load helper. So i had to put the prototype for
bpf_internal_load_pointer_neg_helper somewhere.

This is a v1 to keep the ball rolling.

Testing would also be cool, -ENOHARDWARE.
 
 arch/arm/net/bpf_jit_32.c |  149 ++++++++++++++++++++++++++++++++-------------
 include/linux/filter.h    |    6 ++
 net/core/filter.c         |    4 +
 3 files changed, 117 insertions(+), 42 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi,

Am 06.04.2012 20:57, schrieb Jan Seiffert:
> The arm jit has the same problem as the other two jits.
> It only tests for negative absolute memory references, and if it sees
> one bails out to let the interpreter handle the filter program.
> 
> But this fails if the X register contains a negative memory reference
> and is used for an indirect load.

I don't think that there's any use case for negative values in X. In
both the original BPF design and in the LSF interpreter, A and X are
considered unsigned. The role of X is mainly to allow variable length
headers (load the length -> unsigned).

"Negative" K values are permitted for ancillary data loads based on the
fact that we're not going to see 2GB packets any time soon.

Mircea

> This is only caught at runtime, where the filter will always drop
> the packet.
> Filter which work fine with the interpreter do not work with the jit.
> 
> So this patch tries to fix it for the arm jit.
> 
> First we add the prototype of bpf_internal_load_pointer_neg_helper to
> filter.h, since the arm jit has no assembler component, instead it has
> to be used from C.
> 
> Then the helper functions are prepared to either handle known positive
> offsets, known negative offsets, or any offset and test at runtime.
> 
> Finally the compiler is modified to emit calls to the right function,
> depending if either the offset is known, or not.
> 
> This fixes the case of a negative X register and allows to lift
> the restriction that bpf programs with negative offsets can't
> be jited.
> 
> Signed-off-by: Jan Seiffert <kaffeemonster@googlemail.com>
> ---
> The arm jit structure is a little bit different then the other jits, esp.
> it does not use assembler load helper. So i had to put the prototype for
> bpf_internal_load_pointer_neg_helper somewhere.
> 
> This is a v1 to keep the ball rolling.
> 
> Testing would also be cool, -ENOHARDWARE.
>  
>  arch/arm/net/bpf_jit_32.c |  149 ++++++++++++++++++++++++++++++++-------------
>  include/linux/filter.h    |    6 ++
>  net/core/filter.c         |    4 +
>  3 files changed, 117 insertions(+), 42 deletions(-)
> 
> diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
> index 62135849..19d60af 100644
> --- a/arch/arm/net/bpf_jit_32.c
> +++ b/arch/arm/net/bpf_jit_32.c
> @@ -18,6 +18,7 @@
>  #include <linux/slab.h>
>  #include <asm/cacheflush.h>
>  #include <asm/hwcap.h>
> +#include <asm/unaligned.h>
>  
>  #include "bpf_jit_32.h"
>  
> @@ -71,7 +72,7 @@ struct jit_ctx {
>  
>  int bpf_jit_enable __read_mostly;
>  
> -static u64 jit_get_skb_b(struct sk_buff *skb, unsigned offset)
> +static u64 jit_get_skb_b_pos(struct sk_buff *skb, unsigned offset)
>  {
>  	u8 ret;
>  	int err;
> @@ -81,7 +82,7 @@ static u64 jit_get_skb_b(struct sk_buff *skb, unsigned offset)
>  	return (u64)err << 32 | ret;
>  }
>  
> -static u64 jit_get_skb_h(struct sk_buff *skb, unsigned offset)
> +static u64 jit_get_skb_h_pos(struct sk_buff *skb, unsigned offset)
>  {
>  	u16 ret;
>  	int err;
> @@ -91,7 +92,7 @@ static u64 jit_get_skb_h(struct sk_buff *skb, unsigned offset)
>  	return (u64)err << 32 | ntohs(ret);
>  }
>  
> -static u64 jit_get_skb_w(struct sk_buff *skb, unsigned offset)
> +static u64 jit_get_skb_w_pos(struct sk_buff *skb, unsigned offset)
>  {
>  	u32 ret;
>  	int err;
> @@ -101,6 +102,60 @@ static u64 jit_get_skb_w(struct sk_buff *skb, unsigned offset)
>  	return (u64)err << 32 | ntohl(ret);
>  }
>  
> +static u64 jit_get_skb_b_neg(struct sk_buff *skb, unsigned offset)
> +{
> +	u8 *ptr;
> +
> +	ptr = bpf_internal_load_pointer_neg_helper(skb, offset, 1);
> +	if (!ptr)
> +		return (u64)1 << 32;
> +	return *ptr;
> +}
> +
> +static u64 jit_get_skb_h_neg(struct sk_buff *skb, unsigned offset)
> +{
> +	u16 *ptr;
> +
> +	ptr = bpf_internal_load_pointer_neg_helper(skb, offset, 2);
> +	if (!ptr)
> +		return (u64)1 << 32;
> +	return get_unaligned_be16(ptr);
> +}
> +
> +static u64 jit_get_skb_w_neg(struct sk_buff *skb, unsigned offset)
> +{
> +	u32 *ptr;
> +
> +	ptr = bpf_internal_load_pointer_neg_helper(skb, offset, 4);
> +	if (!ptr)
> +		return (u64)1 << 32;
> +	return get_unaligned_be32(ptr);
> +}
> +
> +static u64 jit_get_skb_b_any(struct sk_buff *skb, unsigned offset)
> +{
> +	if ((int)offset >= 0)
> +		return jit_get_skb_b_pos(skb, offset);
> +	else
> +		return jit_get_skb_b_neg(skb, offset);
> +}
> +
> +static u64 jit_get_skb_h_any(struct sk_buff *skb, unsigned offset)
> +{
> +	if ((int)offset >= 0)
> +		return jit_get_skb_h_pos(skb, offset);
> +	else
> +		return jit_get_skb_h_neg(skb, offset);
> +}
> +
> +static u64 jit_get_skb_w_any(struct sk_buff *skb, unsigned offset)
> +{
> +	if ((int)offset >= 0)
> +		return jit_get_skb_w_pos(skb, offset);
> +	else
> +		return jit_get_skb_w_neg(skb, offset);
> +}
> +
>  /*
>   * Wrapper that handles both OABI and EABI and assures Thumb2 interworking
>   * (where the assembly routines like __aeabi_uidiv could cause problems).
> @@ -458,7 +513,10 @@ static inline void update_on_xread(struct jit_ctx *ctx)
>  
>  static int build_body(struct jit_ctx *ctx)
>  {
> -	void *load_func[] = {jit_get_skb_b, jit_get_skb_h, jit_get_skb_w};
> +	void *load_func_any[] = {jit_get_skb_b_any, jit_get_skb_h_any, jit_get_skb_w_any};
> +	void *load_func_pos[] = {jit_get_skb_b_pos, jit_get_skb_h_pos, jit_get_skb_w_pos};
> +	void *load_func_neg[] = {jit_get_skb_b_neg, jit_get_skb_h_neg, jit_get_skb_w_neg};
> +	void **load_func;
>  	const struct sk_filter *prog = ctx->skf;
>  	const struct sock_filter *inst;
>  	unsigned i, load_order, off, condt;
> @@ -498,36 +556,38 @@ static int build_body(struct jit_ctx *ctx)
>  		case BPF_S_LD_B_ABS:
>  			load_order = 0;
>  load:
> -			/* the interpreter will deal with the negative K */
> -			if ((int)k < 0)
> -				return -ENOTSUPP;
>  			emit_mov_i(r_off, k, ctx);
> -load_common:
> -			ctx->seen |= SEEN_DATA | SEEN_CALL;
> -
> -			if (load_order > 0) {
> -				emit(ARM_SUB_I(r_scratch, r_skb_hl,
> -					       1 << load_order), ctx);
> -				emit(ARM_CMP_R(r_scratch, r_off), ctx);
> -				condt = ARM_COND_HS;
> -			} else {
> -				emit(ARM_CMP_R(r_skb_hl, r_off), ctx);
> -				condt = ARM_COND_HI;
> -			}
>  
> -			_emit(condt, ARM_ADD_R(r_scratch, r_off, r_skb_data),
> -			      ctx);
> -
> -			if (load_order == 0)
> -				_emit(condt, ARM_LDRB_I(r_A, r_scratch, 0),
> +			/* deal with negative K */
> +			if (k >= 0) {
> +				load_func = load_func_pos;
> +				if (load_order > 0) {
> +					emit(ARM_SUB_I(r_scratch, r_skb_hl,
> +						       1 << load_order), ctx);
> +					emit(ARM_CMP_R(r_scratch, r_off), ctx);
> +					condt = ARM_COND_HS;
> +				} else {
> +					emit(ARM_CMP_R(r_skb_hl, r_off), ctx);
> +					condt = ARM_COND_HI;
> +				}
> +
> +				_emit(condt, ARM_ADD_R(r_scratch, r_off, r_skb_data),
>  				      ctx);
> -			else if (load_order == 1)
> -				emit_load_be16(condt, r_A, r_scratch, ctx);
> -			else if (load_order == 2)
> -				emit_load_be32(condt, r_A, r_scratch, ctx);
>  
> -			_emit(condt, ARM_B(b_imm(i + 1, ctx)), ctx);
> +				if (load_order == 0)
> +					_emit(condt, ARM_LDRB_I(r_A, r_scratch, 0),
> +					      ctx);
> +				else if (load_order == 1)
> +					emit_load_be16(condt, r_A, r_scratch, ctx);
> +				else if (load_order == 2)
> +					emit_load_be32(condt, r_A, r_scratch, ctx);
>  
> +				_emit(condt, ARM_B(b_imm(i + 1, ctx)), ctx);
> +			} else {
> +				load_func = load_func_neg;
> +			}
> +load_common:
> +			ctx->seen |= SEEN_DATA | SEEN_CALL;
>  			/* the slowpath */
>  			emit_mov_i(ARM_R3, (u32)load_func[load_order], ctx);
>  			emit(ARM_MOV_R(ARM_R0, r_skb), ctx);
> @@ -547,7 +607,9 @@ load_common:
>  		case BPF_S_LD_B_IND:
>  			load_order = 0;
>  load_ind:
> +			load_func = load_func_any;
>  			OP_IMM3(ARM_ADD, r_off, r_X, k, ctx);
> +			load_func = load_func_any;
>  			goto load_common;
>  		case BPF_S_LDX_IMM:
>  			ctx->seen |= SEEN_X;
> @@ -565,25 +627,28 @@ load_ind:
>  		case BPF_S_LDX_B_MSH:
>  			/* x = ((*(frame + k)) & 0xf) << 2; */
>  			ctx->seen |= SEEN_X | SEEN_DATA | SEEN_CALL;
> -			/* the interpreter should deal with the negative K */
> -			if (k < 0)
> -				return -1;
>  			/* offset in r1: we might have to take the slow path */
>  			emit_mov_i(r_off, k, ctx);
> -			emit(ARM_CMP_R(r_skb_hl, r_off), ctx);
> +			/* deal with negative K */
> +			if (k >= 0) {
> +				load_func = load_func_pos;
> +				emit(ARM_CMP_R(r_skb_hl, r_off), ctx);
>  
> -			/* load in r0: common with the slowpath */
> -			_emit(ARM_COND_HI, ARM_LDRB_R(ARM_R0, r_skb_data,
> -						      ARM_R1), ctx);
> -			/*
> -			 * emit_mov_i() might generate one or two instructions,
> -			 * the same holds for emit_blx_r()
> -			 */
> -			_emit(ARM_COND_HI, ARM_B(b_imm(i + 1, ctx) - 2), ctx);
> +				/* load in r0: common with the slowpath */
> +				_emit(ARM_COND_HI, ARM_LDRB_R(ARM_R0, r_skb_data,
> +							      ARM_R1), ctx);
> +				/*
> +				 * emit_mov_i() might generate one or two instructions,
> +				 * the same holds for emit_blx_r()
> +				 */
> +				_emit(ARM_COND_HI, ARM_B(b_imm(i + 1, ctx) - 2), ctx);
> +			} else {
> +				load_func = load_func_neg;
> +			}
>  
>  			emit(ARM_MOV_R(ARM_R0, r_skb), ctx);
>  			/* r_off is r1 */
> -			emit_mov_i(ARM_R3, (u32)jit_get_skb_b, ctx);
> +			emit_mov_i(ARM_R3, (u32)load_func[0], ctx);
>  			emit_blx_r(ARM_R3, ctx);
>  			/* check the return value of skb_copy_bits */
>  			emit(ARM_CMP_I(ARM_R1, 0), ctx);
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 8eeb205..78cd56d 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -161,6 +161,12 @@ extern int sk_chk_filter(struct sock_filter *filter, unsigned int flen);
>  extern void bpf_jit_compile(struct sk_filter *fp);
>  extern void bpf_jit_free(struct sk_filter *fp);
>  #define SK_RUN_FILTER(FILTER, SKB) (*FILTER->bpf_func)(SKB, FILTER->insns)
> +/*
> + * Only Exported for the bpf jit load helper.
> + * Do not call from anywhere else!
> + */
> +extern void *bpf_internal_load_pointer_neg_helper(const struct sk_buff *skb,
> +		int k, unsigned int size);
>  #else
>  static inline void bpf_jit_compile(struct sk_filter *fp)
>  {
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 6f755cc..9cbaecb 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -42,6 +42,10 @@
>  /* No hurry in this branch
>   *
>   * Exported for the bpf jit load helper.
> + *
> + * CAUTION ! :
> + * If its prototype is ever changed, check arch/{*}/net/{*}.S files,
> + * since it is called from BPF assembly code.
>   */
>  void *bpf_internal_load_pointer_neg_helper(const struct sk_buff *skb, int k, unsigned int size)
>  {
>

Mircea Gherzan schrieb:
> Hi,
> 

Hi

> Am 06.04.2012 20:57, schrieb Jan Seiffert:
>> The arm jit has the same problem as the other two jits.
>> It only tests for negative absolute memory references, and if it sees
>> one bails out to let the interpreter handle the filter program.
>>
>> But this fails if the X register contains a negative memory reference
>> and is used for an indirect load.
> 
> I don't think that there's any use case for negative values in X.

<mode temper="elevated" reason="had that discussion already">

There is.
Say you have a bpf filter on an UDP socket. You want to filter bogus
Source addresses in kernel to prevent the context switch and copying.
Since ipv6 has a v4 mapped space, you have to make the same tests for
the v6-mapped space as for v4, so you can use one program for v4 & v6
to begin with.
You can solve it without a negative offset in X, it's is just very
helpful. Still you have negative offsets, which means no jit.

struct bpf_insn udp_filter[] = {
        /*   0 */ BPF_STMT(BPF_LDX|BPF_W|BPF_IMM, -1048576+(12)),
        /*   1 */ BPF_STMT(BPF_LD|BPF_B|BPF_ABS, -1048576+(0)),
        /*   2 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xf0),
        /*   3 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x40, 23 - 4, 0),
        /*   4 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x60, 5 - 5, 41 - 5),
        /*   5 */ BPF_STMT(BPF_LD|BPF_W|BPF_ABS, -1048576+(8)),
        /*   6 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0, 13 - 7, 0),
        /*   7 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x20010DB8, 41 - 8, 0),
        /*   8 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x20010002, 19 - 9, 0),
        /*   9 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xfffffff0),
        /*  10 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x20010010, 41 - 11, 0),
        /*  11 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xff000000),
        /*  12 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xff000000, 41 - 13, 39 - 13),
        /*  13 */ BPF_STMT(BPF_LD|BPF_W|BPF_ABS, -1048576+(12)),
        /*  14 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0, 0, 39 - 15),
        /*  15 */ BPF_STMT(BPF_LD|BPF_W|BPF_ABS, -1048576+(16)),
        /*  16 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xffff, 22 - 17, 0),
        /*  17 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x0064FF9B, 22 - 18, 0),
        /*  18 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0, 41 - 19, 39 - 19),
        /*  19 */ BPF_STMT(BPF_LD|BPF_W|BPF_ABS, -1048576+(12)),
        /*  20 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xffff0000),
        /*  21 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0, 41 - 22, 39 - 22),
        /*  22 */ BPF_STMT(BPF_LDX|BPF_W|BPF_IMM, -1048576+(20)),
        /*  23 */ BPF_STMT(BPF_LD|BPF_W|BPF_IND, 0),
        /*  24 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xffffffff, 41 - 25, 0),
        /*  25 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xffffff00),
        /*  26 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xC0000000, 41 - 27, 0),
        /*  27 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xC0000200, 41 - 28, 0),
        /*  28 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xC6336400, 41 - 29, 0),
        /*  29 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xCB007100, 41 - 30, 0),
        /*  30 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xC0586300, 41 - 31, 0),
        /*  31 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xfffe0000),
        /*  32 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xC6120000, 41 - 33, 0),
        /*  33 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xff000000),
        /*  34 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0, 41 - 35, 0),
        /*  35 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xf0000000),
        /*  36 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xE0000000, 41 - 37, 0),
        /*  37 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xF0000000, 41 - 38, 0),
        /*  38 */ BPF_JUMP(BPF_JMP|BPF_JA, 39 - 39, 0, 0),
        /*  39 */ BPF_STMT(BPF_LD|BPF_W|BPF_LEN, 0),
        /*  40 */ BPF_STMT(BPF_RET|BPF_A, 0),
        /*  41 */ BPF_STMT(BPF_RET|BPF_K, 0),
};

I already had that discussion with Eric, he said it is a valid use case.
http://marc.info/?l=linux-netdev&m=133296726719053&w=2

The patch for x86 is already in, ppc is being reviewed, i only heard of the
arm jit to late, or you would have been in the loop from the beginning.


> In both the original BPF design and in the LSF interpreter, A and X are
> considered unsigned.

Then use the following test program (filter not usefull, just to show the problem):

#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <pcap-bpf.h>

#define die(x) do {perror(x); return 1;} while (0)
struct bpf_insn udp_filter[] = {
	/*   0 */ BPF_STMT(BPF_LDX|BPF_W|BPF_IMM, -1048576+(0)), /* leax	net[0] */
	/*   1 */ BPF_STMT(BPF_LD|BPF_B|BPF_IND, 0),             /* ldb	[x+0] */
	/*   2 */ BPF_STMT(BPF_RET|BPF_A, 0),                    /* ret	a */
};

int main(int argc, char *argv[])
{
	char buf[512];
	struct sockaddr_in addr;
	struct bpf_program prg;
	socklen_t addr_s;
	ssize_t res;
	int fd;

	addr.sin_family = AF_INET;
	addr.sin_port = htons(5000);
	addr.sin_addr.s_addr = 0;
	addr_s = sizeof(addr);
	prg.bf_len = sizeof(udp_filter)/sizeof(udp_filter[0]);
	prg.bf_insns = udp_filter;
	if(-1 == (fd = socket(AF_INET, SOCK_DGRAM, 0)))
		die("socket");
	if(-1 == bind(fd, (struct sockaddr *)&addr, sizeof(addr)))
		die("bind");
	if(-1 == setsockopt(fd, SOL_SOCKET, SO_ATTACH_FILTER, &prg, sizeof(prg)))
		die("setsockopt");
	res = recvfrom(fd, buf, sizeof(buf), 0, (struct sockaddr *)&addr, &addr_s);
	if(res != -1)
		printf("packet received: %zi bytes\n", res);
	else
		die("recvfrom");
	return 0;
}

when used with the bpf jit disabled works:
console 1 $ ./bpf
console 2 $ echo "hello" | nc -u localhost 5000
console 1: packet received: 6 bytes

When the bpf jit gets enabled (echo 100 >
/proc/sys/net/core/bpf_jit_enable) the same program stops working:
console 1 $ ./bpf
console 2 $ echo "hello" | nc -u localhost 5000
console 1:

This is a regression in the interface in my book.
The interpreter handles this perfectly fine.

> The role of X is mainly to allow variable length
> headers (load the length -> unsigned).
> 
> "Negative" K values are permitted for ancillary data loads based on the
> fact that we're not going to see 2GB packets any time soon.
> 

Exactly, and how do i handle both at the same time? (The ancillary space
contains not only cpu_id and such stuff, but the link layer header and the network
space, which is important when you use LSF on arbitrary sockets, not just raw
sockets, maybe the new syscall filter thingy also wants a new ancillary space)

> Mircea
> 

Have some nice holidays :-)

Greetings
	Jan
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sat, 2012-04-07 at 00:28 +0200, Mircea Gherzan wrote:
> Hi,
> 
> Am 06.04.2012 20:57, schrieb Jan Seiffert:
> > The arm jit has the same problem as the other two jits.
> > It only tests for negative absolute memory references, and if it sees
> > one bails out to let the interpreter handle the filter program.
> > 
> > But this fails if the X register contains a negative memory reference
> > and is used for an indirect load.
> 
> I don't think that there's any use case for negative values in X. In
> both the original BPF design and in the LSF interpreter, A and X are
> considered unsigned. The role of X is mainly to allow variable length
> headers (load the length -> unsigned).
> 
> "Negative" K values are permitted for ancillary data loads based on the
> fact that we're not going to see 2GB packets any time soon.
> 

You are wrong.

Please carefully read net/core/filter.c its all here :

void *bpf_internal_load_pointer_neg_helper(const struct sk_buff *skb, int k, unsigned int size)
{
        u8 *ptr = NULL;

        if (k >= SKF_NET_OFF)
                ptr = skb_network_header(skb) + k - SKF_NET_OFF;
        else if (k >= SKF_LL_OFF)
                ptr = skb_mac_header(skb) + k - SKF_LL_OFF;

        if (ptr >= skb->head && ptr + size <= skb_tail_pointer(skb))
                return ptr;
        return NULL;
}


Then :

commit a998d4342337c82dacdc0897d30a9364de1576a1
Author: Jan Seiffert <kaffeemonster@googlemail.com>
Date:   Fri Mar 30 05:24:05 2012 +0000

    bpf jit: Let the x86 jit handle negative offsets
    
    Now the helper function from filter.c for negative offsets is exported,
    it can be used it in the jit to handle negative offsets.
    
    First modify the asm load helper functions to handle:
    - know positive offsets
    - know negative offsets
    - any offset
    
    then the compiler can be modified to explicitly use these helper
    when appropriate.
    
    This fixes the case of a negative X register and allows to lift
    the restriction that bpf programs with negative offsets can't
    be jited.
    
    Signed-of-by: Jan Seiffert <kaffeemonster@googlemail.com>
    Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html