Re: [PATCH net 1/2] bpf, s390: fix jit branch offset related to ldimm64

[David Miller]

From: Michael Holzheu 
Date: Fri, 4 Aug 2017 19:10:33 +0200

> At least I would vote for "Cc: stable".

No, please do not ever do this for networking patches.

Re: [PATCH net 1/2] bpf, s390: fix jit branch offset related to ldimm64

[Michael Holzheu]

Am Fri, 04 Aug 2017 15:52:47 +0200
schrieb Daniel Borkmann :

> On 08/04/2017 03:44 PM, Michael Holzheu wrote:
> > Am Fri,  4 Aug 2017 14:20:54 +0200
> > schrieb Daniel Borkmann :
> [...]
> >
> > What about "Cc: sta...@vger.kernel.org"?
> 
> Handled by Dave, see also: Documentation/networking/netdev-FAQ.txt +117

Thanks, good to know! At least I would vote for "Cc: stable".

Michael

Re: [PATCH net 1/2] bpf, s390: fix jit branch offset related to ldimm64

[Daniel Borkmann]

On 08/04/2017 03:44 PM, Michael Holzheu wrote:

Am Fri,  4 Aug 2017 14:20:54 +0200
schrieb Daniel Borkmann :

[...]


What about "Cc: sta...@vger.kernel.org"?


Handled by Dave, see also: Documentation/networking/netdev-FAQ.txt +117

Cheers,
Daniel

Re: [PATCH net 1/2] bpf, s390: fix jit branch offset related to ldimm64

[Michael Holzheu]

Am Fri,  4 Aug 2017 14:20:54 +0200
schrieb Daniel Borkmann :

> While testing some other work that required JIT modifications, I
> run into test_bpf causing a hang when JIT enabled on s390. The
> problematic test case was the one from ddc665a4bb4b (bpf, arm64:
> fix jit branch offset related to ldimm64), and turns out that we
> do have a similar issue on s390 as well. In bpf_jit_prog() we
> update next instruction address after returning from bpf_jit_insn()
> with an insn_count. bpf_jit_insn() returns either -1 in case of
> error (e.g. unsupported insn), 1 or 2. The latter is only the
> case for ldimm64 due to spanning 2 insns, however, next address
> is only set to i + 1 not taking actual insn_count into account,
> thus fix is to use insn_count instead of 1. bpf_jit_enable in
> mode 2 provides also disasm on s390:
> 
> Before fix:
> 
>   03ff800349b6: a7f40003   brc 15,3ff800349bc ; target
>   03ff800349ba:    unknown
>   03ff800349bc: e3b0f0700024   stg %r11,112(%r15)
>   03ff800349c2: e3e0f0880024   stg %r14,136(%r15)
>   03ff800349c8: 0db0   basr%r11,%r0
>   03ff800349ca: c0ef   llilf   %r14,0
>   03ff800349d0: e320b0360004   lg  %r2,54(%r11)
>   03ff800349d6: e330b03e0004   lg  %r3,62(%r11)
>   03ff800349dc: ec23ffeda065   clgrj   %r2,%r3,10,3ff800349b6 ; jmp
>   03ff800349e2: e3e0b0460004   lg  %r14,70(%r11)
>   03ff800349e8: e3e0b04e0004   lg  %r14,78(%r11)
>   03ff800349ee: b904002e   lgr %r2,%r14
>   03ff800349f2: e3b0f074   lg  %r11,112(%r15)
>   03ff800349f8: e3e0f0880004   lg  %r14,136(%r15)
>   03ff800349fe: 07fe   bcr 15,%r14
> 
> After fix:
> 
>   03ff80ef3db4: a7f40003   brc 15,3ff80ef3dba
>   03ff80ef3db8:    unknown
>   03ff80ef3dba: e3b0f0700024   stg %r11,112(%r15)
>   03ff80ef3dc0: e3e0f0880024   stg %r14,136(%r15)
>   03ff80ef3dc6: 0db0   basr%r11,%r0
>   03ff80ef3dc8: c0ef   llilf   %r14,0
>   03ff80ef3dce: e320b0360004   lg  %r2,54(%r11)
>   03ff80ef3dd4: e330b03e0004   lg  %r3,62(%r11)
>   03ff80ef3dda: ec230006a065   clgrj   %r2,%r3,10,3ff80ef3de6 ; jmp
>   03ff80ef3de0: e3e0b0460004   lg  %r14,70(%r11)
>   03ff80ef3de6: e3e0b04e0004   lg  %r14,78(%r11)  ; target
>   03ff80ef3dec: b904002e   lgr %r2,%r14
>   03ff80ef3df0: e3b0f074   lg  %r11,112(%r15)
>   03ff80ef3df6: e3e0f0880004   lg  %r14,136(%r15)
>   03ff80ef3dfc: 07fe   bcr 15,%r14
> 
> test_bpf.ko suite runs fine after the fix.
> 
> Fixes: 054623105728 ("s390/bpf: Add s390x eBPF JIT compiler backend")
> Signed-off-by: Daniel Borkmann 
> Tested-by: Michael Holzheu 

What about "Cc: sta...@vger.kernel.org"?

Michael

[PATCH net 1/2] bpf, s390: fix jit branch offset related to ldimm64

[Daniel Borkmann]

While testing some other work that required JIT modifications, I
run into test_bpf causing a hang when JIT enabled on s390. The
problematic test case was the one from ddc665a4bb4b (bpf, arm64:
fix jit branch offset related to ldimm64), and turns out that we
do have a similar issue on s390 as well. In bpf_jit_prog() we
update next instruction address after returning from bpf_jit_insn()
with an insn_count. bpf_jit_insn() returns either -1 in case of
error (e.g. unsupported insn), 1 or 2. The latter is only the
case for ldimm64 due to spanning 2 insns, however, next address
is only set to i + 1 not taking actual insn_count into account,
thus fix is to use insn_count instead of 1. bpf_jit_enable in
mode 2 provides also disasm on s390:

Before fix:

  03ff800349b6: a7f40003   brc 15,3ff800349bc ; target
  03ff800349ba:    unknown
  03ff800349bc: e3b0f0700024   stg %r11,112(%r15)
  03ff800349c2: e3e0f0880024   stg %r14,136(%r15)
  03ff800349c8: 0db0   basr%r11,%r0
  03ff800349ca: c0ef   llilf   %r14,0
  03ff800349d0: e320b0360004   lg  %r2,54(%r11)
  03ff800349d6: e330b03e0004   lg  %r3,62(%r11)
  03ff800349dc: ec23ffeda065   clgrj   %r2,%r3,10,3ff800349b6 ; jmp
  03ff800349e2: e3e0b0460004   lg  %r14,70(%r11)
  03ff800349e8: e3e0b04e0004   lg  %r14,78(%r11)
  03ff800349ee: b904002e   lgr %r2,%r14
  03ff800349f2: e3b0f074   lg  %r11,112(%r15)
  03ff800349f8: e3e0f0880004   lg  %r14,136(%r15)
  03ff800349fe: 07fe   bcr 15,%r14

After fix:

  03ff80ef3db4: a7f40003   brc 15,3ff80ef3dba
  03ff80ef3db8:    unknown
  03ff80ef3dba: e3b0f0700024   stg %r11,112(%r15)
  03ff80ef3dc0: e3e0f0880024   stg %r14,136(%r15)
  03ff80ef3dc6: 0db0   basr%r11,%r0
  03ff80ef3dc8: c0ef   llilf   %r14,0
  03ff80ef3dce: e320b0360004   lg  %r2,54(%r11)
  03ff80ef3dd4: e330b03e0004   lg  %r3,62(%r11)
  03ff80ef3dda: ec230006a065   clgrj   %r2,%r3,10,3ff80ef3de6 ; jmp
  03ff80ef3de0: e3e0b0460004   lg  %r14,70(%r11)
  03ff80ef3de6: e3e0b04e0004   lg  %r14,78(%r11)  ; target
  03ff80ef3dec: b904002e   lgr %r2,%r14
  03ff80ef3df0: e3b0f074   lg  %r11,112(%r15)
  03ff80ef3df6: e3e0f0880004   lg  %r14,136(%r15)
  03ff80ef3dfc: 07fe   bcr 15,%r14

test_bpf.ko suite runs fine after the fix.

Fixes: 054623105728 ("s390/bpf: Add s390x eBPF JIT compiler backend")
Signed-off-by: Daniel Borkmann 
Tested-by: Michael Holzheu 
---
 arch/s390/net/bpf_jit_comp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 01c6fbc..1803797 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -1253,7 +1253,8 @@ static int bpf_jit_prog(struct bpf_jit *jit, struct 
bpf_prog *fp)
insn_count = bpf_jit_insn(jit, fp, i);
if (insn_count < 0)
return -1;
-   jit->addrs[i + 1] = jit->prg; /* Next instruction address */
+   /* Next instruction address */
+   jit->addrs[i + insn_count] = jit->prg;
}
bpf_jit_epilogue(jit);
 
-- 
1.9.3