diff mbox series

Added ability to vmalloc executable memory

Message ID 20221209131052.64235-1-xander.moerkerken@omron.com (mailing list archive)
State New
Headers show
Series Added ability to vmalloc executable memory | expand

Commit Message

Xander Dec. 9, 2022, 1:10 p.m. UTC 
From: Xander Moerkerken <xander.moerkerken@gmail.com>

Since release 5.8-rc1 the pgprot got removed from __vmalloc
because the only usage was PAGE_KERNEL as argument.
However, this removes the ability to input other arguments
such as 'PAGE_KERNEL_EXEC', which can be used to allocate
memory in which you can execute. For this reason a new
function is introduced called '__vmalloc_exec'.

Signed-off-by: Xander Moerkerken <xander.moerkerken@omron.com>
---
 include/linux/vmalloc.h | 1 +
 mm/vmalloc.c            | 8 ++++++++
 2 files changed, 9 insertions(+)

Comments

Mark Rutland Dec. 9, 2022, 1:17 p.m. UTC | #1 
On Fri, Dec 09, 2022 at 02:10:52PM +0100, Xander Moerkerken wrote:
> From: Xander Moerkerken <xander.moerkerken@gmail.com>
> 
> Since release 5.8-rc1 the pgprot got removed from __vmalloc
> because the only usage was PAGE_KERNEL as argument.
> However, this removes the ability to input other arguments
> such as 'PAGE_KERNEL_EXEC', which can be used to allocate
> memory in which you can execute. For this reason a new
> function is introduced called '__vmalloc_exec'.
> 
> Signed-off-by: Xander Moerkerken <xander.moerkerken@omron.com>

What is this going to be used for? There's no user from this patch alone, as a
module or otherwise.

Mark.

> ---
>  include/linux/vmalloc.h | 1 +
>  mm/vmalloc.c            | 8 ++++++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index 096d48aa3437..10c46513b6b2 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -147,6 +147,7 @@ extern void *vzalloc_node(unsigned long size, int node) __alloc_size(1);
>  extern void *vmalloc_32(unsigned long size) __alloc_size(1);
>  extern void *vmalloc_32_user(unsigned long size) __alloc_size(1);
>  extern void *__vmalloc(unsigned long size, gfp_t gfp_mask) __alloc_size(1);
> +extern void *__vmalloc_exec(unsigned long size, gfp_t gfp_mask) __alloc_size(1);
>  extern void *__vmalloc_node_range(unsigned long size, unsigned long align,
>  			unsigned long start, unsigned long end, gfp_t gfp_mask,
>  			pgprot_t prot, unsigned long vm_flags, int node,
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index ccaa461998f3..8fd01ed7082b 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -3294,6 +3294,14 @@ void *__vmalloc(unsigned long size, gfp_t gfp_mask)
>  }
>  EXPORT_SYMBOL(__vmalloc);
>  
> +
> +void *__vmalloc_exec(unsigned long size, gfp_t gfp_mask)
> +{
> +	return __vmalloc_node_prot(size, 1, gfp_mask, PAGE_KERNEL_EXEC,
> +	                          NUMA_NO_NODE, __builtin_return_address(0));
> +}
> +EXPORT_SYMBOL(__vmalloc_exec);
> +
>  /**
>   * vmalloc - allocate virtually contiguous memory
>   * @size:    allocation size
> -- 
> 2.37.2
>
Xander Dec. 9, 2022, 1:38 p.m. UTC | #2 
The pgprot parameter got removed because, according to the commit log, for
no other apparent reason than it being called with 'PAGE_KERNEL' as an
argument in the whole kernel. Therefore it got removed.
This removed the ability to allocate virtual memory with executing rights.
My use case comes from ioremap().
I think this is useful for others too.

I don't see why this pgprot parameter got removed but this is the
alternative to reverting it to the older 5.7 function.

On Fri, 9 Dec 2022 at 14:17, Mark Rutland <mark.rutland@arm.com> wrote:

> On Fri, Dec 09, 2022 at 02:10:52PM +0100, Xander Moerkerken wrote:
> > From: Xander Moerkerken <xander.moerkerken@gmail.com>
> >
> > Since release 5.8-rc1 the pgprot got removed from __vmalloc
> > because the only usage was PAGE_KERNEL as argument.
> > However, this removes the ability to input other arguments
> > such as 'PAGE_KERNEL_EXEC', which can be used to allocate
> > memory in which you can execute. For this reason a new
> > function is introduced called '__vmalloc_exec'.
> >
> > Signed-off-by: Xander Moerkerken <xander.moerkerken@omron.com>
>
> What is this going to be used for? There's no user from this patch alone,
> as a
> module or otherwise.
>
> Mark.
>
> > ---
> >  include/linux/vmalloc.h | 1 +
> >  mm/vmalloc.c            | 8 ++++++++
> >  2 files changed, 9 insertions(+)
> >
> > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> > index 096d48aa3437..10c46513b6b2 100644
> > --- a/include/linux/vmalloc.h
> > +++ b/include/linux/vmalloc.h
> > @@ -147,6 +147,7 @@ extern void *vzalloc_node(unsigned long size, int
> node) __alloc_size(1);
> >  extern void *vmalloc_32(unsigned long size) __alloc_size(1);
> >  extern void *vmalloc_32_user(unsigned long size) __alloc_size(1);
> >  extern void *__vmalloc(unsigned long size, gfp_t gfp_mask)
> __alloc_size(1);
> > +extern void *__vmalloc_exec(unsigned long size, gfp_t gfp_mask)
> __alloc_size(1);
> >  extern void *__vmalloc_node_range(unsigned long size, unsigned long
> align,
> >                       unsigned long start, unsigned long end, gfp_t
> gfp_mask,
> >                       pgprot_t prot, unsigned long vm_flags, int node,
> > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > index ccaa461998f3..8fd01ed7082b 100644
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -3294,6 +3294,14 @@ void *__vmalloc(unsigned long size, gfp_t
> gfp_mask)
> >  }
> >  EXPORT_SYMBOL(__vmalloc);
> >
> > +
> > +void *__vmalloc_exec(unsigned long size, gfp_t gfp_mask)
> > +{
> > +     return __vmalloc_node_prot(size, 1, gfp_mask, PAGE_KERNEL_EXEC,
> > +                               NUMA_NO_NODE,
> __builtin_return_address(0));
> > +}
> > +EXPORT_SYMBOL(__vmalloc_exec);
> > +
> >  /**
> >   * vmalloc - allocate virtually contiguous memory
> >   * @size:    allocation size
> > --
> > 2.37.2
> >
>
Christophe Leroy Dec. 9, 2022, 1:46 p.m. UTC | #3 
Le 09/12/2022 à 14:38, Xander a écrit :
> 	
> The pgprot parameter got removed because, according to the commit log, 
> for no other apparent reason than it being called with 'PAGE_KERNEL' as 
> an argument in the whole kernel. Therefore it got removed.
> This removed the ability to allocate virtual memory with executing rights.
> My use case comes from ioremap().
> I think this is useful for others too.
> 
> I don't see why this pgprot parameter got removed but this is the 
> alternative to reverting it to the older 5.7 function.

Please avoid top-posting, and use only plain text.

I think you don't answer to Mark's question.

You are adding a new function that no driver uses apparently. If you are 
working on some piece of code that needs this new fonction, you can send 
this patch as part of a patch series including that code.

By the way, when you need executable memory, the fonction to use is 
module_alloc(), that's the only function that garanties real executable 
memory on all platforms. For instance, on some powerpc, setting the X 
bit is not enough to get executable memory in vmalloc space.

Christophe

> 
> On Fri, 9 Dec 2022 at 14:17, Mark Rutland <mark.rutland@arm.com 
> <mailto:mark.rutland@arm.com>> wrote:
> 
>     On Fri, Dec 09, 2022 at 02:10:52PM +0100, Xander Moerkerken wrote:
>      > From: Xander Moerkerken <xander.moerkerken@gmail.com
>     <mailto:xander.moerkerken@gmail.com>>
>      >
>      > Since release 5.8-rc1 the pgprot got removed from __vmalloc
>      > because the only usage was PAGE_KERNEL as argument.
>      > However, this removes the ability to input other arguments
>      > such as 'PAGE_KERNEL_EXEC', which can be used to allocate
>      > memory in which you can execute. For this reason a new
>      > function is introduced called '__vmalloc_exec'.
>      >
>      > Signed-off-by: Xander Moerkerken <xander.moerkerken@omron.com
>     <mailto:xander.moerkerken@omron.com>>
> 
>     What is this going to be used for? There's no user from this patch
>     alone, as a
>     module or otherwise.
> 
>     Mark.
> 
>      > ---
>      >  include/linux/vmalloc.h | 1 +
>      >  mm/vmalloc.c            | 8 ++++++++
>      >  2 files changed, 9 insertions(+)
>      >
>      > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
>      > index 096d48aa3437..10c46513b6b2 100644
>      > --- a/include/linux/vmalloc.h
>      > +++ b/include/linux/vmalloc.h
>      > @@ -147,6 +147,7 @@ extern void *vzalloc_node(unsigned long size,
>     int node) __alloc_size(1);
>      >  extern void *vmalloc_32(unsigned long size) __alloc_size(1);
>      >  extern void *vmalloc_32_user(unsigned long size) __alloc_size(1);
>      >  extern void *__vmalloc(unsigned long size, gfp_t gfp_mask)
>     __alloc_size(1);
>      > +extern void *__vmalloc_exec(unsigned long size, gfp_t gfp_mask)
>     __alloc_size(1);
>      >  extern void *__vmalloc_node_range(unsigned long size, unsigned
>     long align,
>      >                       unsigned long start, unsigned long end,
>     gfp_t gfp_mask,
>      >                       pgprot_t prot, unsigned long vm_flags, int
>     node,
>      > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>      > index ccaa461998f3..8fd01ed7082b 100644
>      > --- a/mm/vmalloc.c
>      > +++ b/mm/vmalloc.c
>      > @@ -3294,6 +3294,14 @@ void *__vmalloc(unsigned long size, gfp_t
>     gfp_mask)
>      >  }
>      >  EXPORT_SYMBOL(__vmalloc);
>      >
>      > +
>      > +void *__vmalloc_exec(unsigned long size, gfp_t gfp_mask)
>      > +{
>      > +     return __vmalloc_node_prot(size, 1, gfp_mask, PAGE_KERNEL_EXEC,
>      > +                               NUMA_NO_NODE,
>     __builtin_return_address(0));
>      > +}
>      > +EXPORT_SYMBOL(__vmalloc_exec);
>      > +
>      >  /**
>      >   * vmalloc - allocate virtually contiguous memory
>      >   * @size:    allocation size
>      > --
>      > 2.37.2
>      >
>
Xander Dec. 9, 2022, 1:51 p.m. UTC | #4 
On Fri, 9 Dec 2022 at 14:46, Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 09/12/2022 à 14:38, Xander a écrit :
> >
> > The pgprot parameter got removed because, according to the commit log,
> > for no other apparent reason than it being called with 'PAGE_KERNEL' as
> > an argument in the whole kernel. Therefore it got removed.
> > This removed the ability to allocate virtual memory with executing rights.
> > My use case comes from ioremap().
> > I think this is useful for others too.
> >
> > I don't see why this pgprot parameter got removed but this is the
> > alternative to reverting it to the older 5.7 function.
>
> Please avoid top-posting, and use only plain text.
>
> I think you don't answer to Mark's question.
>
> You are adding a new function that no driver uses apparently. If you are
> working on some piece of code that needs this new fonction, you can send
> this patch as part of a patch series including that code.
>
> By the way, when you need executable memory, the fonction to use is
> module_alloc(), that's the only function that garanties real executable
> memory on all platforms. For instance, on some powerpc, setting the X
> bit is not enough to get executable memory in vmalloc space.
>
> Christophe
>

Loud and clear, thanks for the feedback.

> >
> > On Fri, 9 Dec 2022 at 14:17, Mark Rutland <mark.rutland@arm.com
> > <mailto:mark.rutland@arm.com>> wrote:
> >
> >     On Fri, Dec 09, 2022 at 02:10:52PM +0100, Xander Moerkerken wrote:
> >      > From: Xander Moerkerken <xander.moerkerken@gmail.com
> >     <mailto:xander.moerkerken@gmail.com>>
> >      >
> >      > Since release 5.8-rc1 the pgprot got removed from __vmalloc
> >      > because the only usage was PAGE_KERNEL as argument.
> >      > However, this removes the ability to input other arguments
> >      > such as 'PAGE_KERNEL_EXEC', which can be used to allocate
> >      > memory in which you can execute. For this reason a new
> >      > function is introduced called '__vmalloc_exec'.
> >      >
> >      > Signed-off-by: Xander Moerkerken <xander.moerkerken@omron.com
> >     <mailto:xander.moerkerken@omron.com>>
> >
> >     What is this going to be used for? There's no user from this patch
> >     alone, as a
> >     module or otherwise.
> >
> >     Mark.
> >
> >      > ---
> >      >  include/linux/vmalloc.h | 1 +
> >      >  mm/vmalloc.c            | 8 ++++++++
> >      >  2 files changed, 9 insertions(+)
> >      >
> >      > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> >      > index 096d48aa3437..10c46513b6b2 100644
> >      > --- a/include/linux/vmalloc.h
> >      > +++ b/include/linux/vmalloc.h
> >      > @@ -147,6 +147,7 @@ extern void *vzalloc_node(unsigned long size,
> >     int node) __alloc_size(1);
> >      >  extern void *vmalloc_32(unsigned long size) __alloc_size(1);
> >      >  extern void *vmalloc_32_user(unsigned long size) __alloc_size(1);
> >      >  extern void *__vmalloc(unsigned long size, gfp_t gfp_mask)
> >     __alloc_size(1);
> >      > +extern void *__vmalloc_exec(unsigned long size, gfp_t gfp_mask)
> >     __alloc_size(1);
> >      >  extern void *__vmalloc_node_range(unsigned long size, unsigned
> >     long align,
> >      >                       unsigned long start, unsigned long end,
> >     gfp_t gfp_mask,
> >      >                       pgprot_t prot, unsigned long vm_flags, int
> >     node,
> >      > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> >      > index ccaa461998f3..8fd01ed7082b 100644
> >      > --- a/mm/vmalloc.c
> >      > +++ b/mm/vmalloc.c
> >      > @@ -3294,6 +3294,14 @@ void *__vmalloc(unsigned long size, gfp_t
> >     gfp_mask)
> >      >  }
> >      >  EXPORT_SYMBOL(__vmalloc);
> >      >
> >      > +
> >      > +void *__vmalloc_exec(unsigned long size, gfp_t gfp_mask)
> >      > +{
> >      > +     return __vmalloc_node_prot(size, 1, gfp_mask, PAGE_KERNEL_EXEC,
> >      > +                               NUMA_NO_NODE,
> >     __builtin_return_address(0));
> >      > +}
> >      > +EXPORT_SYMBOL(__vmalloc_exec);
> >      > +
> >      >  /**
> >      >   * vmalloc - allocate virtually contiguous memory
> >      >   * @size:    allocation size
> >      > --
> >      > 2.37.2
> >      >
> >
Mark Rutland Dec. 9, 2022, 2:17 p.m. UTC | #5 
On Fri, Dec 09, 2022 at 01:46:05PM +0000, Christophe Leroy wrote:
> 
> 
> Le 09/12/2022 à 14:38, Xander a écrit :
> > 	
> > The pgprot parameter got removed because, according to the commit log, 
> > for no other apparent reason than it being called with 'PAGE_KERNEL' as 
> > an argument in the whole kernel. Therefore it got removed.
> > This removed the ability to allocate virtual memory with executing rights.
> > My use case comes from ioremap().
> > I think this is useful for others too.
> > 
> > I don't see why this pgprot parameter got removed but this is the 
> > alternative to reverting it to the older 5.7 function.
> 
> Please avoid top-posting, and use only plain text.
> 
> I think you don't answer to Mark's question.
> 
> You are adding a new function that no driver uses apparently. If you are 
> working on some piece of code that needs this new fonction, you can send 
> this patch as part of a patch series including that code.

Yup, that was what I was getting at. Thanks for stating that much more clearly
than I did. :)

> By the way, when you need executable memory, the fonction to use is 
> module_alloc(), that's the only function that garanties real executable 
> memory on all platforms. For instance, on some powerpc, setting the X 
> bit is not enough to get executable memory in vmalloc space.

Yup, and likewise on arm64 there are other constraints to consider, e.g. branch
ranges, whether or not to set PROT_BTI and/or other prot bits in future.

Further, I'm very wary of exporting a generic interface to make some code
executable without an understanding and documenting the precise constraints on
its use, and I'm generally wary of doing that for some arbitrary code given
that could violate other expectations that affect the kernel generally (e.g.
gadgets for ROP/JOP/speculation, RELIABLE_STACKTRACE expectations, and general
things like whether the code will play with CPU control bits or flags in an
unexpected way).

Mark.

> Christophe
> 
> > 
> > On Fri, 9 Dec 2022 at 14:17, Mark Rutland <mark.rutland@arm.com 
> > <mailto:mark.rutland@arm.com>> wrote:
> > 
> >     On Fri, Dec 09, 2022 at 02:10:52PM +0100, Xander Moerkerken wrote:
> >      > From: Xander Moerkerken <xander.moerkerken@gmail.com
> >     <mailto:xander.moerkerken@gmail.com>>
> >      >
> >      > Since release 5.8-rc1 the pgprot got removed from __vmalloc
> >      > because the only usage was PAGE_KERNEL as argument.
> >      > However, this removes the ability to input other arguments
> >      > such as 'PAGE_KERNEL_EXEC', which can be used to allocate
> >      > memory in which you can execute. For this reason a new
> >      > function is introduced called '__vmalloc_exec'.
> >      >
> >      > Signed-off-by: Xander Moerkerken <xander.moerkerken@omron.com
> >     <mailto:xander.moerkerken@omron.com>>
> > 
> >     What is this going to be used for? There's no user from this patch
> >     alone, as a
> >     module or otherwise.
> > 
> >     Mark.
> > 
> >      > ---
> >      >  include/linux/vmalloc.h | 1 +
> >      >  mm/vmalloc.c            | 8 ++++++++
> >      >  2 files changed, 9 insertions(+)
> >      >
> >      > diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> >      > index 096d48aa3437..10c46513b6b2 100644
> >      > --- a/include/linux/vmalloc.h
> >      > +++ b/include/linux/vmalloc.h
> >      > @@ -147,6 +147,7 @@ extern void *vzalloc_node(unsigned long size,
> >     int node) __alloc_size(1);
> >      >  extern void *vmalloc_32(unsigned long size) __alloc_size(1);
> >      >  extern void *vmalloc_32_user(unsigned long size) __alloc_size(1);
> >      >  extern void *__vmalloc(unsigned long size, gfp_t gfp_mask)
> >     __alloc_size(1);
> >      > +extern void *__vmalloc_exec(unsigned long size, gfp_t gfp_mask)
> >     __alloc_size(1);
> >      >  extern void *__vmalloc_node_range(unsigned long size, unsigned
> >     long align,
> >      >                       unsigned long start, unsigned long end,
> >     gfp_t gfp_mask,
> >      >                       pgprot_t prot, unsigned long vm_flags, int
> >     node,
> >      > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> >      > index ccaa461998f3..8fd01ed7082b 100644
> >      > --- a/mm/vmalloc.c
> >      > +++ b/mm/vmalloc.c
> >      > @@ -3294,6 +3294,14 @@ void *__vmalloc(unsigned long size, gfp_t
> >     gfp_mask)
> >      >  }
> >      >  EXPORT_SYMBOL(__vmalloc);
> >      >
> >      > +
> >      > +void *__vmalloc_exec(unsigned long size, gfp_t gfp_mask)
> >      > +{
> >      > +     return __vmalloc_node_prot(size, 1, gfp_mask, PAGE_KERNEL_EXEC,
> >      > +                               NUMA_NO_NODE,
> >     __builtin_return_address(0));
> >      > +}
> >      > +EXPORT_SYMBOL(__vmalloc_exec);
> >      > +
> >      >  /**
> >      >   * vmalloc - allocate virtually contiguous memory
> >      >   * @size:    allocation size
> >      > --
> >      > 2.37.2
> >      >
> >
kernel test robot Dec. 9, 2022, 2:18 p.m. UTC | #6 
Hi Xander,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on linus/master v6.1-rc8 next-20221208]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Xander-Moerkerken/Added-ability-to-vmalloc-executable-memory/20221209-211212
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20221209131052.64235-1-xander.moerkerken%40omron.com
patch subject: [PATCH] Added ability to vmalloc executable memory
config: powerpc-allnoconfig
compiler: powerpc-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/86c4449bb255bd5320d7b260034db7845878bc12
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Xander-Moerkerken/Added-ability-to-vmalloc-executable-memory/20221209-211212
        git checkout 86c4449bb255bd5320d7b260034db7845878bc12
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

   mm/vmalloc.c: In function '__vmalloc_exec':
>> mm/vmalloc.c:3314:16: error: implicit declaration of function '__vmalloc_node_prot'; did you mean '__vmalloc_node'? [-Werror=implicit-function-declaration]
    3314 |         return __vmalloc_node_prot(size, 1, gfp_mask, PAGE_KERNEL_EXEC,
         |                ^~~~~~~~~~~~~~~~~~~
         |                __vmalloc_node
>> mm/vmalloc.c:3314:16: warning: returning 'int' from a function with return type 'void *' makes pointer from integer without a cast [-Wint-conversion]
    3314 |         return __vmalloc_node_prot(size, 1, gfp_mask, PAGE_KERNEL_EXEC,
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    3315 |                                   NUMA_NO_NODE, __builtin_return_address(0));
         |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +3314 mm/vmalloc.c

  3310	
  3311	
  3312	void *__vmalloc_exec(unsigned long size, gfp_t gfp_mask)
  3313	{
> 3314		return __vmalloc_node_prot(size, 1, gfp_mask, PAGE_KERNEL_EXEC,
  3315		                          NUMA_NO_NODE, __builtin_return_address(0));
  3316	}
  3317	EXPORT_SYMBOL(__vmalloc_exec);
  3318
kernel test robot Dec. 9, 2022, 4:10 p.m. UTC | #7 
Hi Xander,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on linus/master v6.1-rc8 next-20221208]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Xander-Moerkerken/Added-ability-to-vmalloc-executable-memory/20221209-211212
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20221209131052.64235-1-xander.moerkerken%40omron.com
patch subject: [PATCH] Added ability to vmalloc executable memory
config: hexagon-randconfig-r041-20221209
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 6e4cea55f0d1104408b26ac574566a0e4de48036)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/86c4449bb255bd5320d7b260034db7845878bc12
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Xander-Moerkerken/Added-ability-to-vmalloc-executable-memory/20221209-211212
        git checkout 86c4449bb255bd5320d7b260034db7845878bc12
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from mm/vmalloc.c:14:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from mm/vmalloc.c:14:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from mm/vmalloc.c:14:
   In file included from include/linux/highmem.h:12:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/hexagon/include/asm/io.h:334:
   include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
>> mm/vmalloc.c:3314:9: error: call to undeclared function '__vmalloc_node_prot'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
           return __vmalloc_node_prot(size, 1, gfp_mask, PAGE_KERNEL_EXEC,
                  ^
>> mm/vmalloc.c:3314:9: error: incompatible integer to pointer conversion returning 'int' from a function with result type 'void *' [-Wint-conversion]
           return __vmalloc_node_prot(size, 1, gfp_mask, PAGE_KERNEL_EXEC,
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   6 warnings and 2 errors generated.


vim +/__vmalloc_node_prot +3314 mm/vmalloc.c

  3310	
  3311	
  3312	void *__vmalloc_exec(unsigned long size, gfp_t gfp_mask)
  3313	{
> 3314		return __vmalloc_node_prot(size, 1, gfp_mask, PAGE_KERNEL_EXEC,
  3315		                          NUMA_NO_NODE, __builtin_return_address(0));
  3316	}
  3317	EXPORT_SYMBOL(__vmalloc_exec);
  3318
kernel test robot Dec. 9, 2022, 7:01 p.m. UTC | #8 
Hi Xander,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on linus/master v6.1-rc8 next-20221208]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Xander-Moerkerken/Added-ability-to-vmalloc-executable-memory/20221209-211212
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20221209131052.64235-1-xander.moerkerken%40omron.com
patch subject: [PATCH] Added ability to vmalloc executable memory
config: i386-randconfig-a013
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/86c4449bb255bd5320d7b260034db7845878bc12
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Xander-Moerkerken/Added-ability-to-vmalloc-executable-memory/20221209-211212
        git checkout 86c4449bb255bd5320d7b260034db7845878bc12
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All error/warnings (new ones prefixed by >>):

>> mm/vmalloc.c:3314:9: error: implicit declaration of function '__vmalloc_node_prot' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
           return __vmalloc_node_prot(size, 1, gfp_mask, PAGE_KERNEL_EXEC,
                  ^
>> mm/vmalloc.c:3314:9: warning: incompatible integer to pointer conversion returning 'int' from a function with result type 'void *' [-Wint-conversion]
           return __vmalloc_node_prot(size, 1, gfp_mask, PAGE_KERNEL_EXEC,
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 warning and 1 error generated.


vim +/__vmalloc_node_prot +3314 mm/vmalloc.c

  3310	
  3311	
  3312	void *__vmalloc_exec(unsigned long size, gfp_t gfp_mask)
  3313	{
> 3314		return __vmalloc_node_prot(size, 1, gfp_mask, PAGE_KERNEL_EXEC,
  3315		                          NUMA_NO_NODE, __builtin_return_address(0));
  3316	}
  3317	EXPORT_SYMBOL(__vmalloc_exec);
  3318
Christoph Hellwig Dec. 12, 2022, 8:41 a.m. UTC | #9 
On Fri, Dec 09, 2022 at 02:10:52PM +0100, Xander Moerkerken wrote:
> From: Xander Moerkerken <xander.moerkerken@gmail.com>
> 
> Since release 5.8-rc1 the pgprot got removed from __vmalloc
> because the only usage was PAGE_KERNEL as argument.
> However, this removes the ability to input other arguments
> such as 'PAGE_KERNEL_EXEC', which can be used to allocate
> memory in which you can execute.

.. which is very much intentional.
diff mbox series

Patch

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 096d48aa3437..10c46513b6b2 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -147,6 +147,7 @@  extern void *vzalloc_node(unsigned long size, int node) __alloc_size(1);
 extern void *vmalloc_32(unsigned long size) __alloc_size(1);
 extern void *vmalloc_32_user(unsigned long size) __alloc_size(1);
 extern void *__vmalloc(unsigned long size, gfp_t gfp_mask) __alloc_size(1);
+extern void *__vmalloc_exec(unsigned long size, gfp_t gfp_mask) __alloc_size(1);
 extern void *__vmalloc_node_range(unsigned long size, unsigned long align,
 			unsigned long start, unsigned long end, gfp_t gfp_mask,
 			pgprot_t prot, unsigned long vm_flags, int node,
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index ccaa461998f3..8fd01ed7082b 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -3294,6 +3294,14 @@  void *__vmalloc(unsigned long size, gfp_t gfp_mask)
 }
 EXPORT_SYMBOL(__vmalloc);
 
+
+void *__vmalloc_exec(unsigned long size, gfp_t gfp_mask)
+{
+	return __vmalloc_node_prot(size, 1, gfp_mask, PAGE_KERNEL_EXEC,
+	                          NUMA_NO_NODE, __builtin_return_address(0));
+}
+EXPORT_SYMBOL(__vmalloc_exec);
+
 /**
  * vmalloc - allocate virtually contiguous memory
  * @size:    allocation size