linux-kernel - [PATCH] net: bpf_jit: fix BPF_S_LDX_B

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1332074448.3722.25.camel@edumazet-laptop>
Date:	Sun, 18 Mar 2012 05:40:48 -0700
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Indan Zupancic <indan@....nu>, David Miller <davem@...emloft.net>
Cc:	Will Drewry <wad@...omium.org>, linux-kernel@...r.kernel.org,
	linux-arch@...r.kernel.org, linux-doc@...r.kernel.org,
	kernel-hardening@...ts.openwall.com, netdev@...r.kernel.org,
	x86@...nel.org, arnd@...db.de, davem@...emloft.net, hpa@...or.com,
	mingo@...hat.com, oleg@...hat.com, peterz@...radead.org,
	rdunlap@...otime.net, mcgrathr@...omium.org, tglx@...utronix.de,
	luto@....edu, eparis@...hat.com, serge.hallyn@...onical.com,
	djm@...drot.org, scarybeasts@...il.com, pmoore@...hat.com,
	akpm@...ux-foundation.org, corbet@....net, markus@...omium.org,
	coreyb@...ux.vnet.ibm.com, keescook@...omium.org,
	Matt Evans <matt@...abs.org>
Subject: [PATCH] net: bpf_jit: fix BPF_S_LDX_B_MSH compilation

Le dimanche 18 mars 2012 à 19:35 +1100, Indan Zupancic a écrit :

> And in the 00.00001% case that the filter uses a computed negative
> offset the BPF JIT fails at runtime. So to not be buggy you need at
> least a call to __load_pointer() for the negative case.

Please show me how and why a real (I mean useful one...) filter could
generate a dynamic negative value, and I'll change the code.

Negative values are there to allow access to network/mac header
components. I cant see how a BPF code could have a valid use of dynamic
indexes in these headers.

Right now we consider such code is evil and filter does "return 0"
saying so.

If I remember well, a more practical issue was mentioned by Matt Evans
and I forgot about it.

Its even _documented_ in his code.

Thanks

[PATCH] net: bpf_jit: fix BPF_S_LDX_B_MSH compilation

Matt Evans spotted that x86 bpf_jit was incorrectly handling negative
constant offsets in BPF_S_LDX_B_MSH instruction.

We need to abort JIT compilation like we do in common_load so that
filter uses the interpreter code and can call __load_pointer()

Reference: http://lists.openwall.net/netdev/2011/07/19/11

Thanks to Indan Zupancic to bring back this issue.

Reported-by: Matt Evans <matt@...abs.org>
Reported-by: Indan Zupancic <indan@....nu>
Signed-off-by: Eric Dumazet <eric.dumazet@...il.com>
---
David, libpcap doesnt generate such pattern, so feel to add this patch
to net-next. Thanks !

 arch/x86/net/bpf_jit_comp.c |   14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 7c1b765..5671752 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -475,8 +475,10 @@ void bpf_jit_compile(struct sk_filter *fp)
 			case BPF_S_LD_W_ABS:
 				func = sk_load_word;
 common_load:			seen |= SEEN_DATAREF;
-				if ((int)K < 0)
+				if ((int)K < 0) {
+					/* Abort the JIT because __load_pointer() is needed. */
 					goto out;
+				}
 				t_offset = func - (image + addrs[i]);
 				EMIT1_off32(0xbe, K); /* mov imm32,%esi */
 				EMIT1_off32(0xe8, t_offset); /* call */
@@ -489,14 +491,8 @@ common_load:			seen |= SEEN_DATAREF;
 				goto common_load;
 			case BPF_S_LDX_B_MSH:
 				if ((int)K < 0) {
-					if (pc_ret0 > 0) {
-						/* addrs[pc_ret0 - 1] is the start address */
-						EMIT_JMP(addrs[pc_ret0 - 1] - addrs[i]);
-						break;
-					}
-					CLEAR_A();
-					EMIT_JMP(cleanup_addr - addrs[i]);
-					break;
+					/* Abort the JIT because __load_pointer() is needed. */
+					goto out;
 				}
 				seen |= SEEN_DATAREF | SEEN_XREG;
 				t_offset = sk_load_byte_msh - (image + addrs[i]);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/