Commit aee636c4 authored by Eric Dumazet's avatar Eric Dumazet Committed by David S. Miller
Browse files

bpf: do not use reciprocal divide 

At first Jakub Zawadzki noticed that some divisions by reciprocal_divide
were not correct. (off by one in some cases)
http://www.wireshark.org/~darkjames/reciprocal-buggy.c

He could also show this with BPF:
http://www.wireshark.org/~darkjames/set-and-dump-filter-k-bug.c



The reciprocal divide in linux kernel is not generic enough,
lets remove its use in BPF, as it is not worth the pain with
current cpus.

Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Reported-by: default avatarJakub Zawadzki <darkjames-ws@darkjames.pl>
Cc: Mircea Gherzan <mgherzan@gmail.com>
Cc: Daniel Borkmann <dxchgb@gmail.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Cc: Matt Evans <matt@ozlabs.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent ba42fad0

arch/arm/net/bpf_jit_32.c

+3 −3
Original line number Diff line number Diff line
@@ -641,10 +641,10 @@ load_ind: 
			emit(ARM_MUL(r_A, r_A, r_X), ctx);

			break;

		case BPF_S_ALU_DIV_K:

			/* current k == reciprocal_value(userspace k) */

			if (k == 1)

				break;

			emit_mov_i(r_scratch, k, ctx);

			/* A = top 32 bits of the product */

			emit(ARM_UMULL(r_scratch, r_A, r_A, r_scratch), ctx);

			emit_udiv(r_A, r_A, r_scratch, ctx);

			break;

		case BPF_S_ALU_DIV_X:

			update_on_xread(ctx);

arch/powerpc/net/bpf_jit_comp.c

+4 −3
Original line number Diff line number Diff line
@@ -223,10 +223,11 @@ static int bpf_jit_build_body(struct sk_filter *fp, u32 *image, 
			}

			PPC_DIVWU(r_A, r_A, r_X);

			break;

		case BPF_S_ALU_DIV_K: /* A = reciprocal_divide(A, K); */

		case BPF_S_ALU_DIV_K: /* A /= K */

			if (K == 1)

				break;

			PPC_LI32(r_scratch1, K);

			/* Top 32 bits of 64bit result -> A */

			PPC_MULHWU(r_A, r_A, r_scratch1);

			PPC_DIVWU(r_A, r_A, r_scratch1);

			break;

		case BPF_S_ALU_AND_X:

			ctx->seen |= SEEN_XREG;

arch/s390/net/bpf_jit_comp.c

+12 −5
Original line number Diff line number Diff line
@@ -371,11 +371,13 @@ static int bpf_jit_insn(struct bpf_jit *jit, struct sock_filter *filter, 
		/* dr %r4,%r12 */

		EMIT2(0x1d4c);

		break;

	case BPF_S_ALU_DIV_K: /* A = reciprocal_divide(A, K) */

		/* m %r4,<d(K)>(%r13) */

		EMIT4_DISP(0x5c40d000, EMIT_CONST(K));

		/* lr %r5,%r4 */

		EMIT2(0x1854);

	case BPF_S_ALU_DIV_K: /* A /= K */

		if (K == 1)

			break;

		/* lhi %r4,0 */

		EMIT4(0xa7480000);

		/* d %r4,<d(K)>(%r13) */

		EMIT4_DISP(0x5d40d000, EMIT_CONST(K));

		break;

	case BPF_S_ALU_MOD_X: /* A %= X */

		jit->seen |= SEEN_XREG | SEEN_RET0;
@@ -391,6 +393,11 @@ static int bpf_jit_insn(struct bpf_jit *jit, struct sock_filter *filter, 
		EMIT2(0x1854);

		break;

	case BPF_S_ALU_MOD_K: /* A %= K */

		if (K == 1) {

			/* lhi %r5,0 */

			EMIT4(0xa7580000);

			break;

		}

		/* lhi %r4,0 */

		EMIT4(0xa7480000);

		/* d %r4,<d(K)>(%r13) */

arch/sparc/net/bpf_jit_comp.c

+14 −3
Original line number Diff line number Diff line
@@ -497,9 +497,20 @@ void bpf_jit_compile(struct sk_filter *fp) 
			case BPF_S_ALU_MUL_K:	/* A *= K */

				emit_alu_K(MUL, K);

				break;

			case BPF_S_ALU_DIV_K:	/* A /= K */

				emit_alu_K(MUL, K);

				emit_read_y(r_A);

			case BPF_S_ALU_DIV_K:	/* A /= K with K != 0*/

				if (K == 1)

					break;

				emit_write_y(G0);

#ifdef CONFIG_SPARC32

				/* The Sparc v8 architecture requires

				 * three instructions between a %y

				 * register write and the first use.

				 */

				emit_nop();

				emit_nop();

				emit_nop();

#endif

				emit_alu_K(DIV, K);

				break;

			case BPF_S_ALU_DIV_X:	/* A /= X; */

				emit_cmpi(r_X, 0);

arch/x86/net/bpf_jit_comp.c

+10 −4
Original line number Diff line number Diff line
@@ -359,15 +359,21 @@ void bpf_jit_compile(struct sk_filter *fp) 
				EMIT2(0x89, 0xd0);	/* mov %edx,%eax */

				break;

			case BPF_S_ALU_MOD_K: /* A %= K; */

				if (K == 1) {

					CLEAR_A();

					break;

				}

				EMIT2(0x31, 0xd2);	/* xor %edx,%edx */

				EMIT1(0xb9);EMIT(K, 4);	/* mov imm32,%ecx */

				EMIT2(0xf7, 0xf1);	/* div %ecx */

				EMIT2(0x89, 0xd0);	/* mov %edx,%eax */

				break;

			case BPF_S_ALU_DIV_K: /* A = reciprocal_divide(A, K); */

				EMIT3(0x48, 0x69, 0xc0); /* imul imm32,%rax,%rax */

				EMIT(K, 4);

				EMIT4(0x48, 0xc1, 0xe8, 0x20); /* shr $0x20,%rax */

			case BPF_S_ALU_DIV_K: /* A /= K */

				if (K == 1)

					break;

				EMIT2(0x31, 0xd2);	/* xor %edx,%edx */

				EMIT1(0xb9);EMIT(K, 4);	/* mov imm32,%ecx */

				EMIT2(0xf7, 0xf1);	/* div %ecx */

				break;

			case BPF_S_ALU_AND_X:

				seen |= SEEN_XREG;

CRA Git | Maintained and supported by SUSTech CRA and CCSE