All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
@ 2017-11-21 13:56 Aleksandar Markovic
  2017-11-21 16:50 ` Randy Dunlap
  2017-11-21 20:53 ` David Daney
  0 siblings, 2 replies; 17+ messages in thread
From: Aleksandar Markovic @ 2017-11-21 13:56 UTC (permalink / raw)
  To: linux-mips
  Cc: Miodrag Dinic, Aleksandar Markovic, Andrew Morton, Dengcheng Zhu,
	Ding Tianhong, Douglas Leung, Frederic Weisbecker, Goran Ferenc,
	Ingo Molnar, James Cowgill, James Hogan, Jonathan Corbet,
	linux-doc, linux-kernel, Marc Zyngier, Matt Redfearn, Mimi Zohar,
	Paul Burton, Paul E. McKenney, Petar Jovanovic, Raghu Gandham,
	Ralf Baechle, Thomas Gleixner, Tom Saeger

From: Miodrag Dinic <miodrag.dinic@mips.com>

Add a new kernel parameter to override the default behavior related
to the decision whether to set up stack as non-executable in function
mips_elf_read_implies_exec().

The new parameter is used to control non executable stack and heap,
regardless of PT_GNU_STACK entry. This does apply to both stack and
heap, despite the name.

Allowed values:

nonxstack=on	Force non-exec stack & heap
nonxstack=off	Force executable stack & heap

If this parameter is omitted, kernel behavior remains the same as it
was before this patch is applied.

This functionality is convenient during debugging and is especially
useful for Android development where non-exec stack is required.

Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
---
 Documentation/admin-guide/kernel-parameters.txt | 11 +++++++
 arch/mips/kernel/elf.c                          | 39 +++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index b74e133..99464ee 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2614,6 +2614,17 @@
 			noexec32=off: disable non-executable mappings
 				read implies executable mappings
 
+	nonxstack	[MIPS]
+			Force setting up stack and heap as non-executable or
+			executable regardless of PT_GNU_STACK entry. Both
+			stack and heap are affected, despite the name. Valid
+			arguments: on, off.
+			nonxstack=on:	Force non-executable stack and heap
+			nonxstack=off:	Force executable stack and heap
+			If ommited, stack and heap will or will not be set
+			up as non-executable depending on PT_GNU_STACK
+			entry and possibly other factors.
+
 	nofpu		[MIPS,SH] Disable hardware FPU at boot time.
 
 	nofxsr		[BUGS=X86-32] Disables x86 floating point extended
diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c
index 731325a..28ef7f3 100644
--- a/arch/mips/kernel/elf.c
+++ b/arch/mips/kernel/elf.c
@@ -326,8 +326,47 @@ void mips_set_personality_nan(struct arch_elf_state *state)
 	}
 }
 
+static int nonxstack = EXSTACK_DEFAULT;
+
+/*
+ * kernel parameter: nonxstack=on|off
+ *
+ *   Force setting up stack and heap as non-executable or
+ *   executable regardless of PT_GNU_STACK entry. Both
+ *   stack and heap are affected, despite the name. Valid
+ *   arguments: on, off.
+ *
+ *     nonxstack=on:   Force non-executable stack and heap
+ *     nonxstack=off:  Force executable stack and heap
+ *
+ *   If ommited, stack and heap will or will not be set
+ *   up as non-executable depending on PT_GNU_STACK
+ *   entry and possibly other factors.
+ */
+static int __init nonxstack_setup(char *str)
+{
+	if (!strcmp(str, "on"))
+		nonxstack = EXSTACK_DISABLE_X;
+	else if (!strcmp(str, "off"))
+		nonxstack = EXSTACK_ENABLE_X;
+	else
+		pr_err("Malformed nonxstack format! nonxstack=on|off\n");
+
+	return 1;
+}
+__setup("nonxstack=", nonxstack_setup);
+
 int mips_elf_read_implies_exec(void *elf_ex, int exstack)
 {
+	switch (nonxstack) {
+	case EXSTACK_DISABLE_X:
+		return 0;
+	case EXSTACK_ENABLE_X:
+		return 1;
+	default:
+		break;
+	}
+
 	if (exstack != EXSTACK_DISABLE_X) {
 		/* The binary doesn't request a non-executable stack */
 		return 1;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
  2017-11-21 13:56 [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter Aleksandar Markovic
@ 2017-11-21 16:50 ` Randy Dunlap
  2017-11-21 20:53 ` David Daney
  1 sibling, 0 replies; 17+ messages in thread
From: Randy Dunlap @ 2017-11-21 16:50 UTC (permalink / raw)
  To: Aleksandar Markovic, linux-mips
  Cc: Miodrag Dinic, Aleksandar Markovic, Andrew Morton, Dengcheng Zhu,
	Ding Tianhong, Douglas Leung, Frederic Weisbecker, Goran Ferenc,
	Ingo Molnar, James Cowgill, James Hogan, Jonathan Corbet,
	linux-doc, linux-kernel, Marc Zyngier, Matt Redfearn, Mimi Zohar,
	Paul Burton, Paul E. McKenney, Petar Jovanovic, Raghu Gandham,
	Ralf Baechle, Thomas Gleixner, Tom Saeger

On 11/21/2017 05:56 AM, Aleksandar Markovic wrote:
> From: Miodrag Dinic <miodrag.dinic@mips.com>
> 
> Add a new kernel parameter to override the default behavior related
> to the decision whether to set up stack as non-executable in function
> mips_elf_read_implies_exec().
> 
> The new parameter is used to control non executable stack and heap,
> regardless of PT_GNU_STACK entry. This does apply to both stack and
> heap, despite the name.
> 
> Allowed values:
> 
> nonxstack=on	Force non-exec stack & heap
> nonxstack=off	Force executable stack & heap
> 
> If this parameter is omitted, kernel behavior remains the same as it
> was before this patch is applied.
> 
> This functionality is convenient during debugging and is especially
> useful for Android development where non-exec stack is required.
> 
> Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 11 +++++++
>  arch/mips/kernel/elf.c                          | 39 +++++++++++++++++++++++++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index b74e133..99464ee 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2614,6 +2614,17 @@
>  			noexec32=off: disable non-executable mappings
>  				read implies executable mappings
>  
> +	nonxstack	[MIPS]
> +			Force setting up stack and heap as non-executable or
> +			executable regardless of PT_GNU_STACK entry. Both
> +			stack and heap are affected, despite the name. Valid
> +			arguments: on, off.
> +			nonxstack=on:	Force non-executable stack and heap
> +			nonxstack=off:	Force executable stack and heap
> +			If ommited, stack and heap will or will not be set

			   omitted,

> +			up as non-executable depending on PT_GNU_STACK
> +			entry and possibly other factors.
> +
>  	nofpu		[MIPS,SH] Disable hardware FPU at boot time.
>  
>  	nofxsr		[BUGS=X86-32] Disables x86 floating point extended
> diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c
> index 731325a..28ef7f3 100644
> --- a/arch/mips/kernel/elf.c
> +++ b/arch/mips/kernel/elf.c
> @@ -326,8 +326,47 @@ void mips_set_personality_nan(struct arch_elf_state *state)
>  	}
>  }
>  
> +static int nonxstack = EXSTACK_DEFAULT;
> +
> +/*
> + * kernel parameter: nonxstack=on|off
> + *
> + *   Force setting up stack and heap as non-executable or
> + *   executable regardless of PT_GNU_STACK entry. Both
> + *   stack and heap are affected, despite the name. Valid
> + *   arguments: on, off.
> + *
> + *     nonxstack=on:   Force non-executable stack and heap
> + *     nonxstack=off:  Force executable stack and heap
> + *
> + *   If ommited, stack and heap will or will not be set

           omitted

> + *   up as non-executable depending on PT_GNU_STACK
> + *   entry and possibly other factors.
> + */


-- 
~Randy

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
  2017-11-21 13:56 [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter Aleksandar Markovic
  2017-11-21 16:50 ` Randy Dunlap
@ 2017-11-21 20:53 ` David Daney
  2017-11-30  9:34     ` Miodrag Dinic
  1 sibling, 1 reply; 17+ messages in thread
From: David Daney @ 2017-11-21 20:53 UTC (permalink / raw)
  To: Aleksandar Markovic, linux-mips
  Cc: Miodrag Dinic, Aleksandar Markovic, Andrew Morton, Dengcheng Zhu,
	Ding Tianhong, Douglas Leung, Frederic Weisbecker, Goran Ferenc,
	Ingo Molnar, James Cowgill, James Hogan, Jonathan Corbet,
	linux-doc, linux-kernel, Marc Zyngier, Matt Redfearn, Mimi Zohar,
	Paul Burton, Paul E. McKenney, Petar Jovanovic, Raghu Gandham,
	Ralf Baechle, Thomas Gleixner, Tom Saeger

On 11/21/2017 05:56 AM, Aleksandar Markovic wrote:
> From: Miodrag Dinic <miodrag.dinic@mips.com>
> 
> Add a new kernel parameter to override the default behavior related
> to the decision whether to set up stack as non-executable in function
> mips_elf_read_implies_exec().
> 
> The new parameter is used to control non executable stack and heap,
> regardless of PT_GNU_STACK entry. This does apply to both stack and
> heap, despite the name.
> 
> Allowed values:
> 
> nonxstack=on	Force non-exec stack & heap
> nonxstack=off	Force executable stack & heap
> 
> If this parameter is omitted, kernel behavior remains the same as it
> was before this patch is applied.

Do other architectures have a similar hack?

If arm{,64} and x86 don't need this, what would make MIPS so special 
that we have to carry this around?


> 
> This functionality is convenient during debugging and is especially
> useful for Android development where non-exec stack is required.

Why not just set the PT_GNU_STACK flags correctly in the first place?

> 
> Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
> ---
>   Documentation/admin-guide/kernel-parameters.txt | 11 +++++++
>   arch/mips/kernel/elf.c                          | 39 +++++++++++++++++++++++++
>   2 files changed, 50 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index b74e133..99464ee 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2614,6 +2614,17 @@
>   			noexec32=off: disable non-executable mappings
>   				read implies executable mappings
>   
> +	nonxstack	[MIPS]
> +			Force setting up stack and heap as non-executable or
> +			executable regardless of PT_GNU_STACK entry. Both
> +			stack and heap are affected, despite the name. Valid
> +			arguments: on, off.
> +			nonxstack=on:	Force non-executable stack and heap
> +			nonxstack=off:	Force executable stack and heap
> +			If ommited, stack and heap will or will not be set
> +			up as non-executable depending on PT_GNU_STACK
> +			entry and possibly other factors.
> +
>   	nofpu		[MIPS,SH] Disable hardware FPU at boot time.
>   
>   	nofxsr		[BUGS=X86-32] Disables x86 floating point extended
> diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c
> index 731325a..28ef7f3 100644
> --- a/arch/mips/kernel/elf.c
> +++ b/arch/mips/kernel/elf.c
> @@ -326,8 +326,47 @@ void mips_set_personality_nan(struct arch_elf_state *state)
>   	}
>   }
>   
> +static int nonxstack = EXSTACK_DEFAULT;
> +
> +/*
> + * kernel parameter: nonxstack=on|off
> + *
> + *   Force setting up stack and heap as non-executable or
> + *   executable regardless of PT_GNU_STACK entry. Both
> + *   stack and heap are affected, despite the name. Valid
> + *   arguments: on, off.
> + *
> + *     nonxstack=on:   Force non-executable stack and heap
> + *     nonxstack=off:  Force executable stack and heap
> + *
> + *   If ommited, stack and heap will or will not be set
> + *   up as non-executable depending on PT_GNU_STACK
> + *   entry and possibly other factors.
> + */
> +static int __init nonxstack_setup(char *str)
> +{
> +	if (!strcmp(str, "on"))
> +		nonxstack = EXSTACK_DISABLE_X;
> +	else if (!strcmp(str, "off"))
> +		nonxstack = EXSTACK_ENABLE_X;
> +	else
> +		pr_err("Malformed nonxstack format! nonxstack=on|off\n");
> +
> +	return 1;
> +}
> +__setup("nonxstack=", nonxstack_setup);
> +
>   int mips_elf_read_implies_exec(void *elf_ex, int exstack)
>   {
> +	switch (nonxstack) {
> +	case EXSTACK_DISABLE_X:
> +		return 0;
> +	case EXSTACK_ENABLE_X:
> +		return 1;
> +	default:
> +		break;
> +	}
> +
>   	if (exstack != EXSTACK_DISABLE_X) {
>   		/* The binary doesn't request a non-executable stack */
>   		return 1;
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
  2017-11-21 20:53 ` David Daney
@ 2017-11-30  9:34     ` Miodrag Dinic
  0 siblings, 0 replies; 17+ messages in thread
From: Miodrag Dinic @ 2017-11-30  9:34 UTC (permalink / raw)
  To: David Daney, Aleksandar Markovic, linux-mips
  Cc: Aleksandar Markovic, Andrew Morton, DengCheng Zhu, Ding Tianhong,
	Douglas Leung, Frederic Weisbecker, Goran Ferenc, Ingo Molnar,
	James Cowgill, James Hogan, Jonathan Corbet, linux-doc,
	linux-kernel, Marc Zyngier, Matt Redfearn, Mimi Zohar,
	Paul Burton, Paul E. McKenney, Petar Jovanovic, Raghu Gandham,
	Ralf Baechle, Thomas Gleixner, Tom Saeger

Hi David,

Sorry for a late response, please find answers in-lined:

> > If this parameter is omitted, kernel behavior remains the same as it
> > was before this patch is applied.
> 
> Do other architectures have a similar hack?
> 
> If arm{,64} and x86 don't need this, what would make MIPS so special
> that we have to carry this around?

Yes, there are similar workarounds. Just a couple lines above
nonxstack description in the documentation there are :
	noexec		[IA-64]

	noexec		[X86]
			On X86-32 available only on PAE configured kernels.
			noexec=on: enable non-executable mappings (default)
			noexec=off: disable non-executable mappings
..

	noexec32	[X86-64]
			This affects only 32-bit executables.
			noexec32=on: enable non-executable mappings (default)
				read doesn't imply executable mappings
			noexec32=off: disable non-executable mappings
				read implies executable mappings

> > 
> > This functionality is convenient during debugging and is especially
> > useful for Android development where non-exec stack is required.
> 
> Why not just set the PT_GNU_STACK flags correctly in the first place?

We do have PT_GNU_STACK flags set correctly, this feature is required to
workaround CPU revisions which do not have RIXI support.

Kind regards,
Miodrag
________________________________________
From: David Daney [ddaney@caviumnetworks.com]
Sent: Tuesday, November 21, 2017 9:53 PM
To: Aleksandar Markovic; linux-mips@linux-mips.org
Cc: Miodrag Dinic; Aleksandar Markovic; Andrew Morton; DengCheng Zhu; Ding Tianhong; Douglas Leung; Frederic Weisbecker; Goran Ferenc; Ingo Molnar; James Cowgill; James Hogan; Jonathan Corbet; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Marc Zyngier; Matt Redfearn; Mimi Zohar; Paul Burton; Paul E. McKenney; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Thomas Gleixner; Tom Saeger
Subject: Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter

On 11/21/2017 05:56 AM, Aleksandar Markovic wrote:
> From: Miodrag Dinic <miodrag.dinic@mips.com>
>
> Add a new kernel parameter to override the default behavior related
> to the decision whether to set up stack as non-executable in function
> mips_elf_read_implies_exec().
>
> The new parameter is used to control non executable stack and heap,
> regardless of PT_GNU_STACK entry. This does apply to both stack and
> heap, despite the name.
>
> Allowed values:
>
> nonxstack=on  Force non-exec stack & heap
> nonxstack=off Force executable stack & heap
>
> If this parameter is omitted, kernel behavior remains the same as it
> was before this patch is applied.

Do other architectures have a similar hack?

If arm{,64} and x86 don't need this, what would make MIPS so special
that we have to carry this around?


>
> This functionality is convenient during debugging and is especially
> useful for Android development where non-exec stack is required.

Why not just set the PT_GNU_STACK flags correctly in the first place?

>
> Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
> ---
>   Documentation/admin-guide/kernel-parameters.txt | 11 +++++++
>   arch/mips/kernel/elf.c                          | 39 +++++++++++++++++++++++++
>   2 files changed, 50 insertions(+)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index b74e133..99464ee 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2614,6 +2614,17 @@
>                       noexec32=off: disable non-executable mappings
>                               read implies executable mappings
>
> +     nonxstack       [MIPS]
> +                     Force setting up stack and heap as non-executable or
> +                     executable regardless of PT_GNU_STACK entry. Both
> +                     stack and heap are affected, despite the name. Valid
> +                     arguments: on, off.
> +                     nonxstack=on:   Force non-executable stack and heap
> +                     nonxstack=off:  Force executable stack and heap
> +                     If ommited, stack and heap will or will not be set
> +                     up as non-executable depending on PT_GNU_STACK
> +                     entry and possibly other factors.
> +
>       nofpu           [MIPS,SH] Disable hardware FPU at boot time.
>
>       nofxsr          [BUGS=X86-32] Disables x86 floating point extended
> diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c
> index 731325a..28ef7f3 100644
> --- a/arch/mips/kernel/elf.c
> +++ b/arch/mips/kernel/elf.c
> @@ -326,8 +326,47 @@ void mips_set_personality_nan(struct arch_elf_state *state)
>       }
>   }
>
> +static int nonxstack = EXSTACK_DEFAULT;
> +
> +/*
> + * kernel parameter: nonxstack=on|off
> + *
> + *   Force setting up stack and heap as non-executable or
> + *   executable regardless of PT_GNU_STACK entry. Both
> + *   stack and heap are affected, despite the name. Valid
> + *   arguments: on, off.
> + *
> + *     nonxstack=on:   Force non-executable stack and heap
> + *     nonxstack=off:  Force executable stack and heap
> + *
> + *   If ommited, stack and heap will or will not be set
> + *   up as non-executable depending on PT_GNU_STACK
> + *   entry and possibly other factors.
> + */
> +static int __init nonxstack_setup(char *str)
> +{
> +     if (!strcmp(str, "on"))
> +             nonxstack = EXSTACK_DISABLE_X;
> +     else if (!strcmp(str, "off"))
> +             nonxstack = EXSTACK_ENABLE_X;
> +     else
> +             pr_err("Malformed nonxstack format! nonxstack=on|off\n");
> +
> +     return 1;
> +}
> +__setup("nonxstack=", nonxstack_setup);
> +
>   int mips_elf_read_implies_exec(void *elf_ex, int exstack)
>   {
> +     switch (nonxstack) {
> +     case EXSTACK_DISABLE_X:
> +             return 0;
> +     case EXSTACK_ENABLE_X:
> +             return 1;
> +     default:
> +             break;
> +     }
> +
>       if (exstack != EXSTACK_DISABLE_X) {
>               /* The binary doesn't request a non-executable stack */
>               return 1;
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
@ 2017-11-30  9:34     ` Miodrag Dinic
  0 siblings, 0 replies; 17+ messages in thread
From: Miodrag Dinic @ 2017-11-30  9:34 UTC (permalink / raw)
  To: David Daney, Aleksandar Markovic, linux-mips
  Cc: Aleksandar Markovic, Andrew Morton, DengCheng Zhu, Ding Tianhong,
	Douglas Leung, Frederic Weisbecker, Goran Ferenc, Ingo Molnar,
	James Cowgill, James Hogan, Jonathan Corbet, linux-doc,
	linux-kernel, Marc Zyngier, Matt Redfearn, Mimi Zohar,
	Paul Burton, Paul E. McKenney, Petar Jovanovic, Raghu Gandham,
	Ralf Baechle, Thomas Gleixner, Tom Saeger

Hi David,

Sorry for a late response, please find answers in-lined:

> > If this parameter is omitted, kernel behavior remains the same as it
> > was before this patch is applied.
> 
> Do other architectures have a similar hack?
> 
> If arm{,64} and x86 don't need this, what would make MIPS so special
> that we have to carry this around?

Yes, there are similar workarounds. Just a couple lines above
nonxstack description in the documentation there are :
	noexec		[IA-64]

	noexec		[X86]
			On X86-32 available only on PAE configured kernels.
			noexec=on: enable non-executable mappings (default)
			noexec=off: disable non-executable mappings
...

	noexec32	[X86-64]
			This affects only 32-bit executables.
			noexec32=on: enable non-executable mappings (default)
				read doesn't imply executable mappings
			noexec32=off: disable non-executable mappings
				read implies executable mappings

> > 
> > This functionality is convenient during debugging and is especially
> > useful for Android development where non-exec stack is required.
> 
> Why not just set the PT_GNU_STACK flags correctly in the first place?

We do have PT_GNU_STACK flags set correctly, this feature is required to
workaround CPU revisions which do not have RIXI support.

Kind regards,
Miodrag
________________________________________
From: David Daney [ddaney@caviumnetworks.com]
Sent: Tuesday, November 21, 2017 9:53 PM
To: Aleksandar Markovic; linux-mips@linux-mips.org
Cc: Miodrag Dinic; Aleksandar Markovic; Andrew Morton; DengCheng Zhu; Ding Tianhong; Douglas Leung; Frederic Weisbecker; Goran Ferenc; Ingo Molnar; James Cowgill; James Hogan; Jonathan Corbet; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Marc Zyngier; Matt Redfearn; Mimi Zohar; Paul Burton; Paul E. McKenney; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Thomas Gleixner; Tom Saeger
Subject: Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter

On 11/21/2017 05:56 AM, Aleksandar Markovic wrote:
> From: Miodrag Dinic <miodrag.dinic@mips.com>
>
> Add a new kernel parameter to override the default behavior related
> to the decision whether to set up stack as non-executable in function
> mips_elf_read_implies_exec().
>
> The new parameter is used to control non executable stack and heap,
> regardless of PT_GNU_STACK entry. This does apply to both stack and
> heap, despite the name.
>
> Allowed values:
>
> nonxstack=on  Force non-exec stack & heap
> nonxstack=off Force executable stack & heap
>
> If this parameter is omitted, kernel behavior remains the same as it
> was before this patch is applied.

Do other architectures have a similar hack?

If arm{,64} and x86 don't need this, what would make MIPS so special
that we have to carry this around?


>
> This functionality is convenient during debugging and is especially
> useful for Android development where non-exec stack is required.

Why not just set the PT_GNU_STACK flags correctly in the first place?

>
> Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
> ---
>   Documentation/admin-guide/kernel-parameters.txt | 11 +++++++
>   arch/mips/kernel/elf.c                          | 39 +++++++++++++++++++++++++
>   2 files changed, 50 insertions(+)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index b74e133..99464ee 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2614,6 +2614,17 @@
>                       noexec32=off: disable non-executable mappings
>                               read implies executable mappings
>
> +     nonxstack       [MIPS]
> +                     Force setting up stack and heap as non-executable or
> +                     executable regardless of PT_GNU_STACK entry. Both
> +                     stack and heap are affected, despite the name. Valid
> +                     arguments: on, off.
> +                     nonxstack=on:   Force non-executable stack and heap
> +                     nonxstack=off:  Force executable stack and heap
> +                     If ommited, stack and heap will or will not be set
> +                     up as non-executable depending on PT_GNU_STACK
> +                     entry and possibly other factors.
> +
>       nofpu           [MIPS,SH] Disable hardware FPU at boot time.
>
>       nofxsr          [BUGS=X86-32] Disables x86 floating point extended
> diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c
> index 731325a..28ef7f3 100644
> --- a/arch/mips/kernel/elf.c
> +++ b/arch/mips/kernel/elf.c
> @@ -326,8 +326,47 @@ void mips_set_personality_nan(struct arch_elf_state *state)
>       }
>   }
>
> +static int nonxstack = EXSTACK_DEFAULT;
> +
> +/*
> + * kernel parameter: nonxstack=on|off
> + *
> + *   Force setting up stack and heap as non-executable or
> + *   executable regardless of PT_GNU_STACK entry. Both
> + *   stack and heap are affected, despite the name. Valid
> + *   arguments: on, off.
> + *
> + *     nonxstack=on:   Force non-executable stack and heap
> + *     nonxstack=off:  Force executable stack and heap
> + *
> + *   If ommited, stack and heap will or will not be set
> + *   up as non-executable depending on PT_GNU_STACK
> + *   entry and possibly other factors.
> + */
> +static int __init nonxstack_setup(char *str)
> +{
> +     if (!strcmp(str, "on"))
> +             nonxstack = EXSTACK_DISABLE_X;
> +     else if (!strcmp(str, "off"))
> +             nonxstack = EXSTACK_ENABLE_X;
> +     else
> +             pr_err("Malformed nonxstack format! nonxstack=on|off\n");
> +
> +     return 1;
> +}
> +__setup("nonxstack=", nonxstack_setup);
> +
>   int mips_elf_read_implies_exec(void *elf_ex, int exstack)
>   {
> +     switch (nonxstack) {
> +     case EXSTACK_DISABLE_X:
> +             return 0;
> +     case EXSTACK_ENABLE_X:
> +             return 1;
> +     default:
> +             break;
> +     }
> +
>       if (exstack != EXSTACK_DISABLE_X) {
>               /* The binary doesn't request a non-executable stack */
>               return 1;
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
  2017-11-30  9:34     ` Miodrag Dinic
  (?)
@ 2017-11-30 10:09     ` James Hogan
  2017-11-30 13:06         ` Miodrag Dinic
  -1 siblings, 1 reply; 17+ messages in thread
From: James Hogan @ 2017-11-30 10:09 UTC (permalink / raw)
  To: Miodrag Dinic
  Cc: David Daney, Aleksandar Markovic, linux-mips,
	Aleksandar Markovic, Andrew Morton, DengCheng Zhu, Ding Tianhong,
	Douglas Leung, Frederic Weisbecker, Goran Ferenc, Ingo Molnar,
	James Cowgill, Jonathan Corbet, linux-doc, linux-kernel,
	Marc Zyngier, Matt Redfearn, Mimi Zohar, Paul Burton,
	Paul E. McKenney, Petar Jovanovic, Raghu Gandham, Ralf Baechle,
	Thomas Gleixner, Tom Saeger

[-- Attachment #1: Type: text/plain, Size: 6896 bytes --]

On Thu, Nov 30, 2017 at 09:34:15AM +0000, Miodrag Dinic wrote:
> Hi David,
> 
> Sorry for a late response, please find answers in-lined:
> 
> > > If this parameter is omitted, kernel behavior remains the same as it
> > > was before this patch is applied.
> > 
> > Do other architectures have a similar hack?
> > 
> > If arm{,64} and x86 don't need this, what would make MIPS so special
> > that we have to carry this around?
> 
> Yes, there are similar workarounds. Just a couple lines above
> nonxstack description in the documentation there are :
> 	noexec		[IA-64]
> 
> 	noexec		[X86]
> 			On X86-32 available only on PAE configured kernels.
> 			noexec=on: enable non-executable mappings (default)
> 			noexec=off: disable non-executable mappings
> ...
> 
> 	noexec32	[X86-64]
> 			This affects only 32-bit executables.
> 			noexec32=on: enable non-executable mappings (default)
> 				read doesn't imply executable mappings
> 			noexec32=off: disable non-executable mappings
> 				read implies executable mappings
> 
> > > 
> > > This functionality is convenient during debugging and is especially
> > > useful for Android development where non-exec stack is required.
> > 
> > Why not just set the PT_GNU_STACK flags correctly in the first place?
> 
> We do have PT_GNU_STACK flags set correctly, this feature is required to
> workaround CPU revisions which do not have RIXI support.

RIXI support can be discovered programatically from CP0_Config3.RXI
(cpu_has_rixi in asm/cpu-features.h), so I don't follow why CPUs without
RIXI would require a kernel parameter.

Cheers
James

> 
> Kind regards,
> Miodrag
> ________________________________________
> From: David Daney [ddaney@caviumnetworks.com]
> Sent: Tuesday, November 21, 2017 9:53 PM
> To: Aleksandar Markovic; linux-mips@linux-mips.org
> Cc: Miodrag Dinic; Aleksandar Markovic; Andrew Morton; DengCheng Zhu; Ding Tianhong; Douglas Leung; Frederic Weisbecker; Goran Ferenc; Ingo Molnar; James Cowgill; James Hogan; Jonathan Corbet; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Marc Zyngier; Matt Redfearn; Mimi Zohar; Paul Burton; Paul E. McKenney; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Thomas Gleixner; Tom Saeger
> Subject: Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
> 
> On 11/21/2017 05:56 AM, Aleksandar Markovic wrote:
> > From: Miodrag Dinic <miodrag.dinic@mips.com>
> >
> > Add a new kernel parameter to override the default behavior related
> > to the decision whether to set up stack as non-executable in function
> > mips_elf_read_implies_exec().
> >
> > The new parameter is used to control non executable stack and heap,
> > regardless of PT_GNU_STACK entry. This does apply to both stack and
> > heap, despite the name.
> >
> > Allowed values:
> >
> > nonxstack=on  Force non-exec stack & heap
> > nonxstack=off Force executable stack & heap
> >
> > If this parameter is omitted, kernel behavior remains the same as it
> > was before this patch is applied.
> 
> Do other architectures have a similar hack?
> 
> If arm{,64} and x86 don't need this, what would make MIPS so special
> that we have to carry this around?
> 
> 
> >
> > This functionality is convenient during debugging and is especially
> > useful for Android development where non-exec stack is required.
> 
> Why not just set the PT_GNU_STACK flags correctly in the first place?
> 
> >
> > Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> > Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
> > ---
> >   Documentation/admin-guide/kernel-parameters.txt | 11 +++++++
> >   arch/mips/kernel/elf.c                          | 39 +++++++++++++++++++++++++
> >   2 files changed, 50 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index b74e133..99464ee 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -2614,6 +2614,17 @@
> >                       noexec32=off: disable non-executable mappings
> >                               read implies executable mappings
> >
> > +     nonxstack       [MIPS]
> > +                     Force setting up stack and heap as non-executable or
> > +                     executable regardless of PT_GNU_STACK entry. Both
> > +                     stack and heap are affected, despite the name. Valid
> > +                     arguments: on, off.
> > +                     nonxstack=on:   Force non-executable stack and heap
> > +                     nonxstack=off:  Force executable stack and heap
> > +                     If ommited, stack and heap will or will not be set
> > +                     up as non-executable depending on PT_GNU_STACK
> > +                     entry and possibly other factors.
> > +
> >       nofpu           [MIPS,SH] Disable hardware FPU at boot time.
> >
> >       nofxsr          [BUGS=X86-32] Disables x86 floating point extended
> > diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c
> > index 731325a..28ef7f3 100644
> > --- a/arch/mips/kernel/elf.c
> > +++ b/arch/mips/kernel/elf.c
> > @@ -326,8 +326,47 @@ void mips_set_personality_nan(struct arch_elf_state *state)
> >       }
> >   }
> >
> > +static int nonxstack = EXSTACK_DEFAULT;
> > +
> > +/*
> > + * kernel parameter: nonxstack=on|off
> > + *
> > + *   Force setting up stack and heap as non-executable or
> > + *   executable regardless of PT_GNU_STACK entry. Both
> > + *   stack and heap are affected, despite the name. Valid
> > + *   arguments: on, off.
> > + *
> > + *     nonxstack=on:   Force non-executable stack and heap
> > + *     nonxstack=off:  Force executable stack and heap
> > + *
> > + *   If ommited, stack and heap will or will not be set
> > + *   up as non-executable depending on PT_GNU_STACK
> > + *   entry and possibly other factors.
> > + */
> > +static int __init nonxstack_setup(char *str)
> > +{
> > +     if (!strcmp(str, "on"))
> > +             nonxstack = EXSTACK_DISABLE_X;
> > +     else if (!strcmp(str, "off"))
> > +             nonxstack = EXSTACK_ENABLE_X;
> > +     else
> > +             pr_err("Malformed nonxstack format! nonxstack=on|off\n");
> > +
> > +     return 1;
> > +}
> > +__setup("nonxstack=", nonxstack_setup);
> > +
> >   int mips_elf_read_implies_exec(void *elf_ex, int exstack)
> >   {
> > +     switch (nonxstack) {
> > +     case EXSTACK_DISABLE_X:
> > +             return 0;
> > +     case EXSTACK_ENABLE_X:
> > +             return 1;
> > +     default:
> > +             break;
> > +     }
> > +
> >       if (exstack != EXSTACK_DISABLE_X) {
> >               /* The binary doesn't request a non-executable stack */
> >               return 1;
> >
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
  2017-11-30 10:09     ` James Hogan
@ 2017-11-30 13:06         ` Miodrag Dinic
  0 siblings, 0 replies; 17+ messages in thread
From: Miodrag Dinic @ 2017-11-30 13:06 UTC (permalink / raw)
  To: James Hogan
  Cc: David Daney, Aleksandar Markovic, linux-mips,
	Aleksandar Markovic, Andrew Morton, DengCheng Zhu, Ding Tianhong,
	Douglas Leung, Frederic Weisbecker, Goran Ferenc, Ingo Molnar,
	James Cowgill, Jonathan Corbet, linux-doc, linux-kernel,
	Marc Zyngier, Matt Redfearn, Mimi Zohar, Paul Burton,
	Paul E. McKenney, Petar Jovanovic, Raghu Gandham, Ralf Baechle,
	Thomas Gleixner, Tom Saeger

Hi James,

> > We do have PT_GNU_STACK flags set correctly, this feature is required to
> > workaround CPU revisions which do not have RIXI support.
> 
> RIXI support can be discovered programatically from CP0_Config3.RXI
> (cpu_has_rixi in asm/cpu-features.h), so I don't follow why CPUs without
> RIXI would require a kernel parameter.

The following patch introduced change in behavior with regards to
stack & heap execute-ability :
commit 1a770b85c1f1c1ee37afd7cef5237ffc4c970f04
Author: Paul Burton <paul.burton@imgtec.com>
Date:   Fri Jul 8 11:06:20 2016 +0100

    MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present
    
    The stack and heap have both been executable by default on MIPS until
    now. This patch changes the default to be non-executable, but only for
    ELF binaries with a non-executable PT_GNU_STACK header present. This
    does apply to both the heap & the stack, despite the name PT_GNU_STACK,
    and this matches the behaviour of other architectures like ARM & x86.
    
    Current MIPS toolchains do not produce the PT_GNU_STACK header, which
    means that we can rely upon this patch not changing the behaviour of
    existing binaries. The new default will only take effect for newly
    compiled binaries once toolchains are updated to support PT_GNU_STACK,
    and since those binaries are newly compiled they can be compiled
    expecting the change in default behaviour. Again this matches the way in
    which the ARM & x86 architectures handled their implementations of
    non-executable memory.
    
    Signed-off-by: Paul Burton <paul.burton@imgtec.com>
    Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com>
    Cc: Maciej Rozycki <maciej.rozycki@imgtec.com>
    Cc: Faraz Shahbazker <faraz.shahbazker@imgtec.com>
    Cc: Raghu Gandham <raghu.gandham@imgtec.com>
    Cc: Matthew Fortune <matthew.fortune@imgtec.com>
    Cc: linux-mips@linux-mips.org
    Patchwork: https://patchwork.linux-mips.org/patch/13765/
    Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

...

When kernel is detecting the type of mapping it should apply :

fs/binfmt_elf.c:
..
	if (elf_read_implies_exec(loc->elf_ex, executable_stack))
		current->personality |= READ_IMPLIES_EXEC;
..

this effectively calls mips_elf_read_implies_exec() which performs a check:
..
	if (!cpu_has_rixi) {
		/* The CPU doesn't support non-executable memory */
		return 1;
	}

	return 0;
}

This will in turn make stack & heap executable on processors without RIXI, which are practically all processors with MIPS ISA R < 6.

We would like to have an option to override this and force non-executable mappings for such systems.

Kind regards,
Miodrag
________________________________________
From: James Hogan
Sent: Thursday, November 30, 2017 11:09 AM
To: Miodrag Dinic
Cc: David Daney; Aleksandar Markovic; linux-mips@linux-mips.org; Aleksandar Markovic; Andrew Morton; DengCheng Zhu; Ding Tianhong; Douglas Leung; Frederic Weisbecker; Goran Ferenc; Ingo Molnar; James Cowgill; Jonathan Corbet; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Marc Zyngier; Matt Redfearn; Mimi Zohar; Paul Burton; Paul E. McKenney; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Thomas Gleixner; Tom Saeger
Subject: Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter

On Thu, Nov 30, 2017 at 09:34:15AM +0000, Miodrag Dinic wrote:
> Hi David,
>
> Sorry for a late response, please find answers in-lined:
>
> > > If this parameter is omitted, kernel behavior remains the same as it
> > > was before this patch is applied.
> >
> > Do other architectures have a similar hack?
> >
> > If arm{,64} and x86 don't need this, what would make MIPS so special
> > that we have to carry this around?
>
> Yes, there are similar workarounds. Just a couple lines above
> nonxstack description in the documentation there are :
>       noexec          [IA-64]
>
>       noexec          [X86]
>                       On X86-32 available only on PAE configured kernels.
>                       noexec=on: enable non-executable mappings (default)
>                       noexec=off: disable non-executable mappings
> ...
>
>       noexec32        [X86-64]
>                       This affects only 32-bit executables.
>                       noexec32=on: enable non-executable mappings (default)
>                               read doesn't imply executable mappings
>                       noexec32=off: disable non-executable mappings
>                               read implies executable mappings
>
> > >
> > > This functionality is convenient during debugging and is especially
> > > useful for Android development where non-exec stack is required.
> >
> > Why not just set the PT_GNU_STACK flags correctly in the first place?
>
> We do have PT_GNU_STACK flags set correctly, this feature is required to
> workaround CPU revisions which do not have RIXI support.

RIXI support can be discovered programatically from CP0_Config3.RXI
(cpu_has_rixi in asm/cpu-features.h), so I don't follow why CPUs without
RIXI would require a kernel parameter.

Cheers
James

>
> Kind regards,
> Miodrag
> ________________________________________
> From: David Daney [ddaney@caviumnetworks.com]
> Sent: Tuesday, November 21, 2017 9:53 PM
> To: Aleksandar Markovic; linux-mips@linux-mips.org
> Cc: Miodrag Dinic; Aleksandar Markovic; Andrew Morton; DengCheng Zhu; Ding Tianhong; Douglas Leung; Frederic Weisbecker; Goran Ferenc; Ingo Molnar; James Cowgill; James Hogan; Jonathan Corbet; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Marc Zyngier; Matt Redfearn; Mimi Zohar; Paul Burton; Paul E. McKenney; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Thomas Gleixner; Tom Saeger
> Subject: Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
>
> On 11/21/2017 05:56 AM, Aleksandar Markovic wrote:
> > From: Miodrag Dinic <miodrag.dinic@mips.com>
> >
> > Add a new kernel parameter to override the default behavior related
> > to the decision whether to set up stack as non-executable in function
> > mips_elf_read_implies_exec().
> >
> > The new parameter is used to control non executable stack and heap,
> > regardless of PT_GNU_STACK entry. This does apply to both stack and
> > heap, despite the name.
> >
> > Allowed values:
> >
> > nonxstack=on  Force non-exec stack & heap
> > nonxstack=off Force executable stack & heap
> >
> > If this parameter is omitted, kernel behavior remains the same as it
> > was before this patch is applied.
>
> Do other architectures have a similar hack?
>
> If arm{,64} and x86 don't need this, what would make MIPS so special
> that we have to carry this around?
>
>
> >
> > This functionality is convenient during debugging and is especially
> > useful for Android development where non-exec stack is required.
>
> Why not just set the PT_GNU_STACK flags correctly in the first place?
>
> >
> > Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> > Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
> > ---
> >   Documentation/admin-guide/kernel-parameters.txt | 11 +++++++
> >   arch/mips/kernel/elf.c                          | 39 +++++++++++++++++++++++++
> >   2 files changed, 50 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index b74e133..99464ee 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -2614,6 +2614,17 @@
> >                       noexec32=off: disable non-executable mappings
> >                               read implies executable mappings
> >
> > +     nonxstack       [MIPS]
> > +                     Force setting up stack and heap as non-executable or
> > +                     executable regardless of PT_GNU_STACK entry. Both
> > +                     stack and heap are affected, despite the name. Valid
> > +                     arguments: on, off.
> > +                     nonxstack=on:   Force non-executable stack and heap
> > +                     nonxstack=off:  Force executable stack and heap
> > +                     If ommited, stack and heap will or will not be set
> > +                     up as non-executable depending on PT_GNU_STACK
> > +                     entry and possibly other factors.
> > +
> >       nofpu           [MIPS,SH] Disable hardware FPU at boot time.
> >
> >       nofxsr          [BUGS=X86-32] Disables x86 floating point extended
> > diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c
> > index 731325a..28ef7f3 100644
> > --- a/arch/mips/kernel/elf.c
> > +++ b/arch/mips/kernel/elf.c
> > @@ -326,8 +326,47 @@ void mips_set_personality_nan(struct arch_elf_state *state)
> >       }
> >   }
> >
> > +static int nonxstack = EXSTACK_DEFAULT;
> > +
> > +/*
> > + * kernel parameter: nonxstack=on|off
> > + *
> > + *   Force setting up stack and heap as non-executable or
> > + *   executable regardless of PT_GNU_STACK entry. Both
> > + *   stack and heap are affected, despite the name. Valid
> > + *   arguments: on, off.
> > + *
> > + *     nonxstack=on:   Force non-executable stack and heap
> > + *     nonxstack=off:  Force executable stack and heap
> > + *
> > + *   If ommited, stack and heap will or will not be set
> > + *   up as non-executable depending on PT_GNU_STACK
> > + *   entry and possibly other factors.
> > + */
> > +static int __init nonxstack_setup(char *str)
> > +{
> > +     if (!strcmp(str, "on"))
> > +             nonxstack = EXSTACK_DISABLE_X;
> > +     else if (!strcmp(str, "off"))
> > +             nonxstack = EXSTACK_ENABLE_X;
> > +     else
> > +             pr_err("Malformed nonxstack format! nonxstack=on|off\n");
> > +
> > +     return 1;
> > +}
> > +__setup("nonxstack=", nonxstack_setup);
> > +
> >   int mips_elf_read_implies_exec(void *elf_ex, int exstack)
> >   {
> > +     switch (nonxstack) {
> > +     case EXSTACK_DISABLE_X:
> > +             return 0;
> > +     case EXSTACK_ENABLE_X:
> > +             return 1;
> > +     default:
> > +             break;
> > +     }
> > +
> >       if (exstack != EXSTACK_DISABLE_X) {
> >               /* The binary doesn't request a non-executable stack */
> >               return 1;
> >
>
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
@ 2017-11-30 13:06         ` Miodrag Dinic
  0 siblings, 0 replies; 17+ messages in thread
From: Miodrag Dinic @ 2017-11-30 13:06 UTC (permalink / raw)
  To: James Hogan
  Cc: David Daney, Aleksandar Markovic, linux-mips,
	Aleksandar Markovic, Andrew Morton, DengCheng Zhu, Ding Tianhong,
	Douglas Leung, Frederic Weisbecker, Goran Ferenc, Ingo Molnar,
	James Cowgill, Jonathan Corbet, linux-doc, linux-kernel,
	Marc Zyngier, Matt Redfearn, Mimi Zohar, Paul Burton,
	Paul E. McKenney, Petar Jovanovic, Raghu Gandham, Ralf Baechle,
	Thomas Gleixner, Tom Saeger

Hi James,

> > We do have PT_GNU_STACK flags set correctly, this feature is required to
> > workaround CPU revisions which do not have RIXI support.
> 
> RIXI support can be discovered programatically from CP0_Config3.RXI
> (cpu_has_rixi in asm/cpu-features.h), so I don't follow why CPUs without
> RIXI would require a kernel parameter.

The following patch introduced change in behavior with regards to
stack & heap execute-ability :
commit 1a770b85c1f1c1ee37afd7cef5237ffc4c970f04
Author: Paul Burton <paul.burton@imgtec.com>
Date:   Fri Jul 8 11:06:20 2016 +0100

    MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present
    
    The stack and heap have both been executable by default on MIPS until
    now. This patch changes the default to be non-executable, but only for
    ELF binaries with a non-executable PT_GNU_STACK header present. This
    does apply to both the heap & the stack, despite the name PT_GNU_STACK,
    and this matches the behaviour of other architectures like ARM & x86.
    
    Current MIPS toolchains do not produce the PT_GNU_STACK header, which
    means that we can rely upon this patch not changing the behaviour of
    existing binaries. The new default will only take effect for newly
    compiled binaries once toolchains are updated to support PT_GNU_STACK,
    and since those binaries are newly compiled they can be compiled
    expecting the change in default behaviour. Again this matches the way in
    which the ARM & x86 architectures handled their implementations of
    non-executable memory.
    
    Signed-off-by: Paul Burton <paul.burton@imgtec.com>
    Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com>
    Cc: Maciej Rozycki <maciej.rozycki@imgtec.com>
    Cc: Faraz Shahbazker <faraz.shahbazker@imgtec.com>
    Cc: Raghu Gandham <raghu.gandham@imgtec.com>
    Cc: Matthew Fortune <matthew.fortune@imgtec.com>
    Cc: linux-mips@linux-mips.org
    Patchwork: https://patchwork.linux-mips.org/patch/13765/
    Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

....

When kernel is detecting the type of mapping it should apply :

fs/binfmt_elf.c:
...
	if (elf_read_implies_exec(loc->elf_ex, executable_stack))
		current->personality |= READ_IMPLIES_EXEC;
...

this effectively calls mips_elf_read_implies_exec() which performs a check:
...
	if (!cpu_has_rixi) {
		/* The CPU doesn't support non-executable memory */
		return 1;
	}

	return 0;
}

This will in turn make stack & heap executable on processors without RIXI, which are practically all processors with MIPS ISA R < 6.

We would like to have an option to override this and force non-executable mappings for such systems.

Kind regards,
Miodrag
________________________________________
From: James Hogan
Sent: Thursday, November 30, 2017 11:09 AM
To: Miodrag Dinic
Cc: David Daney; Aleksandar Markovic; linux-mips@linux-mips.org; Aleksandar Markovic; Andrew Morton; DengCheng Zhu; Ding Tianhong; Douglas Leung; Frederic Weisbecker; Goran Ferenc; Ingo Molnar; James Cowgill; Jonathan Corbet; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Marc Zyngier; Matt Redfearn; Mimi Zohar; Paul Burton; Paul E. McKenney; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Thomas Gleixner; Tom Saeger
Subject: Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter

On Thu, Nov 30, 2017 at 09:34:15AM +0000, Miodrag Dinic wrote:
> Hi David,
>
> Sorry for a late response, please find answers in-lined:
>
> > > If this parameter is omitted, kernel behavior remains the same as it
> > > was before this patch is applied.
> >
> > Do other architectures have a similar hack?
> >
> > If arm{,64} and x86 don't need this, what would make MIPS so special
> > that we have to carry this around?
>
> Yes, there are similar workarounds. Just a couple lines above
> nonxstack description in the documentation there are :
>       noexec          [IA-64]
>
>       noexec          [X86]
>                       On X86-32 available only on PAE configured kernels.
>                       noexec=on: enable non-executable mappings (default)
>                       noexec=off: disable non-executable mappings
> ...
>
>       noexec32        [X86-64]
>                       This affects only 32-bit executables.
>                       noexec32=on: enable non-executable mappings (default)
>                               read doesn't imply executable mappings
>                       noexec32=off: disable non-executable mappings
>                               read implies executable mappings
>
> > >
> > > This functionality is convenient during debugging and is especially
> > > useful for Android development where non-exec stack is required.
> >
> > Why not just set the PT_GNU_STACK flags correctly in the first place?
>
> We do have PT_GNU_STACK flags set correctly, this feature is required to
> workaround CPU revisions which do not have RIXI support.

RIXI support can be discovered programatically from CP0_Config3.RXI
(cpu_has_rixi in asm/cpu-features.h), so I don't follow why CPUs without
RIXI would require a kernel parameter.

Cheers
James

>
> Kind regards,
> Miodrag
> ________________________________________
> From: David Daney [ddaney@caviumnetworks.com]
> Sent: Tuesday, November 21, 2017 9:53 PM
> To: Aleksandar Markovic; linux-mips@linux-mips.org
> Cc: Miodrag Dinic; Aleksandar Markovic; Andrew Morton; DengCheng Zhu; Ding Tianhong; Douglas Leung; Frederic Weisbecker; Goran Ferenc; Ingo Molnar; James Cowgill; James Hogan; Jonathan Corbet; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Marc Zyngier; Matt Redfearn; Mimi Zohar; Paul Burton; Paul E. McKenney; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Thomas Gleixner; Tom Saeger
> Subject: Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
>
> On 11/21/2017 05:56 AM, Aleksandar Markovic wrote:
> > From: Miodrag Dinic <miodrag.dinic@mips.com>
> >
> > Add a new kernel parameter to override the default behavior related
> > to the decision whether to set up stack as non-executable in function
> > mips_elf_read_implies_exec().
> >
> > The new parameter is used to control non executable stack and heap,
> > regardless of PT_GNU_STACK entry. This does apply to both stack and
> > heap, despite the name.
> >
> > Allowed values:
> >
> > nonxstack=on  Force non-exec stack & heap
> > nonxstack=off Force executable stack & heap
> >
> > If this parameter is omitted, kernel behavior remains the same as it
> > was before this patch is applied.
>
> Do other architectures have a similar hack?
>
> If arm{,64} and x86 don't need this, what would make MIPS so special
> that we have to carry this around?
>
>
> >
> > This functionality is convenient during debugging and is especially
> > useful for Android development where non-exec stack is required.
>
> Why not just set the PT_GNU_STACK flags correctly in the first place?
>
> >
> > Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> > Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
> > ---
> >   Documentation/admin-guide/kernel-parameters.txt | 11 +++++++
> >   arch/mips/kernel/elf.c                          | 39 +++++++++++++++++++++++++
> >   2 files changed, 50 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index b74e133..99464ee 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -2614,6 +2614,17 @@
> >                       noexec32=off: disable non-executable mappings
> >                               read implies executable mappings
> >
> > +     nonxstack       [MIPS]
> > +                     Force setting up stack and heap as non-executable or
> > +                     executable regardless of PT_GNU_STACK entry. Both
> > +                     stack and heap are affected, despite the name. Valid
> > +                     arguments: on, off.
> > +                     nonxstack=on:   Force non-executable stack and heap
> > +                     nonxstack=off:  Force executable stack and heap
> > +                     If ommited, stack and heap will or will not be set
> > +                     up as non-executable depending on PT_GNU_STACK
> > +                     entry and possibly other factors.
> > +
> >       nofpu           [MIPS,SH] Disable hardware FPU at boot time.
> >
> >       nofxsr          [BUGS=X86-32] Disables x86 floating point extended
> > diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c
> > index 731325a..28ef7f3 100644
> > --- a/arch/mips/kernel/elf.c
> > +++ b/arch/mips/kernel/elf.c
> > @@ -326,8 +326,47 @@ void mips_set_personality_nan(struct arch_elf_state *state)
> >       }
> >   }
> >
> > +static int nonxstack = EXSTACK_DEFAULT;
> > +
> > +/*
> > + * kernel parameter: nonxstack=on|off
> > + *
> > + *   Force setting up stack and heap as non-executable or
> > + *   executable regardless of PT_GNU_STACK entry. Both
> > + *   stack and heap are affected, despite the name. Valid
> > + *   arguments: on, off.
> > + *
> > + *     nonxstack=on:   Force non-executable stack and heap
> > + *     nonxstack=off:  Force executable stack and heap
> > + *
> > + *   If ommited, stack and heap will or will not be set
> > + *   up as non-executable depending on PT_GNU_STACK
> > + *   entry and possibly other factors.
> > + */
> > +static int __init nonxstack_setup(char *str)
> > +{
> > +     if (!strcmp(str, "on"))
> > +             nonxstack = EXSTACK_DISABLE_X;
> > +     else if (!strcmp(str, "off"))
> > +             nonxstack = EXSTACK_ENABLE_X;
> > +     else
> > +             pr_err("Malformed nonxstack format! nonxstack=on|off\n");
> > +
> > +     return 1;
> > +}
> > +__setup("nonxstack=", nonxstack_setup);
> > +
> >   int mips_elf_read_implies_exec(void *elf_ex, int exstack)
> >   {
> > +     switch (nonxstack) {
> > +     case EXSTACK_DISABLE_X:
> > +             return 0;
> > +     case EXSTACK_ENABLE_X:
> > +             return 1;
> > +     default:
> > +             break;
> > +     }
> > +
> >       if (exstack != EXSTACK_DISABLE_X) {
> >               /* The binary doesn't request a non-executable stack */
> >               return 1;
> >
>
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
  2017-11-30 13:06         ` Miodrag Dinic
@ 2017-12-01 11:35           ` Miodrag Dinic
  -1 siblings, 0 replies; 17+ messages in thread
From: Miodrag Dinic @ 2017-12-01 11:35 UTC (permalink / raw)
  To: James Hogan
  Cc: David Daney, Aleksandar Markovic, linux-mips,
	Aleksandar Markovic, Andrew Morton, DengCheng Zhu, Ding Tianhong,
	Douglas Leung, Frederic Weisbecker, Goran Ferenc, Ingo Molnar,
	James Cowgill, Jonathan Corbet, linux-doc, linux-kernel,
	Marc Zyngier, Matt Redfearn, Mimi Zohar, Paul Burton,
	Paul E. McKenney, Petar Jovanovic, Raghu Gandham, Ralf Baechle,
	Thomas Gleixner, Tom Saeger

Hi everyone,

just a note, even though we started discussion on V2 version of this patch, an up to date V3 version is available at :
https://patchwork.kernel.org/patch/10068871/

It contains spelling check fixes.

Thank you.

Kind regards,
Miodrag

________________________________________
From: Miodrag Dinic
Sent: Thursday, November 30, 2017 2:06 PM
To: James Hogan
Cc: David Daney; Aleksandar Markovic; linux-mips@linux-mips.org; Aleksandar Markovic; Andrew Morton; DengCheng Zhu; Ding Tianhong; Douglas Leung; Frederic Weisbecker; Goran Ferenc; Ingo Molnar; James Cowgill; Jonathan Corbet; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Marc Zyngier; Matt Redfearn; Mimi Zohar; Paul Burton; Paul E. McKenney; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Thomas Gleixner; Tom Saeger
Subject: RE: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter

Hi James,

> > We do have PT_GNU_STACK flags set correctly, this feature is required to
> > workaround CPU revisions which do not have RIXI support.
>
> RIXI support can be discovered programatically from CP0_Config3.RXI
> (cpu_has_rixi in asm/cpu-features.h), so I don't follow why CPUs without
> RIXI would require a kernel parameter.

The following patch introduced change in behavior with regards to
stack & heap execute-ability :
commit 1a770b85c1f1c1ee37afd7cef5237ffc4c970f04
Author: Paul Burton <paul.burton@imgtec.com>
Date:   Fri Jul 8 11:06:20 2016 +0100

    MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present

    The stack and heap have both been executable by default on MIPS until
    now. This patch changes the default to be non-executable, but only for
    ELF binaries with a non-executable PT_GNU_STACK header present. This
    does apply to both the heap & the stack, despite the name PT_GNU_STACK,
    and this matches the behaviour of other architectures like ARM & x86.

    Current MIPS toolchains do not produce the PT_GNU_STACK header, which
    means that we can rely upon this patch not changing the behaviour of
    existing binaries. The new default will only take effect for newly
    compiled binaries once toolchains are updated to support PT_GNU_STACK,
    and since those binaries are newly compiled they can be compiled
    expecting the change in default behaviour. Again this matches the way in
    which the ARM & x86 architectures handled their implementations of
    non-executable memory.

    Signed-off-by: Paul Burton <paul.burton@imgtec.com>
    Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com>
    Cc: Maciej Rozycki <maciej.rozycki@imgtec.com>
    Cc: Faraz Shahbazker <faraz.shahbazker@imgtec.com>
    Cc: Raghu Gandham <raghu.gandham@imgtec.com>
    Cc: Matthew Fortune <matthew.fortune@imgtec.com>
    Cc: linux-mips@linux-mips.org
    Patchwork: https://patchwork.linux-mips.org/patch/13765/
    Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

...

When kernel is detecting the type of mapping it should apply :

fs/binfmt_elf.c:
..
        if (elf_read_implies_exec(loc->elf_ex, executable_stack))
                current->personality |= READ_IMPLIES_EXEC;
..

this effectively calls mips_elf_read_implies_exec() which performs a check:
..
        if (!cpu_has_rixi) {
                /* The CPU doesn't support non-executable memory */
                return 1;
        }

        return 0;
}

This will in turn make stack & heap executable on processors without RIXI, which are practically all processors with MIPS ISA R < 6.

We would like to have an option to override this and force non-executable mappings for such systems.

Kind regards,
Miodrag
________________________________________
From: James Hogan
Sent: Thursday, November 30, 2017 11:09 AM
To: Miodrag Dinic
Cc: David Daney; Aleksandar Markovic; linux-mips@linux-mips.org; Aleksandar Markovic; Andrew Morton; DengCheng Zhu; Ding Tianhong; Douglas Leung; Frederic Weisbecker; Goran Ferenc; Ingo Molnar; James Cowgill; Jonathan Corbet; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Marc Zyngier; Matt Redfearn; Mimi Zohar; Paul Burton; Paul E. McKenney; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Thomas Gleixner; Tom Saeger
Subject: Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter

On Thu, Nov 30, 2017 at 09:34:15AM +0000, Miodrag Dinic wrote:
> Hi David,
>
> Sorry for a late response, please find answers in-lined:
>
> > > If this parameter is omitted, kernel behavior remains the same as it
> > > was before this patch is applied.
> >
> > Do other architectures have a similar hack?
> >
> > If arm{,64} and x86 don't need this, what would make MIPS so special
> > that we have to carry this around?
>
> Yes, there are similar workarounds. Just a couple lines above
> nonxstack description in the documentation there are :
>       noexec          [IA-64]
>
>       noexec          [X86]
>                       On X86-32 available only on PAE configured kernels.
>                       noexec=on: enable non-executable mappings (default)
>                       noexec=off: disable non-executable mappings
> ...
>
>       noexec32        [X86-64]
>                       This affects only 32-bit executables.
>                       noexec32=on: enable non-executable mappings (default)
>                               read doesn't imply executable mappings
>                       noexec32=off: disable non-executable mappings
>                               read implies executable mappings
>
> > >
> > > This functionality is convenient during debugging and is especially
> > > useful for Android development where non-exec stack is required.
> >
> > Why not just set the PT_GNU_STACK flags correctly in the first place?
>
> We do have PT_GNU_STACK flags set correctly, this feature is required to
> workaround CPU revisions which do not have RIXI support.

RIXI support can be discovered programatically from CP0_Config3.RXI
(cpu_has_rixi in asm/cpu-features.h), so I don't follow why CPUs without
RIXI would require a kernel parameter.

Cheers
James

>
> Kind regards,
> Miodrag
> ________________________________________
> From: David Daney [ddaney@caviumnetworks.com]
> Sent: Tuesday, November 21, 2017 9:53 PM
> To: Aleksandar Markovic; linux-mips@linux-mips.org
> Cc: Miodrag Dinic; Aleksandar Markovic; Andrew Morton; DengCheng Zhu; Ding Tianhong; Douglas Leung; Frederic Weisbecker; Goran Ferenc; Ingo Molnar; James Cowgill; James Hogan; Jonathan Corbet; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Marc Zyngier; Matt Redfearn; Mimi Zohar; Paul Burton; Paul E. McKenney; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Thomas Gleixner; Tom Saeger
> Subject: Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
>
> On 11/21/2017 05:56 AM, Aleksandar Markovic wrote:
> > From: Miodrag Dinic <miodrag.dinic@mips.com>
> >
> > Add a new kernel parameter to override the default behavior related
> > to the decision whether to set up stack as non-executable in function
> > mips_elf_read_implies_exec().
> >
> > The new parameter is used to control non executable stack and heap,
> > regardless of PT_GNU_STACK entry. This does apply to both stack and
> > heap, despite the name.
> >
> > Allowed values:
> >
> > nonxstack=on  Force non-exec stack & heap
> > nonxstack=off Force executable stack & heap
> >
> > If this parameter is omitted, kernel behavior remains the same as it
> > was before this patch is applied.
>
> Do other architectures have a similar hack?
>
> If arm{,64} and x86 don't need this, what would make MIPS so special
> that we have to carry this around?
>
>
> >
> > This functionality is convenient during debugging and is especially
> > useful for Android development where non-exec stack is required.
>
> Why not just set the PT_GNU_STACK flags correctly in the first place?
>
> >
> > Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> > Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
> > ---
> >   Documentation/admin-guide/kernel-parameters.txt | 11 +++++++
> >   arch/mips/kernel/elf.c                          | 39 +++++++++++++++++++++++++
> >   2 files changed, 50 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index b74e133..99464ee 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -2614,6 +2614,17 @@
> >                       noexec32=off: disable non-executable mappings
> >                               read implies executable mappings
> >
> > +     nonxstack       [MIPS]
> > +                     Force setting up stack and heap as non-executable or
> > +                     executable regardless of PT_GNU_STACK entry. Both
> > +                     stack and heap are affected, despite the name. Valid
> > +                     arguments: on, off.
> > +                     nonxstack=on:   Force non-executable stack and heap
> > +                     nonxstack=off:  Force executable stack and heap
> > +                     If ommited, stack and heap will or will not be set
> > +                     up as non-executable depending on PT_GNU_STACK
> > +                     entry and possibly other factors.
> > +
> >       nofpu           [MIPS,SH] Disable hardware FPU at boot time.
> >
> >       nofxsr          [BUGS=X86-32] Disables x86 floating point extended
> > diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c
> > index 731325a..28ef7f3 100644
> > --- a/arch/mips/kernel/elf.c
> > +++ b/arch/mips/kernel/elf.c
> > @@ -326,8 +326,47 @@ void mips_set_personality_nan(struct arch_elf_state *state)
> >       }
> >   }
> >
> > +static int nonxstack = EXSTACK_DEFAULT;
> > +
> > +/*
> > + * kernel parameter: nonxstack=on|off
> > + *
> > + *   Force setting up stack and heap as non-executable or
> > + *   executable regardless of PT_GNU_STACK entry. Both
> > + *   stack and heap are affected, despite the name. Valid
> > + *   arguments: on, off.
> > + *
> > + *     nonxstack=on:   Force non-executable stack and heap
> > + *     nonxstack=off:  Force executable stack and heap
> > + *
> > + *   If ommited, stack and heap will or will not be set
> > + *   up as non-executable depending on PT_GNU_STACK
> > + *   entry and possibly other factors.
> > + */
> > +static int __init nonxstack_setup(char *str)
> > +{
> > +     if (!strcmp(str, "on"))
> > +             nonxstack = EXSTACK_DISABLE_X;
> > +     else if (!strcmp(str, "off"))
> > +             nonxstack = EXSTACK_ENABLE_X;
> > +     else
> > +             pr_err("Malformed nonxstack format! nonxstack=on|off\n");
> > +
> > +     return 1;
> > +}
> > +__setup("nonxstack=", nonxstack_setup);
> > +
> >   int mips_elf_read_implies_exec(void *elf_ex, int exstack)
> >   {
> > +     switch (nonxstack) {
> > +     case EXSTACK_DISABLE_X:
> > +             return 0;
> > +     case EXSTACK_ENABLE_X:
> > +             return 1;
> > +     default:
> > +             break;
> > +     }
> > +
> >       if (exstack != EXSTACK_DISABLE_X) {
> >               /* The binary doesn't request a non-executable stack */
> >               return 1;
> >
>
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
@ 2017-12-01 11:35           ` Miodrag Dinic
  0 siblings, 0 replies; 17+ messages in thread
From: Miodrag Dinic @ 2017-12-01 11:35 UTC (permalink / raw)
  To: James Hogan
  Cc: David Daney, Aleksandar Markovic, linux-mips,
	Aleksandar Markovic, Andrew Morton, DengCheng Zhu, Ding Tianhong,
	Douglas Leung, Frederic Weisbecker, Goran Ferenc, Ingo Molnar,
	James Cowgill, Jonathan Corbet, linux-doc, linux-kernel,
	Marc Zyngier, Matt Redfearn, Mimi Zohar, Paul Burton,
	Paul E. McKenney, Petar Jovanovic, Raghu Gandham, Ralf Baechle,
	Thomas Gleixner, Tom Saeger

Hi everyone,

just a note, even though we started discussion on V2 version of this patch, an up to date V3 version is available at :
https://patchwork.kernel.org/patch/10068871/

It contains spelling check fixes.

Thank you.

Kind regards,
Miodrag

________________________________________
From: Miodrag Dinic
Sent: Thursday, November 30, 2017 2:06 PM
To: James Hogan
Cc: David Daney; Aleksandar Markovic; linux-mips@linux-mips.org; Aleksandar Markovic; Andrew Morton; DengCheng Zhu; Ding Tianhong; Douglas Leung; Frederic Weisbecker; Goran Ferenc; Ingo Molnar; James Cowgill; Jonathan Corbet; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Marc Zyngier; Matt Redfearn; Mimi Zohar; Paul Burton; Paul E. McKenney; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Thomas Gleixner; Tom Saeger
Subject: RE: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter

Hi James,

> > We do have PT_GNU_STACK flags set correctly, this feature is required to
> > workaround CPU revisions which do not have RIXI support.
>
> RIXI support can be discovered programatically from CP0_Config3.RXI
> (cpu_has_rixi in asm/cpu-features.h), so I don't follow why CPUs without
> RIXI would require a kernel parameter.

The following patch introduced change in behavior with regards to
stack & heap execute-ability :
commit 1a770b85c1f1c1ee37afd7cef5237ffc4c970f04
Author: Paul Burton <paul.burton@imgtec.com>
Date:   Fri Jul 8 11:06:20 2016 +0100

    MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present

    The stack and heap have both been executable by default on MIPS until
    now. This patch changes the default to be non-executable, but only for
    ELF binaries with a non-executable PT_GNU_STACK header present. This
    does apply to both the heap & the stack, despite the name PT_GNU_STACK,
    and this matches the behaviour of other architectures like ARM & x86.

    Current MIPS toolchains do not produce the PT_GNU_STACK header, which
    means that we can rely upon this patch not changing the behaviour of
    existing binaries. The new default will only take effect for newly
    compiled binaries once toolchains are updated to support PT_GNU_STACK,
    and since those binaries are newly compiled they can be compiled
    expecting the change in default behaviour. Again this matches the way in
    which the ARM & x86 architectures handled their implementations of
    non-executable memory.

    Signed-off-by: Paul Burton <paul.burton@imgtec.com>
    Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com>
    Cc: Maciej Rozycki <maciej.rozycki@imgtec.com>
    Cc: Faraz Shahbazker <faraz.shahbazker@imgtec.com>
    Cc: Raghu Gandham <raghu.gandham@imgtec.com>
    Cc: Matthew Fortune <matthew.fortune@imgtec.com>
    Cc: linux-mips@linux-mips.org
    Patchwork: https://patchwork.linux-mips.org/patch/13765/
    Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

....

When kernel is detecting the type of mapping it should apply :

fs/binfmt_elf.c:
...
        if (elf_read_implies_exec(loc->elf_ex, executable_stack))
                current->personality |= READ_IMPLIES_EXEC;
...

this effectively calls mips_elf_read_implies_exec() which performs a check:
...
        if (!cpu_has_rixi) {
                /* The CPU doesn't support non-executable memory */
                return 1;
        }

        return 0;
}

This will in turn make stack & heap executable on processors without RIXI, which are practically all processors with MIPS ISA R < 6.

We would like to have an option to override this and force non-executable mappings for such systems.

Kind regards,
Miodrag
________________________________________
From: James Hogan
Sent: Thursday, November 30, 2017 11:09 AM
To: Miodrag Dinic
Cc: David Daney; Aleksandar Markovic; linux-mips@linux-mips.org; Aleksandar Markovic; Andrew Morton; DengCheng Zhu; Ding Tianhong; Douglas Leung; Frederic Weisbecker; Goran Ferenc; Ingo Molnar; James Cowgill; Jonathan Corbet; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Marc Zyngier; Matt Redfearn; Mimi Zohar; Paul Burton; Paul E. McKenney; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Thomas Gleixner; Tom Saeger
Subject: Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter

On Thu, Nov 30, 2017 at 09:34:15AM +0000, Miodrag Dinic wrote:
> Hi David,
>
> Sorry for a late response, please find answers in-lined:
>
> > > If this parameter is omitted, kernel behavior remains the same as it
> > > was before this patch is applied.
> >
> > Do other architectures have a similar hack?
> >
> > If arm{,64} and x86 don't need this, what would make MIPS so special
> > that we have to carry this around?
>
> Yes, there are similar workarounds. Just a couple lines above
> nonxstack description in the documentation there are :
>       noexec          [IA-64]
>
>       noexec          [X86]
>                       On X86-32 available only on PAE configured kernels.
>                       noexec=on: enable non-executable mappings (default)
>                       noexec=off: disable non-executable mappings
> ...
>
>       noexec32        [X86-64]
>                       This affects only 32-bit executables.
>                       noexec32=on: enable non-executable mappings (default)
>                               read doesn't imply executable mappings
>                       noexec32=off: disable non-executable mappings
>                               read implies executable mappings
>
> > >
> > > This functionality is convenient during debugging and is especially
> > > useful for Android development where non-exec stack is required.
> >
> > Why not just set the PT_GNU_STACK flags correctly in the first place?
>
> We do have PT_GNU_STACK flags set correctly, this feature is required to
> workaround CPU revisions which do not have RIXI support.

RIXI support can be discovered programatically from CP0_Config3.RXI
(cpu_has_rixi in asm/cpu-features.h), so I don't follow why CPUs without
RIXI would require a kernel parameter.

Cheers
James

>
> Kind regards,
> Miodrag
> ________________________________________
> From: David Daney [ddaney@caviumnetworks.com]
> Sent: Tuesday, November 21, 2017 9:53 PM
> To: Aleksandar Markovic; linux-mips@linux-mips.org
> Cc: Miodrag Dinic; Aleksandar Markovic; Andrew Morton; DengCheng Zhu; Ding Tianhong; Douglas Leung; Frederic Weisbecker; Goran Ferenc; Ingo Molnar; James Cowgill; James Hogan; Jonathan Corbet; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Marc Zyngier; Matt Redfearn; Mimi Zohar; Paul Burton; Paul E. McKenney; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Thomas Gleixner; Tom Saeger
> Subject: Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
>
> On 11/21/2017 05:56 AM, Aleksandar Markovic wrote:
> > From: Miodrag Dinic <miodrag.dinic@mips.com>
> >
> > Add a new kernel parameter to override the default behavior related
> > to the decision whether to set up stack as non-executable in function
> > mips_elf_read_implies_exec().
> >
> > The new parameter is used to control non executable stack and heap,
> > regardless of PT_GNU_STACK entry. This does apply to both stack and
> > heap, despite the name.
> >
> > Allowed values:
> >
> > nonxstack=on  Force non-exec stack & heap
> > nonxstack=off Force executable stack & heap
> >
> > If this parameter is omitted, kernel behavior remains the same as it
> > was before this patch is applied.
>
> Do other architectures have a similar hack?
>
> If arm{,64} and x86 don't need this, what would make MIPS so special
> that we have to carry this around?
>
>
> >
> > This functionality is convenient during debugging and is especially
> > useful for Android development where non-exec stack is required.
>
> Why not just set the PT_GNU_STACK flags correctly in the first place?
>
> >
> > Signed-off-by: Miodrag Dinic <miodrag.dinic@mips.com>
> > Signed-off-by: Aleksandar Markovic <aleksandar.markovic@mips.com>
> > ---
> >   Documentation/admin-guide/kernel-parameters.txt | 11 +++++++
> >   arch/mips/kernel/elf.c                          | 39 +++++++++++++++++++++++++
> >   2 files changed, 50 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index b74e133..99464ee 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -2614,6 +2614,17 @@
> >                       noexec32=off: disable non-executable mappings
> >                               read implies executable mappings
> >
> > +     nonxstack       [MIPS]
> > +                     Force setting up stack and heap as non-executable or
> > +                     executable regardless of PT_GNU_STACK entry. Both
> > +                     stack and heap are affected, despite the name. Valid
> > +                     arguments: on, off.
> > +                     nonxstack=on:   Force non-executable stack and heap
> > +                     nonxstack=off:  Force executable stack and heap
> > +                     If ommited, stack and heap will or will not be set
> > +                     up as non-executable depending on PT_GNU_STACK
> > +                     entry and possibly other factors.
> > +
> >       nofpu           [MIPS,SH] Disable hardware FPU at boot time.
> >
> >       nofxsr          [BUGS=X86-32] Disables x86 floating point extended
> > diff --git a/arch/mips/kernel/elf.c b/arch/mips/kernel/elf.c
> > index 731325a..28ef7f3 100644
> > --- a/arch/mips/kernel/elf.c
> > +++ b/arch/mips/kernel/elf.c
> > @@ -326,8 +326,47 @@ void mips_set_personality_nan(struct arch_elf_state *state)
> >       }
> >   }
> >
> > +static int nonxstack = EXSTACK_DEFAULT;
> > +
> > +/*
> > + * kernel parameter: nonxstack=on|off
> > + *
> > + *   Force setting up stack and heap as non-executable or
> > + *   executable regardless of PT_GNU_STACK entry. Both
> > + *   stack and heap are affected, despite the name. Valid
> > + *   arguments: on, off.
> > + *
> > + *     nonxstack=on:   Force non-executable stack and heap
> > + *     nonxstack=off:  Force executable stack and heap
> > + *
> > + *   If ommited, stack and heap will or will not be set
> > + *   up as non-executable depending on PT_GNU_STACK
> > + *   entry and possibly other factors.
> > + */
> > +static int __init nonxstack_setup(char *str)
> > +{
> > +     if (!strcmp(str, "on"))
> > +             nonxstack = EXSTACK_DISABLE_X;
> > +     else if (!strcmp(str, "off"))
> > +             nonxstack = EXSTACK_ENABLE_X;
> > +     else
> > +             pr_err("Malformed nonxstack format! nonxstack=on|off\n");
> > +
> > +     return 1;
> > +}
> > +__setup("nonxstack=", nonxstack_setup);
> > +
> >   int mips_elf_read_implies_exec(void *elf_ex, int exstack)
> >   {
> > +     switch (nonxstack) {
> > +     case EXSTACK_DISABLE_X:
> > +             return 0;
> > +     case EXSTACK_ENABLE_X:
> > +             return 1;
> > +     default:
> > +             break;
> > +     }
> > +
> >       if (exstack != EXSTACK_DISABLE_X) {
> >               /* The binary doesn't request a non-executable stack */
> >               return 1;
> >
>
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
  2017-11-30 13:06         ` Miodrag Dinic
  (?)
  (?)
@ 2017-12-01 17:38         ` David Daney
  -1 siblings, 0 replies; 17+ messages in thread
From: David Daney @ 2017-12-01 17:38 UTC (permalink / raw)
  To: Miodrag Dinic, James Hogan
  Cc: Aleksandar Markovic, linux-mips, Aleksandar Markovic,
	Andrew Morton, DengCheng Zhu, Ding Tianhong, Douglas Leung,
	Frederic Weisbecker, Goran Ferenc, Ingo Molnar, James Cowgill,
	Jonathan Corbet, linux-doc, linux-kernel, Marc Zyngier,
	Matt Redfearn, Mimi Zohar, Paul Burton, Paul E. McKenney,
	Petar Jovanovic, Raghu Gandham, Ralf Baechle, Thomas Gleixner,
	Tom Saeger

On 11/30/2017 05:06 AM, Miodrag Dinic wrote:
> Hi James,
> 
>>> We do have PT_GNU_STACK flags set correctly, this feature is required to
>>> workaround CPU revisions which do not have RIXI support.
>>
>> RIXI support can be discovered programatically from CP0_Config3.RXI
>> (cpu_has_rixi in asm/cpu-features.h), so I don't follow why CPUs without
>> RIXI would require a kernel parameter.
> 
> The following patch introduced change in behavior with regards to
> stack & heap execute-ability :
> commit 1a770b85c1f1c1ee37afd7cef5237ffc4c970f04
> Author: Paul Burton <paul.burton@imgtec.com>
> Date:   Fri Jul 8 11:06:20 2016 +0100
> 
>      MIPS: non-exec stack & heap when non-exec PT_GNU_STACK is present
>      
>      The stack and heap have both been executable by default on MIPS until
>      now. This patch changes the default to be non-executable, but only for
>      ELF binaries with a non-executable PT_GNU_STACK header present. This
>      does apply to both the heap & the stack, despite the name PT_GNU_STACK,
>      and this matches the behaviour of other architectures like ARM & x86.
>      
>      Current MIPS toolchains do not produce the PT_GNU_STACK header, which
>      means that we can rely upon this patch not changing the behaviour of
>      existing binaries. The new default will only take effect for newly
>      compiled binaries once toolchains are updated to support PT_GNU_STACK,
>      and since those binaries are newly compiled they can be compiled
>      expecting the change in default behaviour. Again this matches the way in
>      which the ARM & x86 architectures handled their implementations of
>      non-executable memory.
>      
>      Signed-off-by: Paul Burton <paul.burton@imgtec.com>
>      Cc: Leonid Yegoshin <leonid.yegoshin@imgtec.com>
>      Cc: Maciej Rozycki <maciej.rozycki@imgtec.com>
>      Cc: Faraz Shahbazker <faraz.shahbazker@imgtec.com>
>      Cc: Raghu Gandham <raghu.gandham@imgtec.com>
>      Cc: Matthew Fortune <matthew.fortune@imgtec.com>
>      Cc: linux-mips@linux-mips.org
>      Patchwork: https://patchwork.linux-mips.org/patch/13765/
>      Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
> 
> ....
> 
> When kernel is detecting the type of mapping it should apply :
> 
> fs/binfmt_elf.c:
> ...
> 	if (elf_read_implies_exec(loc->elf_ex, executable_stack))
> 		current->personality |= READ_IMPLIES_EXEC;
> ...
> 
> this effectively calls mips_elf_read_implies_exec() which performs a check:
> ...
> 	if (!cpu_has_rixi) {
> 		/* The CPU doesn't support non-executable memory */
> 		return 1;
> 	}
> 
> 	return 0;
> }
> 
> This will in turn make stack & heap executable on processors without RIXI, which are practically all processors with MIPS ISA R < 6.
> 

All Cavium processors since OCTEON Plus (more than ten years ago) 
support RIXI.

> We would like to have an option to override this and force non-executable mappings for such systems.

This is what I don't understand.  If a system doesn't support XI, then 
no mapping can possibly be non-executable.

There may be some utility in disabling the use of the RIXI bits on 
systems that do support them.  But no command line can conjure 
functional RIXI on systems that don't support it.

Also, this does nothing for multi-threaded programs where libc sets the 
permissions on the thread stacks.

If you really need something, at a minimum, use the same parameter name 
that x86 uses.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
  2017-11-30 13:06         ` Miodrag Dinic
                           ` (2 preceding siblings ...)
  (?)
@ 2017-12-06 17:50         ` Maciej W. Rozycki
  2017-12-06 18:24           ` Paul Burton
  -1 siblings, 1 reply; 17+ messages in thread
From: Maciej W. Rozycki @ 2017-12-06 17:50 UTC (permalink / raw)
  To: Miodrag Dinic
  Cc: James Hogan, David Daney, Aleksandar Markovic, linux-mips,
	Aleksandar Markovic, Andrew Morton, DengCheng Zhu, Ding Tianhong,
	Douglas Leung, Frederic Weisbecker, Goran Ferenc, Ingo Molnar,
	James Cowgill, Jonathan Corbet, linux-doc, linux-kernel,
	Marc Zyngier, Matt Redfearn, Mimi Zohar, Paul Burton,
	Paul E. McKenney, Petar Jovanovic, Raghu Gandham, Ralf Baechle,
	Thomas Gleixner, Tom Saeger

Hi Miodrag,

> When kernel is detecting the type of mapping it should apply :
> 
> fs/binfmt_elf.c:
> ...
> 	if (elf_read_implies_exec(loc->elf_ex, executable_stack))
> 		current->personality |= READ_IMPLIES_EXEC;
> ...
> 
> this effectively calls mips_elf_read_implies_exec() which performs a check:
> ...
> 	if (!cpu_has_rixi) {
> 		/* The CPU doesn't support non-executable memory */
> 		return 1;
> 	}
> 
> 	return 0;
> }
> 
> This will in turn make stack & heap executable on processors without 
> RIXI, which are practically all processors with MIPS ISA R < 6.
> 
> We would like to have an option to override this and force 
> non-executable mappings for such systems.

 Of course you can't force a non-executable mapping with a system where 
all valid pages are executable, as David has already noted.  Did you mean 
the other condition, that is:

	if (exstack != EXSTACK_DISABLE_X) {
		/* The binary doesn't request a non-executable stack */
		return 1;
	}

?  In which case you do want to respect the lack of the RIXI feature, 
i.e.:

int mips_elf_read_implies_exec(void *elf_ex, int exstack)
{
	if (!cpu_has_rixi) {
		/* The CPU doesn't support non-executable memory */
		return 1;
        }

	switch (nonxstack) {
	case EXSTACK_DISABLE_X:
		return 0;
	case EXSTACK_ENABLE_X:
		return 1;
	default:
		break;
	}

	if (exstack != EXSTACK_DISABLE_X) {
		/* The binary doesn't request a non-executable stack */
		return 1;
	}

	return 0;
}

(I'd replace `break' with `return exstack != EXSTACK_DISABLE_X' and 
discard the code that follows, but that can be a separate optimisation).

 What problem are you trying to solve anyway?  Is it not something that 
can be handled with the `execstack' utility?

 NB as someone has observed with programs that do not request a 
non-executable stack we actually propagate the execute permission to all 
data pages.  Is it not something we would want to handle differently?

  Maciej

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
  2017-12-06 17:50         ` Maciej W. Rozycki
@ 2017-12-06 18:24           ` Paul Burton
  2017-12-07 11:33             ` Miodrag Dinic
  0 siblings, 1 reply; 17+ messages in thread
From: Paul Burton @ 2017-12-06 18:24 UTC (permalink / raw)
  To: Maciej W. Rozycki, Aleksandar Markovic, Aleksandar Markovic
  Cc: Miodrag Dinic, James Hogan, David Daney, linux-mips,
	Andrew Morton, DengCheng Zhu, Ding Tianhong, Douglas Leung,
	Frederic Weisbecker, Goran Ferenc, Ingo Molnar, James Cowgill,
	Jonathan Corbet, linux-doc, linux-kernel, Marc Zyngier,
	Matt Redfearn, Mimi Zohar, Paul E. McKenney, Petar Jovanovic,
	Raghu Gandham, Ralf Baechle, Thomas Gleixner, Tom Saeger

Hi Maciej, Aleksandar,

On Wed, Dec 06, 2017 at 05:50:52PM +0000, Maciej W. Rozycki wrote:
>  What problem are you trying to solve anyway?  Is it not something that 
> can be handled with the `execstack' utility?

The commit message states that for Android "non-exec stack is required".
Is Android checking that then Aleksandar? If so, how? I presume what you
actually want here is for the kernel to lie & indicate to whatever part
of Android that performs this check that the stack is non-executable
even when it is really executable?

Is this aimed at the Android emulator? If so would it be possible to
instead implement RIXI support & make the non-exec stack actually work?

>  NB as someone has observed with programs that do not request a 
> non-executable stack we actually propagate the execute permission to all 
> data pages.  Is it not something we would want to handle differently?

It would of course be ideal to mark data/heap memory non-executable -
the question is how should we know that it's safe to do so. The approach
I took in 1a770b85c1f1 ("MIPS: non-exec stack & heap when non-exec
PT_GNU_STACK is present") was to require the PT_GNU_STACK header in
order to mark both stack & heap non-executable, for reasons outlined in
its commit message:

  - I was told at the time that no MIPS tools were yet emitting
    PT_GNU_STACK, so we wouldn't be changing the behaviour of any
    existing binaries & thus wouldn't break any.

  - It matches the behaviour of both ARM & x86.

Marking the heap non-executable by default would have advantages in that
we wouldn't need to worry about icache coherency for it in
set_pte_at()/__update_cache(), so one idea I had was that we could
possibly initially mark pages non-executable in the TLB & later enable
execution only if we take a TLBXI exception, with the assumption being
that in most cases we'll never try executing from the heap. That's not
an idea I've yet found the time to implement or measure the impact of
though.

Thanks,
    Paul

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
  2017-12-06 18:24           ` Paul Burton
@ 2017-12-07 11:33             ` Miodrag Dinic
  2018-01-02 18:35               ` Maciej W. Rozycki
  2018-02-08 11:55               ` James Hogan
  0 siblings, 2 replies; 17+ messages in thread
From: Miodrag Dinic @ 2017-12-07 11:33 UTC (permalink / raw)
  To: Paul Burton, Maciej Rozycki, Aleksandar Markovic, Aleksandar Markovic
  Cc: James Hogan, David Daney, linux-mips, Andrew Morton,
	DengCheng Zhu, Ding Tianhong, Douglas Leung, Frederic Weisbecker,
	Goran Ferenc, Ingo Molnar, James Cowgill, Jonathan Corbet,
	linux-doc, linux-kernel, Marc Zyngier, Matt Redfearn, Mimi Zohar,
	Paul E. McKenney, Petar Jovanovic, Raghu Gandham, Ralf Baechle,
	Thomas Gleixner, Tom Saeger

Hi Paul, David, Maciej,

thanks for taking interest in this topic and your valuable comments.
Aleksandar is currently on sick leave but I think I can address
all previous questions and concerns by answering to this Pauls last post:

> On Wed, Dec 06, 2017 at 05:50:52PM +0000, Maciej W. Rozycki wrote:
> >  What problem are you trying to solve anyway?  Is it not something that 
> > can be handled with the `execstack' utility?
> 
> The commit message states that for Android "non-exec stack is required".
> Is Android checking that then Aleksandar? If so, how? 

Android is using SELinux configured to disallow NX mappings by handling
the following sepolicy rules :
* Executable stack (execstack)
* Executable heap (execheap)
* File-based executable code which has been modified (execmod)
* All other executable memory (execmem)

Nice explanation can be found here:
https://android-review.googlesource.com/#/c/platform/bionic/+/145004/

> I presume what you
> actually want here is for the kernel to lie & indicate to whatever part
> of Android that performs this check that the stack is non-executable
> even when it is really executable?

Basically yes, because we do not have other options at this point.
And this kernel parameter should definitely not be used in a production
environments. Hopefully, any future MIPS Android device would have
CPU with RIXI.

> Is this aimed at the Android emulator? If so would it be possible to
> instead implement RIXI support & make the non-exec stack actually work?

This issue was indeed detected using Android emulator while testing the "Ranchu"
platform kernel we are trying to integrate in another series.

We could probably change the emulator and enable RIXI, but the whole point of using it
is to emulate real HW as close as possible (And detect issues which could be
found on real HW as well), so I would tend to keep it unmodified.
But if we do not come to a consensus regarding this patch, we would need to modify it.

For emulating android on different MIPS arch variants we are using the
following CPU emulations:
- For mips32[r1|r2][dsp] - we are using 74Kf - No RIXI
- For mips32r5[msa] - we are using P5600 - has RIXI
- For mips[32|64]r6[msa] - I6400 - has RIXI

The CI20 development board (lacks XI) would also be affected  by this issue and it has been running
Android for quite some time now. The kernel for CI20 which we are currently using
is 3.18 and it does NOT yet contain 1a770b85c1f1 ("MIPS: non-exec stack & heap when non-exec
PT_GNU_STACK is present").

The effect of not having some workaround like this in the kernel, would
be to run Android only in SELinux permissive mode.

> >  NB as someone has observed with programs that do not request a 
> > non-executable stack we actually propagate the execute permission to all 
> > data pages.  Is it not something we would want to handle differently?
> 
> It would of course be ideal to mark data/heap memory non-executable -
> the question is how should we know that it's safe to do so. The approach
> I took in 1a770b85c1f1 ("MIPS: non-exec stack & heap when non-exec
> PT_GNU_STACK is present") was to require the PT_GNU_STACK header in
> order to mark both stack & heap non-executable, for reasons outlined in
> its commit message:
> 
>   - I was told at the time that no MIPS tools were yet emitting
>     PT_GNU_STACK, so we wouldn't be changing the behaviour of any
>     existing binaries & thus wouldn't break any.
> 
>   - It matches the behaviour of both ARM & x86.
> 

For Android, Google placed PT_GNU_STACK header in crtend* and this effectively
makes all executables and libraries request NX stack:
https://android-review.googlesource.com/#/c/platform/bionic/+/40773/

The toolchain for Android was updated as well:
https://android-review.googlesource.com/#/c/platform/ndk/+/37140/

------------------

@David:

> All Cavium processors since OCTEON Plus (more than ten years ago)
> support RIXI.

Sure, I missed to mention Cavium. Than I suppose, Cavium would have no issues running Android.

> Also, this does nothing for multi-threaded programs where libc sets the
> permissions on the thread stacks.

I don't think Android bionic is doing anything similar, at least we
haven't seen any issues so far related to this.

> If you really need something, at a minimum, use the same parameter name
> that x86 uses.

Sure, I agree, this was my dilemma as well, we will update the parameter name
to be "noexec".

Kind regards,
Miodrag
________________________________________
From: Paul Burton [paul.burton@mips.com]
Sent: Wednesday, December 6, 2017 7:24 PM
To: Maciej Rozycki; Aleksandar Markovic; Aleksandar Markovic
Cc: Miodrag Dinic; James Hogan; David Daney; linux-mips@linux-mips.org; Andrew Morton; DengCheng Zhu; Ding Tianhong; Douglas Leung; Frederic Weisbecker; Goran Ferenc; Ingo Molnar; James Cowgill; Jonathan Corbet; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; Marc Zyngier; Matt Redfearn; Mimi Zohar; Paul E. McKenney; Petar Jovanovic; Raghu Gandham; Ralf Baechle; Thomas Gleixner; Tom Saeger
Subject: Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter

Hi Maciej, Aleksandar,

On Wed, Dec 06, 2017 at 05:50:52PM +0000, Maciej W. Rozycki wrote:
>  What problem are you trying to solve anyway?  Is it not something that
> can be handled with the `execstack' utility?

The commit message states that for Android "non-exec stack is required".
Is Android checking that then Aleksandar? If so, how? I presume what you
actually want here is for the kernel to lie & indicate to whatever part
of Android that performs this check that the stack is non-executable
even when it is really executable?

Is this aimed at the Android emulator? If so would it be possible to
instead implement RIXI support & make the non-exec stack actually work?

>  NB as someone has observed with programs that do not request a
> non-executable stack we actually propagate the execute permission to all
> data pages.  Is it not something we would want to handle differently?

It would of course be ideal to mark data/heap memory non-executable -
the question is how should we know that it's safe to do so. The approach
I took in 1a770b85c1f1 ("MIPS: non-exec stack & heap when non-exec
PT_GNU_STACK is present") was to require the PT_GNU_STACK header in
order to mark both stack & heap non-executable, for reasons outlined in
its commit message:

  - I was told at the time that no MIPS tools were yet emitting
    PT_GNU_STACK, so we wouldn't be changing the behaviour of any
    existing binaries & thus wouldn't break any.

  - It matches the behaviour of both ARM & x86.

Marking the heap non-executable by default would have advantages in that
we wouldn't need to worry about icache coherency for it in
set_pte_at()/__update_cache(), so one idea I had was that we could
possibly initially mark pages non-executable in the TLB & later enable
execution only if we take a TLBXI exception, with the assumption being
that in most cases we'll never try executing from the heap. That's not
an idea I've yet found the time to implement or measure the impact of
though.

Thanks,
    Paul

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
  2017-12-07 11:33             ` Miodrag Dinic
@ 2018-01-02 18:35               ` Maciej W. Rozycki
  2018-02-08 11:55               ` James Hogan
  1 sibling, 0 replies; 17+ messages in thread
From: Maciej W. Rozycki @ 2018-01-02 18:35 UTC (permalink / raw)
  To: Miodrag Dinic
  Cc: Paul Burton, Aleksandar Markovic, Aleksandar Markovic,
	James Hogan, David Daney, linux-mips, Andrew Morton,
	DengCheng Zhu, Ding Tianhong, Douglas Leung, Frederic Weisbecker,
	Goran Ferenc, Ingo Molnar, James Cowgill, Jonathan Corbet,
	linux-doc, linux-kernel, Marc Zyngier, Matt Redfearn, Mimi Zohar,
	Paul E. McKenney, Petar Jovanovic, Raghu Gandham, Ralf Baechle,
	Thomas Gleixner, Tom Saeger

Hi Miodrag,

> > I presume what you
> > actually want here is for the kernel to lie & indicate to whatever part
> > of Android that performs this check that the stack is non-executable
> > even when it is really executable?
> 
> Basically yes, because we do not have other options at this point.

 Please make the purpose of this option unambiguous in documentation then, 
along with suitable precautionary notes about any adverse consequences of 
its use.

  Maciej

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
  2017-12-07 11:33             ` Miodrag Dinic
  2018-01-02 18:35               ` Maciej W. Rozycki
@ 2018-02-08 11:55               ` James Hogan
  2018-02-13 16:06                 ` Aleksandar Markovic
  1 sibling, 1 reply; 17+ messages in thread
From: James Hogan @ 2018-02-08 11:55 UTC (permalink / raw)
  To: Miodrag Dinic
  Cc: Paul Burton, Maciej Rozycki, Aleksandar Markovic,
	Aleksandar Markovic, David Daney, linux-mips, Andrew Morton,
	DengCheng Zhu, Ding Tianhong, Douglas Leung, Frederic Weisbecker,
	Goran Ferenc, Ingo Molnar, James Cowgill, Jonathan Corbet,
	linux-doc, linux-kernel, Marc Zyngier, Matt Redfearn, Mimi Zohar,
	Paul E. McKenney, Petar Jovanovic, Raghu Gandham, Ralf Baechle,
	Thomas Gleixner, Tom Saeger

[-- Attachment #1: Type: text/plain, Size: 2109 bytes --]

Hi,

On Thu, Dec 07, 2017 at 11:33:47AM +0000, Miodrag Dinic wrote:
> > On Wed, Dec 06, 2017 at 05:50:52PM +0000, Maciej W. Rozycki wrote:
> > >  What problem are you trying to solve anyway?  Is it not something that 
> > > can be handled with the `execstack' utility?
> > 
> > The commit message states that for Android "non-exec stack is required".
> > Is Android checking that then Aleksandar? If so, how? 
> 
> Android is using SELinux configured to disallow NX mappings by handling
> the following sepolicy rules :
> * Executable stack (execstack)
> * Executable heap (execheap)
> * File-based executable code which has been modified (execmod)
> * All other executable memory (execmem)

...

> The effect of not having some workaround like this in the kernel, would
> be to run Android only in SELinux permissive mode.

So you want to override the lack of RIXI so that SELinux sees an
RX->RW->RX transition as execmod instead of execmem (since without RIXI
its effectively RX->RWX->RX which is execmem)?

Looking at file_map_prot_check(), it does the execmem check on this
condition:

if (default_noexec &&
    (prot & PROT_EXEC) && (!file || IS_PRIVATE(file_inode(file)) ||
			   (!shared && (prot & PROT_WRITE)))) {
	/*
	 * We are making executable an anonymous mapping or a
	 * private file mapping that will also be writable.
	 * This has an additional check.
	 */

default_noexec is set if VM_DATA_DEFAULT_FLAGS doesn't have the exec
flag set, and that flag depends on current->personality &
READ_IMPLIES_EXEC, which depends on elf_read_implies_exec(), i.e.
mips_elf_read_implies_exec(), and that should already return 1 if RIXI
is unavailable.

I.e.

mips_elf_read_implies_exec() == 1

elf_read_implies_exec() == 1

READ_IMPLIES_EXEC will be set in current->personality

VM_DATA_DEFAULT_FLAGS will have VM_EXEC set

default_noexec will be set to 0 in selinux_init()

none of the execmem, execheap, execstack, execmod permission
checks should take place.

So whats the problem exactly? Perhaps I misinterpreted something.

Cheers
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter
  2018-02-08 11:55               ` James Hogan
@ 2018-02-13 16:06                 ` Aleksandar Markovic
  0 siblings, 0 replies; 17+ messages in thread
From: Aleksandar Markovic @ 2018-02-13 16:06 UTC (permalink / raw)
  To: James Hogan, Miodrag Dinic
  Cc: Paul Burton, Maciej Rozycki, Aleksandar Markovic, David Daney,
	linux-mips, Andrew Morton, DengCheng Zhu, Ding Tianhong,
	Douglas Leung, Frederic Weisbecker, Goran Ferenc, Ingo Molnar,
	James Cowgill, Jonathan Corbet, linux-doc, linux-kernel,
	Marc Zyngier, Matt Redfearn, Mimi Zohar, Paul E. McKenney,
	Petar Jovanovic, Raghu Gandham, Ralf Baechle, Thomas Gleixner,
	Tom Saeger

> 
> ________________________________________
> From: James Hogan [jhogan@kernel.org]
> Sent: Thursday, February 8, 2018 12:55 PM
> 
> Hi,
> 
> On Thu, Dec 07, 2017 at 11:33:47AM +0000, Miodrag Dinic wrote:
> > > On Wed, Dec 06, 2017 at 05:50:52PM +0000, Maciej W. Rozycki wrote:
> > > >  What problem are you trying to solve anyway?  Is it not something that
> > > > can be handled with the `execstack' utility?
> > >
> > > The commit message states that for Android "non-exec stack is required".
> > > Is Android checking that then Aleksandar? If so, how?
> >
> > Android is using SELinux configured to disallow NX mappings by handling
> > the following sepolicy rules :
> > * Executable stack (execstack)
> > * Executable heap (execheap)
> > * File-based executable code which has been modified (execmod)
> > * All other executable memory (execmem)
> 
> ...
> 
> > The effect of not having some workaround like this in the kernel, would
> > be to run Android only in SELinux permissive mode.
> 
> So you want to override the lack of RIXI so that SELinux sees an
> RX->RW->RX transition as execmod instead of execmem (since without RIXI
> its effectively RX->RWX->RX which is execmem)?
> 

That is correct.

> 
> Looking at file_map_prot_check(), it does the execmem check on this
> condition:
> 
> if (default_noexec &&
>     (prot & PROT_EXEC) && (!file || IS_PRIVATE(file_inode(file)) ||
>                            (!shared && (prot & PROT_WRITE)))) {
>         /*
>          * We are making executable an anonymous mapping or a
>          * private file mapping that will also be writable.
>          * This has an additional check.
>          */
> 
> default_noexec is set if VM_DATA_DEFAULT_FLAGS doesn't have the exec
> flag set, and that flag depends on current->personality &
> READ_IMPLIES_EXEC, which depends on elf_read_implies_exec(), i.e.
> mips_elf_read_implies_exec(), and that should already return 1 if RIXI
> is unavailable.
> 
> I.e.
> 
> mips_elf_read_implies_exec() == 1
> 
> elf_read_implies_exec() == 1
> 
> READ_IMPLIES_EXEC will be set in current->personality
> 
> VM_DATA_DEFAULT_FLAGS will have VM_EXEC set
> 
> default_noexec will be set to 0 in selinux_init()
> 
> none of the execmem, execheap, execstack, execmod permission
> checks should take place.
> 
> So whats the problem exactly? Perhaps I misinterpreted something.

Thanks, James, for the analysis of this scenario! Hope the additional
info below will be useful for clarifying this matter.

-------------------

Let me rephrase the scenario (for configurations where cpu_has_rixi
equals to 0) that you described: (line numbers may be approximate)


1. mips_elf_read_implies_exec() will return 1
    (arch/mips/kernel/elf.c:336)

2. elf_read_implies_exec() will return 1
    (arch/mips/include/asm/elf.h:513)

3. READ_IMPLIES_EXEC will be set in current->personality
    (fs/binfmt_elf_fdpic.c:357)

4. VM_DATA_DEFAULT_FLAGS will have VM_EXEC set
    (while execiting selinux_init() in security/selinux/hooks.c:6644)

5. default_noexec will be set to 0 in selinux_init()
    (security/selinux/hooks.c:6644)

6. none of the execmem, execstack permission checks should take place.

-------------------

However, in reality, these steps are not executed in this order, but in the following one:


4a. VM_DATA_DEFAULT_FLAGS will *NOT* have VM_EXEC set
    (while execiting selinux_init() in security/selinux/hooks.c:6644)

5a. default_noexec will be set to *1* in selinux_init()
    (security/selinux/hooks.c:6644)

1. mips_elf_read_implies_exec() will return 1
    (arch/mips/kernel/elf.c:336)

2. elf_read_implies_exec() will return 1
    (arch/mips/include/asm/elf.h:513)

3. READ_IMPLIES_EXEC will be set in current->personality
    (fs/binfmt_elf_fdpic.c:357)

6a. both the execmem, execstack permission checks *do* take place.

------------------

The proposed change does affect the kernel in the sense that it indeed
hides the executable vulnerability of the system without RIXI support.
But we have made it unambiguous in the comments what are the
consequences of using this option.

------------------


Regards,

Aleksandar

> 
> Cheers
> James

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2018-02-13 16:14 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-21 13:56 [PATCH v2] MIPS: Add nonxstack=on|off kernel parameter Aleksandar Markovic
2017-11-21 16:50 ` Randy Dunlap
2017-11-21 20:53 ` David Daney
2017-11-30  9:34   ` Miodrag Dinic
2017-11-30  9:34     ` Miodrag Dinic
2017-11-30 10:09     ` James Hogan
2017-11-30 13:06       ` Miodrag Dinic
2017-11-30 13:06         ` Miodrag Dinic
2017-12-01 11:35         ` Miodrag Dinic
2017-12-01 11:35           ` Miodrag Dinic
2017-12-01 17:38         ` David Daney
2017-12-06 17:50         ` Maciej W. Rozycki
2017-12-06 18:24           ` Paul Burton
2017-12-07 11:33             ` Miodrag Dinic
2018-01-02 18:35               ` Maciej W. Rozycki
2018-02-08 11:55               ` James Hogan
2018-02-13 16:06                 ` Aleksandar Markovic

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.