Skip to contentSkip to conversation

Patch "bpf, arm64: fix faulty emission of map access in tail calls" has been added to the 4.9-stable tree

PATCH [email protected] 1 participant 1 message May 31, 2017

Subsystems (4)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

bpf, arm64: fix faulty emission of map access in tail calls

to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
bpf-arm64-fix-faulty-emission-of-map-access-in-tail-calls.patch
and it can be found in the queue-4.9 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From foo@baz Wed May 31 09:13:34 JST 2017

From: Daniel Borkmann <[email protected]>
Date: Thu, 11 May 2017 01:53:15 +0200
Subject: bpf, arm64: fix faulty emission of map access in tail calls

From: Daniel Borkmann <[email protected]>

[ Upstream commit d8b54110ee944de522ccd3531191f39986ec20f9 ]

Shubham was recently asking on netdev why in arm64 JIT we don't multiply the index for accessing the tail call map by 8. That led me into testing out arm64 JIT wrt tail calls and it turned out I got a NULL pointer dereference on the tail call.

The buggy access is at:

  prog = array->ptrs[index];
  if (prog == NULL)
      goto out;

[...]

  00000060:  d2800e0a  mov x10, #0x70 // #112
  00000064:  f86a682a  ldr x10, [x1,x10]
  00000068:  f862694b  ldr x11, [x10,x2]
  0000006c:  b40000ab  cbz x11, 0x00000080

[...]

The code triggering the crash is f862694b. x1 at the time contains the address of the bpf array, x10 offsetof(struct bpf_array, ptrs). Meaning, above we load the pointer to the program at map slot 0 into x10. x10 can then be NULL if the slot is not occupied, which we later on try to access with a user given offset in x2 that is the map index.

Fix this by emitting the following instead:

[...]

  00000060:  d2800e0a  mov x10, #0x70 // #112
  00000064:  8b0a002a  add x10, x1, x10
  00000068:  d37df04b  lsl x11, x2, #3
  0000006c:  f86b694b  ldr x11, [x10,x11]
  00000070:  b40000ab  cbz x11, 0x00000084

[...]

This basically adds the offset to ptrs to the base address of the bpf array we got and we later on access the map with an index * 8 offset relative to that. The tail call map itself is basically one large area with meta data at the head followed by the array of prog pointers. This makes tail calls working again, tested on Cavium ThunderX ARMv8.

Fixes ddb55992b04d ("arm64: bpf: implement bpf_tail_call() helper")
---
 arch/arm64/net/bpf_jit_comp.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
arch/arm64/net/bpf_jit_comp.c +3 -2
@@ -252,8 +252,9 @@ static int emit_bpf_tail_call(struct jit
 	 */
 	off = offsetof(struct bpf_array, ptrs);
 	emit_a64_mov_i64(tmp, off, ctx);
-	emit(A64_LDR64(tmp, r2, tmp), ctx);
-	emit(A64_LDR64(prg, tmp, r3), ctx);
+	emit(A64_ADD(1, tmp, r2, tmp), ctx);
+	emit(A64_LSL(1, prg, r3, 3), ctx);
+	emit(A64_LDR64(prg, tmp, prg), ctx);
 	emit(A64_CBZ(1, prg, jmp_offset), ctx);
 
 	/* goto *(prog->bpf_func + prologue_size); */

Patches currently in stable-queue which might be from [email protected] are

queue-4.9/bpf-arm64-fix-faulty-emission-of-map-access-in-tail-calls.patch queue-4.9/bpf-add-bpf_clone_redirect-to-bpf_helper_changes_pkt_data.patch