| Message ID | 1326873533.2606.46.camel@edumazet-laptop |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, Jan 18, 2012 at 08:58:53AM +0100, Eric Dumazet wrote: > Target of the conditional jump in case a divide by 0 is performed by a > bpf is wrong. > > Also change the wrong length detection at the end of code generation to > issue a more explicit message and abort the compilation. > > Reported-by: Phil Oester <kernel@linuxace.com> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > --- > Please Phil test following fix, thanks ! Got the following output after applying this fix (no panic this time): proglen=231 != oldproglen=235 bpb_jit_compile fatal error Filter being used is 'not net a.b.x.112/28 and not net a.b.y.112/28' Thanks, Phil -- 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
Le mercredi 18 janvier 2012 à 07:57 -0800, Phil Oester a écrit : > Got the following output after applying this fix (no panic this time): > > proglen=231 != oldproglen=235 > bpb_jit_compile fatal error > > Filter being used is 'not net a.b.x.112/28 and not net a.b.y.112/28' Thanks ! It seems there is another bug, I'll send you a new patch once fixed. -- 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
Le mercredi 18 janvier 2012 à 17:01 +0100, Eric Dumazet a écrit : > Le mercredi 18 janvier 2012 à 07:57 -0800, Phil Oester a écrit : > > > Got the following output after applying this fix (no panic this time): > > > > proglen=231 != oldproglen=235 > > bpb_jit_compile fatal error > > > > Filter being used is 'not net a.b.x.112/28 and not net a.b.y.112/28' > By the way, libcap gives following code, not optimal (000) ldh [12] (001) jeq #0x800 jt 2 jf 14 (002) ld [26] (003) and #0xfffffff0 (004) jeq #0x1020340 jt 28 jf 5 (005) ld [26] (006) and #0xfffffff0 (007) jeq #0x1021480 jt 28 jf 8 (008) ld [30] (009) and #0xfffffff0 (010) jeq #0x1020340 jt 28 jf 11 (011) ld [30] (012) and #0xfffffff0 (013) jeq #0x1021480 jt 28 jf 29 ... could be : (000) ldh [12] (001) jeq #0x800 jt 2 jf 10 (002) ld [26] (003) and #0xfffffff0 (004) jeq #0x1020340 jt ok jf 5 (005) jeq #0x1021480 jt ok jf 6 (006) ld [30] (007) and #0xfffffff0 (008) jeq #0x1020340 jt ok jf 9 (009) jeq #0x1021480 jt ok jf ret0 ... -- 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
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c index 7b65f75..5dfa72c 100644 --- a/arch/x86/net/bpf_jit_comp.c +++ b/arch/x86/net/bpf_jit_comp.c @@ -260,9 +260,14 @@ void bpf_jit_compile(struct sk_filter *fp) case BPF_S_ALU_DIV_X: /* A /= X; */ seen |= SEEN_XREG; EMIT2(0x85, 0xdb); /* test %ebx,%ebx */ - if (pc_ret0 != -1) - EMIT_COND_JMP(X86_JE, addrs[pc_ret0] - (addrs[i] - 4)); - else { + if (pc_ret0 > 0) { + /* addrs[pc_ret0 - 1] is start address of target + * (addrs[i] - 4) is the address following this jmp + * ("xor %edx,%edx; div %ebx" being 4 bytes long) + */ + EMIT_COND_JMP(X86_JE, addrs[pc_ret0 - 1] - + (addrs[i] - 4)); + } else { EMIT_COND_JMP(X86_JNE, 2 + 5); CLEAR_A(); EMIT1_off32(0xe9, cleanup_addr - (addrs[i] - 4)); /* jmp .+off32 */ @@ -483,8 +488,9 @@ common_load: seen |= SEEN_DATAREF; goto common_load; case BPF_S_LDX_B_MSH: if ((int)K < 0) { - if (pc_ret0 != -1) { - EMIT_JMP(addrs[pc_ret0] - addrs[i]); + if (pc_ret0 > 0) { + /* addrs[pc_ret0 - 1] is the start address */ + EMIT_JMP(addrs[pc_ret0 - 1] - addrs[i]); break; } CLEAR_A(); @@ -584,6 +590,7 @@ cond_branch: f_offset = addrs[i + filter[i].jf] - addrs[i]; ilen = prog - temp; if (image) { if (unlikely(proglen + ilen > oldproglen)) { +bpf_fatal_error: pr_err("bpb_jit_compile fatal error\n"); kfree(addrs); module_free(NULL, image); @@ -605,7 +612,10 @@ cond_branch: f_offset = addrs[i + filter[i].jf] - addrs[i]; cleanup_addr -= 4; /* mov -8(%rbp),%rbx */ if (image) { - WARN_ON(proglen != oldproglen); + if (proglen != oldproglen) { + pr_err("proglen=%u != oldproglen=%u\n", proglen, oldproglen); + goto bpf_fatal_error; + } break; } if (proglen == oldproglen) {
Target of the conditional jump in case a divide by 0 is performed by a bpf is wrong. Also change the wrong length detection at the end of code generation to issue a more explicit message and abort the compilation. Reported-by: Phil Oester <kernel@linuxace.com> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> --- Please Phil test following fix, thanks ! arch/x86/net/bpf_jit_comp.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 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