Thread (18 messages) 18 messages, 5 authors, 2024-03-26

Re: [PATCH v2] arch/riscv: Enable kprobes when CONFIG_MODULES=n

From: Mark Rutland <mark.rutland@arm.com>
Date: 2024-03-26 16:45:25
Also in: linux-riscv, lkml

On Tue, Mar 26, 2024 at 09:15:14AM -0700, Calvin Owens wrote:
On Wednesday 03/27 at 00:24 +0900, Masami Hiramatsu wrote:
quoted
On Tue, 26 Mar 2024 14:46:10 +0000
Mark Rutland [off-list ref] wrote:
quoted
Different exectuable allocations can have different requirements. For example,
on arm64 modules need to be within 2G of the kernel image, but the kprobes XOL
areas can be anywhere in the kernel VA space.

Forcing those behind the same interface makes things *harder* for architectures
and/or makes the common code more complicated (if that ends up having to track
all those different requirements). From my PoV it'd be much better to have
separate kprobes_alloc_*() functions for kprobes which an architecture can then
choose to implement using a common library if it wants to.

I took a look at doing that using the core ifdeffery fixups from Jarkko's v6,
and it looks pretty clean to me (and works in testing on arm64):

  https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=kprobes/without-modules

Could we please start with that approach, with kprobe-specific alloc/free code
provided by the architecture?
Heh, I also noticed that dead !RWX branch in arm64 patch_map(), I was
about to send a patch to remove it.
quoted
OK, as far as I can read the code, this method also works and neat! 
(and minimum intrusion). I actually found that exposing CONFIG_ALLOC_EXECMEM
to user does not help, it should be an internal change. So hiding this change
from user is better choice. Then there is no reason to introduce the new
alloc_execmem, but just expand kprobe_alloc_insn_page() is reasonable.
I'm happy with this, it solves the first half of my problem. But I want
eBPF to work in the !MODULES case too.

I think Mark's approach can work for bpf as well, without needing to
touch module_alloc() at all? So I might be able to drop that first patch
entirely.
I'd be very happy with eBPF following the same approach, with BPF-specific
alloc/free functions that we can implement in arch code.

IIUC eBPF code *does* want to be within range of the core kernel image, so for
arm64 we'd want to factor some common logic out of module_alloc() and into
something that module_alloc() and "bpf_alloc()" (or whatever it would be
called) could use. So I don't think we'd necessarily save on touching
module_alloc(), but I think the resulting split would be better.

Thanks,
Mark.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help