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

From: Jarkko Sakkinen <[email-masked]>
Date: Fri, 24 Jul 2020 18:46:59 +0300

On Fri, Jul 24, 2020 at 10:05:08AM +0200, Jessica Yu wrote:
> +++ Jarkko Sakkinen [24/07/20 10:36 +0300]:
> > On Thu, Jul 23, 2020 at 03:42:09PM +0300, Ard Biesheuvel wrote:
> > > On Thu, 23 Jul 2020 at 04:52, Jarkko Sakkinen
> > > <[email-masked]> wrote:
> > > >
> > > > On Thu, Jul 16, 2020 at 06:49:09PM +0200, Christophe Leroy wrote:
> > > > > Jarkko Sakkinen <[email-masked]> a écrit :
> > > > >
> > > > > > Rename module_alloc() to text_alloc() and module_memfree() to
> > > > > > text_memfree(), and move them to kernel/text.c, which is unconditionally
> > > > > > compiled to the kernel proper. This allows kprobes, ftrace and bpf to
> > > > > > allocate space for executable code without requiring to compile the modules
> > > > > > support (CONFIG_MODULES=y) in.
> > > > >
> > > > > You are not changing enough in powerpc to have this work.
> > > > > On powerpc 32 bits (6xx), when STRICT_KERNEL_RWX is selected, the vmalloc
> > > > > space is set to NX (no exec) at segment level (ie by 256Mbytes zone) unless
> > > > > CONFIG_MODULES is selected.
> > > > >
> > > > > Christophe
> > > >
> > > > This has been deduced down to:
> > > >
> > > > https://lore.kernel.org/lkml/[email-masked]/
> > > >
> > > > I.e. not intruding PPC anymore :-)
> > > >
> > > 
> > > Ok, so after the elaborate discussion we had between Jessica, Russell,
> > > Peter, Will, Mark, you and myself, where we pointed out that
> > > a) a single text_alloc() abstraction for bpf, kprobes and ftrace does
> > > not fit other architectures very well, and
> > > b) that module_alloc() is not suitable as a default to base text_alloc() on,
> > 
> > In the latest iteration (v5) it is conditionally available only if arch
> > defines and fallback has been removed.
> > 
> > > you went ahead and implemented that anyway, but only cc'ing Peter,
> > > akpm, Masami and the mm list this time?
> > 
> > No problems with that. Actually each patch gets everything that
> > get_maintainer.pl gives with a cc cmd script, not just the ones
> > explicitly listed in the patch.
> > 
> > Should I explicitly CC you to the next version? I'm happy to grow
> > the list when requested.
> 
> Yes, please CC everybody that was part of the discussion last time
> especially during v2, and please use a consistent CC list for the
> whole patchset. It is difficult to review when you only receive patch
> 1 out of 6 with no mention of text_alloc() anywhere and without being
> CC'd on the cover letter.
> 
> Jessica

I'll make the following additions to the CC list (in alphabetical
order):

  Cc: Ard Biesheuvel <[email-masked]>
  Cc: Jessica Yu <[email-masked]>
  Cc: Mark Rutland <[email-masked]>,
  Cc: Mike Rapoport <[email-masked]>
  Cc: Russell King <[email-masked]>
  Cc: Will Deacon <[email-masked]>

This is also reflected now in the kprobes branch of
https://github.com/jsakkine-intel/linux-sgx.git. My candidate patches
are located in my SGX tree because I mainly use them when poking SGX.
The patches are kind of sidekick of SGX upstreaming process.

/Jarkko