[PATCH] riscv: patch: fix handling of bpf-jit execmem addresses

[PATCH] riscv: patch: fix handling of bpf-jit execmem addresses

[Wei-Jie Hung]

Commit 8718e5a3090b ("riscv: patch: skip fixmap mapping when kernel text
is already writable") gated the core_kernel_text() branch of patch_map()
on CONFIG_STRICT_KERNEL_RWX, and explicitly left the vmalloc branch
unchanged. That branch has a separate problem.

That branch only creates a temporary writable fixmap alias when
CONFIG_STRICT_MODULE_RWX is enabled, and otherwise assumes the target is
directly writable and returns the address unchanged. That assumption no
longer holds: bpf_prog_pack_alloc calls set_memory_rox() on every pack
unconditionally in alloc_new_pack(), independently of any
CONFIG_STRICT_*_RWX option, so BPF text in the vmalloc area is read-only
regardless of what CONFIG_STRICT_MODULE_RWX says.

With CONFIG_STRICT_MODULE_RWX=n, patch_map() returns the read-only
address directly, the subsequent copy_to_kernel_nofault() takes a
page fault and returns -EFAULT. bpf_arch_text_copy() turns that into
-EINVAL, which propagates through bpf_jit_binary_pack_finalize() to the
WARN_ON() in bpf_int_jit_compile() and leaves the program un-JITed.

Note that BPF cannot be fixed the way kprobes was in
commit bdc46e507b59 ("riscv: mm: make EXECMEM_KPROBES writable without
CONFIG_STRICT_MODULE_RWX"). EXECMEM_BPF is already PAGE_KERNEL, i.e.
writable at allocation time; the read-only mapping is established
afterwards by generic code in alloc_new_pack(), which arch code cannot
influence. The only place this can be handled is patch_map().

This is the same problem that was fixed on arm64 by commit b1480ed230ac
("arm64: patching: fix handling of execmem addresses"), and the fix is
the same: CONFIG_EXECMEM is what actually tracks whether the vmalloc
area can hold executable memory that needs a temporary alias to be
written. CONFIG_BPF_JIT, CONFIG_KPROBES and CONFIG_MODULES all select
it.

Note that this is not limited to CONFIG_MODULES=n. Unlike arm64, riscv
selects CONFIG_ARCH_OPTIONAL_KERNEL_RWX, so CONFIG_STRICT_MODULE_RWX can
be disabled with CONFIG_MODULES=y as well, and the bug is reachable in
that configuration too.

The !CONFIG_MMU build is unaffected: it has its own __patch_insn_set()
and __patch_insn_write() implementations and never calls patch_map().

Fixes: 2c9e5d4a0082 ("bpf: remove CONFIG_BPF_JIT dependency on CONFIG_MODULES of")
Signed-off-by: Wei-Jie Hung <imbigking12@gmail.com>
---
Based on riscv/fixes (b94cec5761d2).

Reproduced on qemu-system-riscv64 -M virt with CONFIG_BPF_JIT=y,
CONFIG_BPF_JIT_ALWAYS_ON=y and CONFIG_STRICT_MODULE_RWX=n. 
ptp_classifier_init() builds a cBPF filter from sock_init(), 
so the failure happens during boot without any userspace involved:

  WARNING: arch/riscv/net/bpf_jit_core.c:156 at bpf_int_jit_compile+0x3dc/0x43e
  Call Trace:
   bpf_int_jit_compile+0x3dc/0x43e
   __bpf_prog_select_runtime+0xec/0x186
   bpf_prog_select_runtime+0x12/0x1a
   bpf_prepare_filter+0x368/0x46a
   bpf_prog_create+0x66/0x90
   ptp_classifier_init+0x3e/0x60
   sock_init+0xc4/0xe6
   do_one_initcall+0x78/0x14a

  kernel BUG at net/core/ptp_classifier.c:227!
  Kernel panic - not syncing: Fatal exception in interrupt

The BUG_ON() is reached because CONFIG_BPF_JIT_ALWAYS_ON=y turns the
silent interpreter fallback into -ENOTSUPP. With
CONFIG_BPF_JIT_ALWAYS_ON=n the failure is silent: writing 1 to
/proc/sys/net/core/bpf_jit_enable and loading any program reproduces the
same warning, but the program simply falls back to the interpreter and
the kernel keeps running.

Tested with CONFIG_BPF_JIT=y:
  - CONFIG_MODULES=n, CONFIG_BPF_JIT_ALWAYS_ON=y:
    panics before this patch, boots after
  - CONFIG_MODULES=y, CONFIG_STRICT_MODULE_RWX=n,
    CONFIG_BPF_JIT_ALWAYS_ON=y: same panic before this patch, boots after
  - CONFIG_BPF_JIT_ALWAYS_ON=n: the warning above is emitted when a
    program is loaded from userspace before this patch, and gone after
  - riscv defconfig (CONFIG_STRICT_MODULE_RWX=y): unaffected, boots
    before and after
  - kprobes (kretprobe on __riscv_sys_openat via kprobe_events):
    works before and after this patch

 arch/riscv/kernel/patch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/patch.c b/arch/riscv/kernel/patch.c
index 2239c28981bc..2b5adf01c550 100644
--- a/arch/riscv/kernel/patch.c
+++ b/arch/riscv/kernel/patch.c
@@ -48,7 +48,7 @@ static __always_inline void *patch_map(void *addr, const unsigned int fixmap)
 		if (!IS_ENABLED(CONFIG_STRICT_KERNEL_RWX))
 			return addr;
 		phys = __pa_symbol(addr);
-	} else if (IS_ENABLED(CONFIG_STRICT_MODULE_RWX)) {
+	} else if (IS_ENABLED(CONFIG_EXECMEM)) {
 		struct page *page = vmalloc_to_page(addr);
 
 		BUG_ON(!page);
-- 
2.43.0