[PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[Scott Wood]

In older versions of glibc (through 2.3), the dynamic linker executes a
small amount of code from the data segment, which is not marked as
executable.  A recent change (commit 9ba4ace39fdfe22268daca9f28c5df384ae462cf)
stops this from working; there should be a deprecation period before
older glibc versions stop working.

The problem has been observed on glibc 2.2.  While glibc 2.3 has the same
code, I did not see the problem; it may be that it accesses the page in
question as data before executing from it, and thus it is already mapped.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
Unfortunately, this didn't make it into 2.6.22, but it should probably go
into the stable branch...

 arch/powerpc/mm/fault.c |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 0ece513..2445512 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -125,6 +125,18 @@ static void do_dabr(struct pt_regs *regs, unsigned long address,
 }
 #endif /* !(CONFIG_4xx || CONFIG_BOOKE)*/
 
+#ifdef CONFIG_PPC32
+static void warn_exec_from_noexec(void)
+{
+	if (printk_ratelimit())
+		printk(KERN_WARNING "Process %s (%d) attempted to execute from "
+		                    "a non-executable page.\n"
+		       KERN_WARNING "This may stop working in future kernels.  "
+		                    "Please upgrade your libc.\n",
+		       current->comm, current->pid);
+}
+#endif
+
 /*
  * For 600- and 800-family processors, the error_code parameter is DSISR
  * for a data fault, SRR1 for an instruction fault. For 400-family processors
@@ -283,8 +295,16 @@ good_area:
 		/* protection fault */
 		if (error_code & DSISR_PROTFAULT)
 			goto bad_area;
-		if (!(vma->vm_flags & VM_EXEC))
+		if (!(vma->vm_flags & VM_EXEC)) {
+#ifdef CONFIG_PPC32
+			if (vma->vm_flags & VM_READ)
+				warn_exec_from_noexec();
+			else
+				goto bad_area;
+#else
 			goto bad_area;
+#endif
+		}
 #else
 		pte_t *ptep;
 		pmd_t *pmdp;
-- 
1.5.0.3

Re: [PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[Arnd Bergmann]

On Monday 09 July 2007, Scott Wood wrote:
> In older versions of glibc (through 2.3), the dynamic linker executes a
> small amount of code from the data segment, which is not marked as
> executable. =A0A recent change (commit 9ba4ace39fdfe22268daca9f28c5df384a=
e462cf)
> stops this from working; there should be a deprecation period before
> older glibc versions stop working.
>=20
> The problem has been observed on glibc 2.2. =A0While glibc 2.3 has the sa=
me
> code, I did not see the problem; it may be that it accesses the page in
> question as data before executing from it, and thus it is already mapped.

I may be missing the obvious, but doesn't that defeat the purpose of
non-executable mappings?

	Arnd <><

Re: [PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[Arnd Bergmann]

On Monday 09 July 2007, Scott Wood wrote:
> The hardware in question doesn't support non-executable mappings;=20
> otherwise, it'd never have worked in the first place. =A0Note that this i=
s=20
> only allowed on 32-bit, non-book-E.
>=20
> There isn't much value in enforcing non-exec mappings only if it happens=
=20
> to be the first fault on a given page.

Ok, much clearer now. Do you mind adding that explanation to the
changelog text? If it's going into the stable kernel update, there
may be more people reading this with the same problem understanding
the patch.

	Arnd <><

Re: [PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[Scott Wood]

Arnd Bergmann wrote:
> On Monday 09 July 2007, Scott Wood wrote:
> 
>>In older versions of glibc (through 2.3), the dynamic linker executes a
>>small amount of code from the data segment, which is not marked as
>>executable.  A recent change (commit 9ba4ace39fdfe22268daca9f28c5df384ae462cf)
>>stops this from working; there should be a deprecation period before
>>older glibc versions stop working.
>>
>>The problem has been observed on glibc 2.2.  While glibc 2.3 has the same
>>code, I did not see the problem; it may be that it accesses the page in
>>question as data before executing from it, and thus it is already mapped.
> 
> 
> I may be missing the obvious, but doesn't that defeat the purpose of
> non-executable mappings?

The hardware in question doesn't support non-executable mappings; 
otherwise, it'd never have worked in the first place.  Note that this is 
only allowed on 32-bit, non-book-E.

There isn't much value in enforcing non-exec mappings only if it happens 
to be the first fault on a given page.

-Scott

Re: [PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[Linas Vepstas]

On Mon, Jul 09, 2007 at 04:16:40PM -0500, Scott Wood wrote:
> Arnd Bergmann wrote:
> > I may be missing the obvious, but doesn't that defeat the purpose of
> > non-executable mappings?
> 
> The hardware in question doesn't support non-executable mappings; 
> otherwise, it'd never have worked in the first place.  Note that this is 
> only allowed on 32-bit, non-book-E.
> 
> There isn't much value in enforcing non-exec mappings only if it happens 
> to be the first fault on a given page.

Thank you. I was reading this thread last week, and scratching my head,
thinking wtf ??

--linas

Re: [PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[Scott Wood]

Scott Wood wrote:
> Arnd Bergmann wrote:
>> I may be missing the obvious, but doesn't that defeat the purpose of
>> non-executable mappings?
> 
> 
> The hardware in question doesn't support non-executable mappings; 
> otherwise, it'd never have worked in the first place.  Note that this is 
> only allowed on 32-bit, non-book-E.

To be more precise, the classic MMU does appear to have non-exec 
functionality, but at the segment level, which is less than useful for 
implementing vma flags.

-Scott

Re: [PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[Segher Boessenkool]

>> I may be missing the obvious, but doesn't that defeat the purpose of
>> non-executable mappings?
>
> The hardware in question doesn't support non-executable mappings;

Not on a per-page basis, anyway.

> otherwise, it'd never have worked in the first place.  Note that  
> this is
> only allowed on 32-bit, non-book-E.
>
> There isn't much value in enforcing non-exec mappings only if it  
> happens
> to be the first fault on a given page.

Yeah.  Giving the warning is a good thing though.


Segher

Re: [PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[Segher Boessenkool]

> In older versions of glibc (through 2.3), the dynamic linker  
> executes a
> small amount of code from the data segment, which is not marked as
> executable.  A recent change (commit  
> 9ba4ace39fdfe22268daca9f28c5df384ae462cf)
> stops this from working; there should be a deprecation period before
> older glibc versions stop working.
>
> The problem has been observed on glibc 2.2.  While glibc 2.3 has  
> the same
> code, I did not see the problem; it may be that it accesses the  
> page in
> question as data before executing from it, and thus it is already  
> mapped.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>

Acked-by: Segher Boessenkool <segher@kernel.crashing.org>

> Unfortunately, this didn't make it into 2.6.22, but it should  
> probably go
> into the stable branch...

Both .21.x and .22.x I suppose; if we care about glibc 2.2.x
at all still, that is.

So to make double sure, this doesn't warn on glibc 2.3.x?


Segher

Re: [PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[Paul Mackerras]

Segher Boessenkool writes:

> Yeah.  Giving the warning is a good thing though.

No, it isn't; it's just noise, if we're not ever going to do anything
to prevent the behaviour - and we can't.

Paul.

Re: [PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[Segher Boessenkool]

>> Yeah.  Giving the warning is a good thing though.
>
> No, it isn't; it's just noise, if we're not ever going to do anything
> to prevent the behaviour - and we can't.

The same userland code will not run correctly on PPC64 or BookE
systems.  Is that not a reason to warn?


Segher

Re: [PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[Paul Mackerras]

Segher Boessenkool writes:

> >> Yeah.  Giving the warning is a good thing though.
> >
> > No, it isn't; it's just noise, if we're not ever going to do anything
> > to prevent the behaviour - and we can't.
> 
> The same userland code will not run correctly on PPC64 or BookE
> systems.  Is that not a reason to warn?

It *will* run on ppc64 systems, because there we get the
READ_IMPLIES_EXEC personality flag set via the elf_read_implies_exec
thing in include/asm-powerpc/elf.h.  The READ_IMPLIES_EXEC flag is
only set if we don't have the non-executable stack note in the ELF
header, i.e. only for old binaries or libraries.

As for Book E, that could be fixed using elf_read_implies_exec too, if
anyone cared.  In fact maybe the correct solution is to have

#define elf_read_implies_exec(ex, exec_stk) \
	(exec_stk != EXSTACK_DISABLE_X) : 0)

for all 32-bit powerpc.

Paul.

Re: [PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[David Gibson]

On Wed, Jul 11, 2007 at 02:03:24AM +0200, Segher Boessenkool wrote:
> >> Yeah.  Giving the warning is a good thing though.
> >
> > No, it isn't; it's just noise, if we're not ever going to do anything
> > to prevent the behaviour - and we can't.
> 
> The same userland code will not run correctly on PPC64 or BookE
> systems.  Is that not a reason to warn?

Way back when, I distinctly recall aborting my plans to implement
per-page exec on 40x, precisely because of executables like this.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[Segher Boessenkool]

>>>> Yeah.  Giving the warning is a good thing though.
>>>
>>> No, it isn't; it's just noise, if we're not ever going to do  
>>> anything
>>> to prevent the behaviour - and we can't.
>>
>> The same userland code will not run correctly on PPC64 or BookE
>> systems.  Is that not a reason to warn?
>
> Way back when, I distinctly recall aborting my plans to implement
> per-page exec on 40x, precisely because of executables like this.

I noticed some comments to that effect in the BookE code,
yes.  It seems userland has been fixed enough that you
could think about enabling it again FWIW.


Segher

Re: [PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[Kumar Gala]

On Jul 17, 2007, at 10:18 AM, Segher Boessenkool wrote:

>>>>> Yeah.  Giving the warning is a good thing though.
>>>>
>>>> No, it isn't; it's just noise, if we're not ever going to do
>>>> anything
>>>> to prevent the behaviour - and we can't.
>>>
>>> The same userland code will not run correctly on PPC64 or BookE
>>> systems.  Is that not a reason to warn?
>>
>> Way back when, I distinctly recall aborting my plans to implement
>> per-page exec on 40x, precisely because of executables like this.
>
> I noticed some comments to that effect in the BookE code,
> yes.  It seems userland has been fixed enough that you
> could think about enabling it again FWIW.

Did I miss the posting of the patch with the fix?

- k

Re: [PATCH] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[Segher Boessenkool]

>>> Way back when, I distinctly recall aborting my plans to implement
>>> per-page exec on 40x, precisely because of executables like this.
>>
>> I noticed some comments to that effect in the BookE code,
>> yes.  It seems userland has been fixed enough that you
>> could think about enabling it again FWIW.
>
> Did I miss the posting of the patch with the fix?

glibc-2.2 seems to be the last "bad" one.  We are at
glibc-2.6 or so nowadays...


Segher