Skip to content
  • Daniel Borkmann's avatar
    bpf, arm64: fix out of bounds access in tail call · 54c6d01b
    Daniel Borkmann authored
    [ upstream commit 16338a9b ]
    
    I recently noticed a crash on arm64 when feeding a bogus index
    into BPF tail call helper. The crash would not occur when the
    interpreter is used, but only in case of JIT. Output looks as
    follows:
    
      [  347.007486] Unable to handle kernel paging request at virtual address fffb850e96492510
      [...]
      [  347.043065] [fffb850e96492510] address between user and kernel address ranges
      [  347.050205] Internal error: Oops: 96000004 [#1] SMP
      [...]
      [  347.190829] x13: 0000000000000000 x12: 0000000000000000
      [  347.196128] x11: fffc047ebe782800 x10: ffff808fd7d0fd10
      [  347.201427] x9 : 0000000000000000 x8 : 0000000000000000
      [  347.206726] x7 : 0000000000000000 x6 : 001c991738000000
      [  347.212025] x5 : 0000000000000018 x4 : 000000000000ba5a
      [  347.217325] x3 : 00000000000329c4 x2 : ffff808fd7cf0500
      [  347.222625] x1 : ffff808fd7d0fc00 x0 : ffff808fd7cf0500
      [  347.227926] Process test_verifier (pid: 4548, stack limit = 0x000000007467fa61)
      [  347.235221] Call trace:
      [  347.237656]  0xffff000002f3a4fc
      [  347.240784]  bpf_test_run+0x78/0xf8
      [  347.244260]  bpf_prog_test_run_skb+0x148/0x230
      [  347.248694]  SyS_bpf+0x77c/0x1110
      [  347.251999]  el0_svc_naked+0x30/0x34
      [  347.255564] Code: 9100075a d280220a 8b0a002a d37df04b (f86b694b)
      [...]
    
    In this case the index used in BPF r3 is the same as in r1
    at the time of the call, meaning we fed a pointer as index;
    here, it had the value 0xffff808fd7cf0500 which sits in x2.
    
    While I found tail calls to be working in general (also for
    hitting the error cases), I noticed the following in the code
    emission:
    
      # bpftool p d j i 988
      [...]
      38:   ldr     w10, [x1,x10]
      3c:   cmp     w2, w10
      40:   b.ge    0x000000000000007c              <-- signed cmp
      44:   mov     x10, #0x20                      // #32
      48:   cmp     x26, x10
      4c:   b.gt    0x000000000000007c
      50:   add     x26, x26, #0x1
      54:   mov     x10, #0x110                     // #272
      58:   add     x10, x1, x10
      5c:   lsl     x11, x2, #3
      60:   ldr     x11, [x10,x11]                  <-- faulting insn (f86b694b)
      64:   cbz     x11, 0x000000000000007c
      [...]
    
    Meaning, the tests passed because commit ddb55992 ("arm64:
    bpf: implement bpf_tail_call() helper") was using signed compares
    instead of unsigned which as a result had the test wrongly passing.
    
    Change this but also the tail call count test both into unsigned
    and cap the index as u32. Latter we did as well in 90caccdd
    ("bpf: fix bpf_tail_call() x64 JIT") and is needed in addition here,
    too. Tested on HiSilicon Hi1616.
    
    Result after patch:
    
      # bpftool p d j i 268
      [...]
      38:	ldr	w10, [x1,x10]
      3c:	add	w2, w2, #0x0
      40:	cmp	w2, w10
      44:	b.cs	0x0000000000000080
      48:	mov	x10, #0x20                  	// #32
      4c:	cmp	x26, x10
      50:	b.hi	0x0000000000000080
      54:	add	x26, x26, #0x1
      58:	mov	x10, #0x110                 	// #272
      5c:	add	x10, x1, x10
      60:	lsl	x11, x2, #3
      64:	ldr	x11, [x10,x11]
      68:	cbz	x11, 0x0000000000000080
      [...]
    
    Fixes: ddb55992
    
     ("arm64: bpf: implement bpf_tail_call() helper")
    Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
    Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    54c6d01b