[PATCH v2 bpf-next 0/2] arm64/bpf: remove 128MB limit for BPF JIT programs

[PATCH v2 bpf-next 0/2] arm64/bpf: remove 128MB limit for BPF JIT programs

[Alan Maguire]

There is a 128MB limit on BPF JIT program allocations; this is
to ensure BPF programs are in branching range of each other.
Patch 1 in this series removes this restriction.  To verify
exception handling still works, a test case to validate
exception handling in BPF programs is added in patch 2.

There was previous discussion around this topic [1], in particular
would be good to get feedback from Daniel if this approach makes
sense.

[1] https://lore.kernel.org/all/20181121131733.14910-1-ard.biesheuvel@linaro.org/

Changes since v1:

 - respin picked up changes in arm64 exception handling changes
   which removed need for special BPF exception handling in patch 1
 - made selftest use task creation/task_struct field instead to
   minimize BPF selftests parallel runs, moved from CHECK()s
   to ASSERT_*()s (Andrii, patch 2)

Alan Maguire (1):
  selftests/bpf: add exception handling selftests for tp_bpf program

Russell King (1):
  arm64/bpf: remove 128MB limit for BPF JIT programs

 arch/arm64/include/asm/extable.h                   |  9 -----
 arch/arm64/include/asm/memory.h                    |  5 +--
 arch/arm64/kernel/traps.c                          |  2 +-
 arch/arm64/mm/ptdump.c                             |  2 -
 arch/arm64/net/bpf_jit_comp.c                      |  7 +---
 tools/testing/selftests/bpf/prog_tests/exhandler.c | 43 ++++++++++++++++++++++
 tools/testing/selftests/bpf/progs/exhandler_kern.c | 43 ++++++++++++++++++++++
 7 files changed, 90 insertions(+), 21 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/exhandler.c
 create mode 100644 tools/testing/selftests/bpf/progs/exhandler_kern.c

-- 
1.8.3.1

[PATCH v2 bpf-next 1/2] arm64/bpf: remove 128MB limit for BPF JIT programs

[Alan Maguire]

From: Russell King <russell.king@oracle.com>

commit 91fc957c9b1d ("arm64/bpf: don't allocate BPF JIT programs in module memory")

...restricts BPF JIT program allocation to a 128MB region to ensure
BPF programs are still in branching range of each other.  However
this restriction should not apply to the aarch64 JIT, since
BPF_JMP | BPF_CALL are implemented as a 64-bit move into a register
and then a BLR instruction - which has the effect of being able to call
anything without proximity limitation.

The practical reason to relax this restriction on JIT memory is that 128MB
of JIT memory can be quickly exhausted, especially where PAGE_SIZE is 64KB -
one page is needed per program.  In cases where seccomp filters are applied
to multiple VMs on VM launch - such filters are classic BPF but converted to
BPF - this can severely limit the number of VMs that can be launched.  In a
world where we support BPF JIT always on, turning off the JIT isn't always
an option either.

Fixes: 91fc957c9b1d ("arm64/bpf: don't allocate BPF JIT programs in module memory")

Suggested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Russell King <russell.king@oracle.com>
Tested-by: Alan Maguire <alan.maguire@oracle.com>
---
 arch/arm64/include/asm/extable.h | 9 ---------
 arch/arm64/include/asm/memory.h  | 5 +----
 arch/arm64/kernel/traps.c        | 2 +-
 arch/arm64/mm/ptdump.c           | 2 --
 arch/arm64/net/bpf_jit_comp.c    | 7 ++-----
 5 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/arch/arm64/include/asm/extable.h b/arch/arm64/include/asm/extable.h
index 8b300dd..72b0e71 100644
--- a/arch/arm64/include/asm/extable.h
+++ b/arch/arm64/include/asm/extable.h
@@ -33,15 +33,6 @@ struct exception_table_entry
 	(b)->data = (tmp).data;				\
 } while (0)
 
-static inline bool in_bpf_jit(struct pt_regs *regs)
-{
-	if (!IS_ENABLED(CONFIG_BPF_JIT))
-		return false;
-
-	return regs->pc >= BPF_JIT_REGION_START &&
-	       regs->pc < BPF_JIT_REGION_END;
-}
-
 #ifdef CONFIG_BPF_JIT
 bool ex_handler_bpf(const struct exception_table_entry *ex,
 		    struct pt_regs *regs);
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index 1b9a1e2..0af70d9 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -44,11 +44,8 @@
 #define _PAGE_OFFSET(va)	(-(UL(1) << (va)))
 #define PAGE_OFFSET		(_PAGE_OFFSET(VA_BITS))
 #define KIMAGE_VADDR		(MODULES_END)
-#define BPF_JIT_REGION_START	(_PAGE_END(VA_BITS_MIN))
-#define BPF_JIT_REGION_SIZE	(SZ_128M)
-#define BPF_JIT_REGION_END	(BPF_JIT_REGION_START + BPF_JIT_REGION_SIZE)
 #define MODULES_END		(MODULES_VADDR + MODULES_VSIZE)
-#define MODULES_VADDR		(BPF_JIT_REGION_END)
+#define MODULES_VADDR		(_PAGE_END(VA_BITS_MIN))
 #define MODULES_VSIZE		(SZ_128M)
 #define VMEMMAP_START		(-(UL(1) << (VA_BITS - VMEMMAP_SHIFT)))
 #define VMEMMAP_END		(VMEMMAP_START + VMEMMAP_SIZE)
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 7b21213..e8986e6 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -994,7 +994,7 @@ static int bug_handler(struct pt_regs *regs, unsigned int esr)
 static int reserved_fault_handler(struct pt_regs *regs, unsigned int esr)
 {
 	pr_err("%s generated an invalid instruction at %pS!\n",
-		in_bpf_jit(regs) ? "BPF JIT" : "Kernel text patching",
+		"Kernel text patching",
 		(void *)instruction_pointer(regs));
 
 	/* We cannot handle this */
diff --git a/arch/arm64/mm/ptdump.c b/arch/arm64/mm/ptdump.c
index 1c40353..9bc4066 100644
--- a/arch/arm64/mm/ptdump.c
+++ b/arch/arm64/mm/ptdump.c
@@ -41,8 +41,6 @@ enum address_markers_idx {
 	{ 0 /* KASAN_SHADOW_START */,	"Kasan shadow start" },
 	{ KASAN_SHADOW_END,		"Kasan shadow end" },
 #endif
-	{ BPF_JIT_REGION_START,		"BPF start" },
-	{ BPF_JIT_REGION_END,		"BPF end" },
 	{ MODULES_VADDR,		"Modules start" },
 	{ MODULES_END,			"Modules end" },
 	{ VMALLOC_START,		"vmalloc() area" },
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 3a8a714..86c9dc0 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -1141,15 +1141,12 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 
 u64 bpf_jit_alloc_exec_limit(void)
 {
-	return BPF_JIT_REGION_SIZE;
+	return VMALLOC_END - VMALLOC_START;
 }
 
 void *bpf_jit_alloc_exec(unsigned long size)
 {
-	return __vmalloc_node_range(size, PAGE_SIZE, BPF_JIT_REGION_START,
-				    BPF_JIT_REGION_END, GFP_KERNEL,
-				    PAGE_KERNEL, 0, NUMA_NO_NODE,
-				    __builtin_return_address(0));
+	return vmalloc(size);
 }
 
 void bpf_jit_free_exec(void *addr)
-- 
1.8.3.1

[PATCH v2 bpf-next 2/2] selftests/bpf: add exception handling selftests for tp_bpf program

[Alan Maguire]

Exception handling is triggered in BPF tracing programs when
a NULL pointer is dereferenced; the exception handler zeroes the
target register and execution of the BPF program progresses.

To test exception handling then, we need to trigger a NULL pointer
dereference for a field which should never be zero; if it is, the
only explanation is the exception handler ran.  task->task_works
is the NULL pointer chosen (for a new task from fork() no work
is associated), and the task_works->func field should not be zero
if task_works is non-NULL.  Test verifies task_works and
task_works->func are 0.

Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
 tools/testing/selftests/bpf/prog_tests/exhandler.c | 43 ++++++++++++++++++++++
 tools/testing/selftests/bpf/progs/exhandler_kern.c | 43 ++++++++++++++++++++++
 2 files changed, 86 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/exhandler.c
 create mode 100644 tools/testing/selftests/bpf/progs/exhandler_kern.c

diff --git a/tools/testing/selftests/bpf/prog_tests/exhandler.c b/tools/testing/selftests/bpf/prog_tests/exhandler.c
new file mode 100644
index 0000000..118bb18
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/exhandler.c
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021, Oracle and/or its affiliates. */
+
+#include <test_progs.h>
+
+/* Test that verifies exception handling is working. fork()
+ * triggers task_newtask tracepoint; that new task will have a
+ * NULL pointer task_works, and the associated task->task_works->func
+ * should not be NULL if task_works itself is non-NULL.
+ *
+ * So to verify exception handling we want to see a NULL task_works
+ * and task_works->func; if we see this we can conclude that the
+ * exception handler ran when we attempted to dereference task->task_works
+ * and zeroed the destination register.
+ */
+#include "exhandler_kern.skel.h"
+
+void test_exhandler(void)
+{
+	int err = 0, duration = 0, status;
+	struct exhandler_kern *skel;
+	pid_t cpid;
+
+	skel = exhandler_kern__open_and_load();
+	if (CHECK(!skel, "skel_load", "skeleton failed: %d\n", err))
+		goto cleanup;
+
+	skel->bss->test_pid = getpid();
+
+	err = exhandler_kern__attach(skel);
+	if (!ASSERT_OK(err, "attach"))
+		goto cleanup;
+	cpid = fork();
+	if (!ASSERT_GT(cpid, -1, "fork failed"))
+		goto cleanup;
+	if (cpid == 0)
+		_exit(0);
+	waitpid(cpid, &status, 0);
+
+	ASSERT_NEQ(skel->bss->exception_triggered, 0, "verify exceptions occurred");
+cleanup:
+	exhandler_kern__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/exhandler_kern.c b/tools/testing/selftests/bpf/progs/exhandler_kern.c
new file mode 100644
index 0000000..f5ca142
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/exhandler_kern.c
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021, Oracle and/or its affiliates. */
+
+#include "vmlinux.h"
+
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_core_read.h>
+
+char _license[] SEC("license") = "GPL";
+
+unsigned int exception_triggered;
+int test_pid;
+
+/* TRACE_EVENT(task_newtask,
+ *         TP_PROTO(struct task_struct *p, u64 clone_flags)
+ */
+SEC("tp_btf/task_newtask")
+int BPF_PROG(trace_task_newtask, struct task_struct *task, u64 clone_flags)
+{
+	int pid = bpf_get_current_pid_tgid() >> 32;
+	struct callback_head *work;
+	void *func;
+
+	if (test_pid != pid)
+		return 0;
+
+	/* To verify we hit an exception we dereference task->task_works->func.
+	 * If task work has been added,
+	 * - task->task_works is non-NULL; and
+	 * - task->task_works->func is non-NULL also (the callback function
+	 *   must be specified for the task work.
+	 *
+	 * However, for a newly-created task, task->task_works is NULLed,
+	 * so we know the exception handler triggered if task_works is
+	 * NULL and func is NULL.
+	 */
+	work = task->task_works;
+	func = work->func;
+	if (!work && !func)
+		exception_triggered++;
+	return 0;
+}
-- 
1.8.3.1

Re: [PATCH v2 bpf-next 2/2] selftests/bpf: add exception handling selftests for tp_bpf program

[Andrii Nakryiko]

On Fri, Nov 5, 2021 at 10:11 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> Exception handling is triggered in BPF tracing programs when
> a NULL pointer is dereferenced; the exception handler zeroes the
> target register and execution of the BPF program progresses.
>
> To test exception handling then, we need to trigger a NULL pointer
> dereference for a field which should never be zero; if it is, the
> only explanation is the exception handler ran.  task->task_works
> is the NULL pointer chosen (for a new task from fork() no work
> is associated), and the task_works->func field should not be zero
> if task_works is non-NULL.  Test verifies task_works and
> task_works->func are 0.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
>  tools/testing/selftests/bpf/prog_tests/exhandler.c | 43 ++++++++++++++++++++++
>  tools/testing/selftests/bpf/progs/exhandler_kern.c | 43 ++++++++++++++++++++++
>  2 files changed, 86 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/exhandler.c
>  create mode 100644 tools/testing/selftests/bpf/progs/exhandler_kern.c
>

The test looks good, thank you!

Acked-by: Andrii Nakryiko <andrii@kernel.org>

[...]

Re: [PATCH v2 bpf-next 0/2] arm64/bpf: remove 128MB limit for BPF JIT programs

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Fri,  5 Nov 2021 16:50:44 +0000 you wrote:
> There is a 128MB limit on BPF JIT program allocations; this is
> to ensure BPF programs are in branching range of each other.
> Patch 1 in this series removes this restriction.  To verify
> exception handling still works, a test case to validate
> exception handling in BPF programs is added in patch 2.
> 
> There was previous discussion around this topic [1], in particular
> would be good to get feedback from Daniel if this approach makes
> sense.
> 
> [...]

Here is the summary with links:
  - [v2,bpf-next,1/2] arm64/bpf: remove 128MB limit for BPF JIT programs
    https://git.kernel.org/bpf/bpf-next/c/b89ddf4cca43
  - [v2,bpf-next,2/2] selftests/bpf: add exception handling selftests for tp_bpf program
    https://git.kernel.org/bpf/bpf-next/c/c23551c9c36a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html