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

[PATCH v2] 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.

Note that this only applies to the classic 32-bit PowerPC MMU and the
MPC8xx MMU, not Book E, 64-bit, etc.  These MMUs do not support per-page
no-exec, and thus this patch isn't taking away any effective protection
enforcement.  Currently, such accesses will fail only if the page in
question has not already been faulted on (and thus mapped).

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
v2: Added to the changelog to explain why this change isn't harmful.

 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 v2] Allow exec on 32-bit from readable, non-exec pages, with a warning.

[Paul Mackerras]

Scott Wood writes:

> 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.

Actually I see no good reason to enforce no-exec at all if we can't do
it consistently.  And if we're not going to enforce it then there is
no point whinging about it.

When I applied Segher's original patch I thought that we had the
read-implies-exec stuff enabled for the affected CPUs, but it turns
out we don't.  (We only have that stuff turned on for 32-bit processes
on 64-bit cpus - see elf_read_implies_exec in include/asm-powerpc/elf.h.)

Paul.

Re: [PATCH v2] 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.
>
> Actually I see no good reason to enforce no-exec at all if we can't do
> it consistently.  And if we're not going to enforce it then there is
> no point whinging about it.

I have a new patch with just this behaviour, Scott is
testing it on old glibc (I think it succeeded, need
confirmation though), I'll have it tested on new glibc
tomorrow.  So patch tomorrow, and let's try to forget
about all this after that, eh?  :-)

> When I applied Segher's original patch I thought that we had the
> read-implies-exec stuff enabled for the affected CPUs, but it turns
> out we don't.  (We only have that stuff turned on for 32-bit processes
> on 64-bit cpus - see elf_read_implies_exec in include/asm-powerpc/ 
> elf.h.)

I never thought "real code" would rely on executing stuff
it didn't map as executable before.  Silly me.  In my
defense, no one else noticed in time either ;-)


Segher

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

[Scott Wood]

Segher Boessenkool wrote:
>> Actually I see no good reason to enforce no-exec at all if we can't do
>> it consistently.  And if we're not going to enforce it then there is
>> no point whinging about it.
> 
> 
> I have a new patch with just this behaviour, Scott is
> testing it on old glibc (I think it succeeded, need
> confirmation though),

It worked fine on glibc 2.2.5 and 2.3.3 (the former of which failed 
without the patch).

-Scott

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

[Kumar Gala]

On Jul 11, 2007, at 10:32 AM, Scott Wood wrote:

> Segher Boessenkool wrote:
>>> Actually I see no good reason to enforce no-exec at all if we  
>>> can't do
>>> it consistently.  And if we're not going to enforce it then there is
>>> no point whinging about it.
>>
>>
>> I have a new patch with just this behaviour, Scott is
>> testing it on old glibc (I think it succeeded, need
>> confirmation though),
>
> It worked fine on glibc 2.2.5 and 2.3.3 (the former of which failed
> without the patch).

It sounds like this is a candidate for stable 2.6.22.x as well once  
you release the patch.

- k

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

[Segher Boessenkool]

>>>> Actually I see no good reason to enforce no-exec at all if we  
>>>> can't do
>>>> it consistently.  And if we're not going to enforce it then  
>>>> there is
>>>> no point whinging about it.
>>>
>>>
>>> I have a new patch with just this behaviour, Scott is
>>> testing it on old glibc (I think it succeeded, need
>>> confirmation though),
>>
>> It worked fine on glibc 2.2.5 and 2.3.3 (the former of which failed
>> without the patch).
>
> It sounds like this is a candidate for stable 2.6.22.x as well

Yes, it replaces the previous patch and is a bugfix to it.

> once you release the patch.

The only reason I didn't send it out immediately was to not
confuse things further with a broken patch ;-)


Segher

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

[Jon Loeliger]

On Mon, 2007-07-09 at 16:48, 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.
> 
> Note that this only applies to the classic 32-bit PowerPC MMU and the
> MPC8xx MMU, not Book E, 64-bit, etc.  These MMUs do not support per-page
> no-exec, and thus this patch isn't taking away any effective protection
> enforcement.  Currently, such accesses will fail only if the page in
> question has not already been faulted on (and thus mapped).
> 
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---

Well now.  I've spent a good chunk of today with Our Friend, git-bisect,
verifying that, in fact, the commit 9ba4ace39fdfe22268daca9f28c5df384ae462cf
breaks the 8641 HPCN port.  init doesn't run off an old NFS'ed root FS
worth a Steven J Hill of beans.

But luckily, this gave me the opportunity to then realize that
we should give a great big...

Amen-brother-by: Jon Loeliger <jdl@freescale.com>

to this patch from Scott.

So, an official plea to Paul to apply this to his tree.

jdl


> v2: Added to the changelog to explain why this change isn't harmful.
> 
>  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;

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

[Scott Wood]

Jon Loeliger wrote:
> But luckily, this gave me the opportunity to then realize that
> we should give a great big...
> 
> Amen-brother-by: Jon Loeliger <jdl@freescale.com>
> 
> to this patch from Scott.
> 
> So, an official plea to Paul to apply this to his tree.

Segher has a newer patch that supersedes this one.  I don't know if he's 
posted it to the list yet, though.

-Scott

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

[Jon Loeliger]

<Top Posting Carnac>

A:  They haven't been posted yet.

Q:  How do we know Segher has new patches?

</Top Posting Carnac>

So, like, the other day Scott Wood mumbled:
> Jon Loeliger wrote:
> > But luckily, this gave me the opportunity to then realize that
> > we should give a great big...
> > 
> > Amen-brother-by: Jon Loeliger <jdl@freescale.com>
> > 
> > to this patch from Scott.
> > 
> > So, an official plea to Paul to apply this to his tree.
> 
> Segher has a newer patch that supersedes this one.  I don't know if he's 
> posted it to the list yet, though.
> 
> -Scott

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

[Scott Wood]

Jon Loeliger wrote:
> A:  They haven't been posted yet.
> 
> Q:  How do we know Segher has new patches?

He sent it to me to test, and I told him it worked...

-Scott

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

[Segher Boessenkool]

>> A:  They haven't been posted yet.
>>
>> Q:  How do we know Segher has new patches?
>
> He sent it to me to test, and I told him it worked...

And I sent it to the list hours later, over a week ago.


Segher

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

[Kumar Gala]

On Jul 17, 2007, at 10:30 PM, Segher Boessenkool wrote:

>>> A:  They haven't been posted yet.
>>>
>>> Q:  How do we know Segher has new patches?
>>
>> He sent it to me to test, and I told him it worked...
>
> And I sent it to the list hours later, over a week ago.

Can someone send a ozlabs linuxppc list link or patchworks to the  
"new" patch.

All I can find are patches from Scott w/Paul doesn't like, not  
Segher's "new" version.

- k

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

[Segher Boessenkool]

>>>> A:  They haven't been posted yet.
>>>>
>>>> Q:  How do we know Segher has new patches?
>>>
>>> He sent it to me to test, and I told him it worked...
>>
>> And I sent it to the list hours later, over a week ago.
>
> Can someone send a ozlabs linuxppc list link or patchworks to the  
> "new" patch.

<http://ozlabs.org/pipermail/linuxppc-dev/2007-July/039076.html>
is Johannes' ack.
It seems ozlabs ate the original, will resend.


Segher

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

[Kumar Gala]

On Jul 18, 2007, at 7:07 AM, Segher Boessenkool wrote:

>>>>> A:  They haven't been posted yet.
>>>>>
>>>>> Q:  How do we know Segher has new patches?
>>>>
>>>> He sent it to me to test, and I told him it worked...
>>>
>>> And I sent it to the list hours later, over a week ago.
>>
>> Can someone send a ozlabs linuxppc list link or patchworks to the  
>> "new" patch.
>
> <http://ozlabs.org/pipermail/linuxppc-dev/2007-July/039076.html>
> is Johannes' ack.
> It seems ozlabs ate the original, will resend.

Thanks, I know I remember seeing the ack but never the original patch :)

- k

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

[Linas Vepstas]

On Tue, Jul 17, 2007 at 05:44:14PM -0500, Jon Loeliger wrote:
> <Top Posting Carnac>
> 
> A:  They haven't been posted yet.
> 
> Q:  How do we know Segher has new patches?
> 
> </Top Posting Carnac>

rotfl -- rolling on the floor loeliger !