Message ID | 20160617130042.GO3516@indiana.gru.redhat.com |
---|---|
State | Not Applicable, archived |
Delegated to: | David Miller |
Headers | show |
On 2016/06/17 10:00AM, Thadeu Lima de Souza Cascardo wrote: > On Fri, Jun 17, 2016 at 10:53:21PM +1000, Michael Ellerman wrote: > > On Tue, 2016-07-06 at 13:32:23 UTC, "Naveen N. Rao" wrote: > > > diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c > > > new file mode 100644 > > > index 0000000..954ff53 > > > --- /dev/null > > > +++ b/arch/powerpc/net/bpf_jit_comp64.c > > > @@ -0,0 +1,956 @@ > > ... > > > + > > > +static void bpf_jit_fill_ill_insns(void *area, unsigned int size) > > > +{ > > > + int *p = area; > > > + > > > + /* Fill whole space with trap instructions */ > > > + while (p < (int *)((char *)area + size)) > > > + *p++ = BREAKPOINT_INSTRUCTION; > > > +} > > > > This breaks the build for some configs, presumably you're missing a header: > > > > arch/powerpc/net/bpf_jit_comp64.c:30:10: error: 'BREAKPOINT_INSTRUCTION' undeclared (first use in this function) > > > > http://kisskb.ellerman.id.au/kisskb/buildresult/12720611/ > > > > cheers > > Hi, Michael and Naveen. > > I noticed independently that there is a problem with BPF JIT and ABIv2, and > worked out the patch below before I noticed Naveen's patchset and the latest > changes in ppc tree for a better way to check for ABI versions. > > However, since the issue described below affect mainline and stable kernels, > would you consider applying it before merging your two patchsets, so that we can > more easily backport the fix? Hi Cascardo, Given that this has been broken on ABIv2 since forever, I didn't bother fixing it. But, I can see why this would be a good thing to have for -stable and existing distros. However, while your patch below may fix the crash you're seeing on ppc64le, it is not sufficient -- you'll need changes in bpf_jit_asm.S as well. Regards, Naveen
On Sun, Jun 19, 2016 at 11:19:14PM +0530, Naveen N. Rao wrote: > On 2016/06/17 10:00AM, Thadeu Lima de Souza Cascardo wrote: > > On Fri, Jun 17, 2016 at 10:53:21PM +1000, Michael Ellerman wrote: > > > On Tue, 2016-07-06 at 13:32:23 UTC, "Naveen N. Rao" wrote: > > > > diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c > > > > new file mode 100644 > > > > index 0000000..954ff53 > > > > --- /dev/null > > > > +++ b/arch/powerpc/net/bpf_jit_comp64.c > > > > @@ -0,0 +1,956 @@ > > > ... > > > > + > > > > +static void bpf_jit_fill_ill_insns(void *area, unsigned int size) > > > > +{ > > > > + int *p = area; > > > > + > > > > + /* Fill whole space with trap instructions */ > > > > + while (p < (int *)((char *)area + size)) > > > > + *p++ = BREAKPOINT_INSTRUCTION; > > > > +} > > > > > > This breaks the build for some configs, presumably you're missing a header: > > > > > > arch/powerpc/net/bpf_jit_comp64.c:30:10: error: 'BREAKPOINT_INSTRUCTION' undeclared (first use in this function) > > > > > > http://kisskb.ellerman.id.au/kisskb/buildresult/12720611/ > > > > > > cheers > > > > Hi, Michael and Naveen. > > > > I noticed independently that there is a problem with BPF JIT and ABIv2, and > > worked out the patch below before I noticed Naveen's patchset and the latest > > changes in ppc tree for a better way to check for ABI versions. > > > > However, since the issue described below affect mainline and stable kernels, > > would you consider applying it before merging your two patchsets, so that we can > > more easily backport the fix? > > Hi Cascardo, > Given that this has been broken on ABIv2 since forever, I didn't bother > fixing it. But, I can see why this would be a good thing to have for > -stable and existing distros. However, while your patch below may fix > the crash you're seeing on ppc64le, it is not sufficient -- you'll need > changes in bpf_jit_asm.S as well. Hi, Naveen. Any tips on how to exercise possible issues there? Or what changes you think would be sufficient? I will see what I can find by myself, but would appreciate any help. Regards. Cascardo. > > Regards, > Naveen >
On Fri, 2016-06-17 at 10:00 -0300, Thadeu Lima de Souza Cascardo wrote: > From a984dc02b6317a1d3a3c2302385adba5227be5bd Mon Sep 17 00:00:00 2001 > From: Thadeu Lima de Souza Cascardo <cascardo@redhat.com> > Date: Wed, 15 Jun 2016 13:22:12 -0300 > Subject: [PATCH] ppc: Fix BPF JIT for ABIv2 > > ABIv2 used for ppc64le does not use function descriptors. Without this patch, > whenever BPF JIT is enabled, we get a crash as below. > ... > diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h > index 889fd19..28b89ed 100644 > --- a/arch/powerpc/net/bpf_jit.h > +++ b/arch/powerpc/net/bpf_jit.h > @@ -70,7 +70,7 @@ DECLARE_LOAD_FUNC(sk_load_half); > DECLARE_LOAD_FUNC(sk_load_byte); > DECLARE_LOAD_FUNC(sk_load_byte_msh); > > -#ifdef CONFIG_PPC64 > +#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF != 2) > #define FUNCTION_DESCR_SIZE 24 > #else > #define FUNCTION_DESCR_SIZE 0 > diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c > index 2d66a84..035b887 100644 > --- a/arch/powerpc/net/bpf_jit_comp.c > +++ b/arch/powerpc/net/bpf_jit_comp.c > @@ -664,7 +664,7 @@ void bpf_jit_compile(struct bpf_prog *fp) > > if (image) { > bpf_flush_icache(code_base, code_base + (proglen/4)); > -#ifdef CONFIG_PPC64 > +#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF != 2) > /* Function descriptor nastiness: Address + TOC */ > ((u64 *)image)[0] = (u64)code_base; > ((u64 *)image)[1] = local_paca->kernel_toc; Confirmed that even with this patch we still crash: # echo 1 > /proc/sys/net/core/bpf_jit_enable # modprobe test_bpf BPF filter opcode 0020 (@3) unsupported BPF filter opcode 0020 (@2) unsupported BPF filter opcode 0020 (@0) unsupported Unable to handle kernel paging request for data at address 0xd0000000054f65e8 Faulting instruction address: 0xc0000000008765f8 cpu 0x0: Vector: 300 (Data Access) at [c0000000034f3480] pc: c0000000008765f8: skb_copy_bits+0x158/0x330 lr: c00000000008fb7c: bpf_slow_path_byte+0x28/0x54 sp: c0000000034f3700 msr: 800000010280b033 dar: d0000000054f65e8 dsisr: 40000000 current = 0xc0000001f857d8d0 paca = 0xc000000007b80000 softe: 0 irq_happened: 0x01 pid = 2993, comm = modprobe Linux version 4.7.0-rc3-00055-g9497a1c1c5b4-dirty (michael@ka3.ozlabs.ibm.com) () #30 SMP Wed Jun 22 15:06:58 AEST 2016 enter ? for help [c0000000034f3770] c00000000008fb7c bpf_slow_path_byte+0x28/0x54 [c0000000034f37e0] d000000007bb004c [c0000000034f3900] d000000005331668 test_bpf_init+0x5fc/0x7f8 [test_bpf] [c0000000034f3a30] c00000000000b628 do_one_initcall+0x68/0x1d0 [c0000000034f3af0] c0000000009beb24 do_init_module+0x90/0x240 [c0000000034f3b80] c0000000001642bc load_module+0x206c/0x22f0 [c0000000034f3d30] c0000000001648b0 SyS_finit_module+0x120/0x180 [c0000000034f3e30] c000000000009260 system_call+0x38/0x108 --- Exception: c01 (System Call) at 00003fff7ffa2db4 cheers
diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h index 889fd19..28b89ed 100644 --- a/arch/powerpc/net/bpf_jit.h +++ b/arch/powerpc/net/bpf_jit.h @@ -70,7 +70,7 @@ DECLARE_LOAD_FUNC(sk_load_half); DECLARE_LOAD_FUNC(sk_load_byte); DECLARE_LOAD_FUNC(sk_load_byte_msh); -#ifdef CONFIG_PPC64 +#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF != 2) #define FUNCTION_DESCR_SIZE 24 #else #define FUNCTION_DESCR_SIZE 0 diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c index 2d66a84..035b887 100644 --- a/arch/powerpc/net/bpf_jit_comp.c +++ b/arch/powerpc/net/bpf_jit_comp.c @@ -664,7 +664,7 @@ void bpf_jit_compile(struct bpf_prog *fp) if (image) { bpf_flush_icache(code_base, code_base + (proglen/4)); -#ifdef CONFIG_PPC64 +#if defined(CONFIG_PPC64) && (!defined(_CALL_ELF) || _CALL_ELF != 2) /* Function descriptor nastiness: Address + TOC */ ((u64 *)image)[0] = (u64)code_base; ((u64 *)image)[1] = local_paca->kernel_toc;