Subject: Re: [PATCH v2 1/3] module: Rename module_alloc() to text_alloc() and move to kernel proper

From: Jarkko Sakkinen <[email-masked]>
Date: Tue, 14 Jul 2020 15:25:18 +0300

On Tue, Jul 14, 2020 at 01:29:27PM +0200, Peter Zijlstra wrote:
> On Tue, Jul 14, 2020 at 11:28:27AM +0100, Will Deacon wrote:
> 
> > As Ard says, module_alloc() _is_ special, in the sense that the virtual
> > memory it allocates wants to be close to the kernel text, whereas the
> > concept of allocating executable memory is broader and doesn't have these
> > restrictions. So, while I'm in favour of having a text_alloc() interface
> > that can be used by callers which only require an executable mapping, I'd
> > much prefer for the module_alloc() code to remain for, err, modules.
> 
> So on x86 all those things (kprobes, bpf, ftrace) require that same
> closeness.
> 
> An interface like the late vmalloc_exec() will simply not work for us.
> 
> We recently talked about arm64-kprobes and how you're not doing any of
> the optimizations and fully rely on the exception return. And I see
> you're one of the few archs that has bpf_jit_alloc_exec() (also,
> shouldn't you be using PAGE_KERNEL_EXEC there?). But the BPF core seems
> to use module_alloc() as a default means of allocating text.
> 
> 
> So what should this look like? Have a text_alloc() with an argument that
> indicates where? But then I suppose we also need a means to manage PLT
> entries. Otherwise I don't exactly see how you're going to call BPF
> code, or how that BPF stuff is going to call back into its helpers.

To make this less of a havoc to arch maintainers what if:

void * __weak module_alloc(unsigned long size)
{
	if (IS_ENABLED(HAS_TEXT_ALLOC))
		return text_alloc(size);

	return __vmalloc_node_range(size, 1, VMALLOC_START, VMALLOC_END,
			GFP_KERNEL, PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS,
			NUMA_NO_NODE, __builtin_return_address(0));
}

Then in arch/x86/Kconfig I could put:

config HAS_TEXT_ALLOC
	def_bool y

This would scale down the patch set just to add kernel/text.c and
arch/x86/kernel/text.c, and allows gradual migration to other arch's.

/Jarkko