* Re: [PATCH] Treat ISI faults as read faults on classic 32-bit PowerPC [not found] <5d56173bee3f9ea0050aa508e7f27cc932af7229.1184104284.git.segher@kernel.crashing.org> @ 2007-07-12 9:47 ` Johannes Berg 2007-07-18 15:30 ` Segher Boessenkool 1 sibling, 0 replies; 12+ messages in thread From: Johannes Berg @ 2007-07-12 9:47 UTC (permalink / raw) To: Segher Boessenkool; +Cc: linuxppc-dev [-- Attachment #1: Type: text/plain, Size: 2146 bytes --] On Wed, 2007-07-11 at 20:18 +0200, Segher Boessenkool wrote: > Some old software on ppc32 executes from pages it hasn't marked > executable. Since "classic" hardware doesn't distinguish between > execute and read accesses, the do_page_fault() code shouldn't > either. This makes glibc-2.2 work again on such hardware. > > Signed-off-by: Segher Boessenkool <segher@kernel•crashing.org> > Cc: Scott Wood <scottwood@freescale•com> > Cc: Johannes Berg <johannes@sipsolutions•net> > --- > Tested by Scott on 32-bit, glibc-2.2.5 and glibc-2.3.3 (no new > failures and problem solved), needs confirmation from Johannes > on his glibc-2.4 "---p" testcase. Could use testing on ppc64 > and BookE too, for good measure. Acked-by: Johannes Berg <johannes@sipsolutions•net> I tested this patch and it does fix the problem I was seeing. > This reverts the previous change and makes the bugfix behave > more like the arch/ppc code. > arch/powerpc/mm/fault.c | 14 ++++++++++---- > 1 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c > index 115b25f..5d7add0 100644 > --- a/arch/powerpc/mm/fault.c > +++ b/arch/powerpc/mm/fault.c > @@ -278,14 +278,17 @@ good_area: > goto bad_area; > #endif /* CONFIG_8xx */ > > +#ifdef CONFIG_PPC64 > if (is_exec) { > -#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE)) > /* protection fault */ > if (error_code & DSISR_PROTFAULT) > goto bad_area; > if (!(vma->vm_flags & VM_EXEC)) > goto bad_area; > -#else > + } else > + /* A read or write, code continues below... */ > +#elsif defined(CONFIG_4xx) || defined(CONFIG_BOOKE) > + if (is_exec) { > pte_t *ptep; > pmd_t *pmdp; > > @@ -310,9 +313,12 @@ good_area: > } > pte_unmap_unlock(ptep, ptl); > } > + } else > + /* A read or write, code continues below... */ > #endif > - /* a write */ > - } else if (is_write) { > + > + /* A read or write. Classic PPC32 execute is considered a read. */ > + if (is_write) { > if (!(vma->vm_flags & VM_WRITE)) > goto bad_area; > /* a read */ [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 190 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] Treat ISI faults as read faults on classic 32-bit PowerPC [not found] <5d56173bee3f9ea0050aa508e7f27cc932af7229.1184104284.git.segher@kernel.crashing.org> 2007-07-12 9:47 ` [PATCH] Treat ISI faults as read faults on classic 32-bit PowerPC Johannes Berg @ 2007-07-18 15:30 ` Segher Boessenkool 2007-07-19 0:00 ` Paul Mackerras 1 sibling, 1 reply; 12+ messages in thread From: Segher Boessenkool @ 2007-07-18 15:30 UTC (permalink / raw) To: linuxppc-dev; +Cc: Johannes Berg Some old software on ppc32 executes from pages it hasn't marked executable. Since "classic" hardware doesn't distinguish between execute and read accesses, the do_page_fault() code shouldn't either. This makes glibc-2.2 work again on such hardware. Signed-off-by: Segher Boessenkool <segher@kernel•crashing.org> Cc: Scott Wood <scottwood@freescale•com> Cc: Johannes Berg <johannes@sipsolutions•net> --- [Resend again, ozlabs' greylisting doesn't like me at all.] Tested by Scott on 32-bit, glibc-2.2.5 and glibc-2.3.3 (no new failures and problem solved), and by Johannes on his glibc-2.4 "---p" testcase. Could use testing on ppc64 and BookE too, for good measure. This reverts the previous change and makes the bugfix behave more like the arch/ppc code. arch/powerpc/mm/fault.c | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c index 115b25f..5d7add0 100644 --- a/arch/powerpc/mm/fault.c +++ b/arch/powerpc/mm/fault.c @@ -278,14 +278,17 @@ good_area: goto bad_area; #endif /* CONFIG_8xx */ +#ifdef CONFIG_PPC64 if (is_exec) { -#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE)) /* protection fault */ if (error_code & DSISR_PROTFAULT) goto bad_area; if (!(vma->vm_flags & VM_EXEC)) goto bad_area; -#else + } else + /* A read or write, code continues below... */ +#elsif defined(CONFIG_4xx) || defined(CONFIG_BOOKE) + if (is_exec) { pte_t *ptep; pmd_t *pmdp; @@ -310,9 +313,12 @@ good_area: } pte_unmap_unlock(ptep, ptl); } + } else + /* A read or write, code continues below... */ #endif - /* a write */ - } else if (is_write) { + + /* A read or write. Classic PPC32 execute is considered a read. */ + if (is_write) { if (!(vma->vm_flags & VM_WRITE)) goto bad_area; /* a read */ -- 1.5.2.1.144.gabc40-dirty ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] Treat ISI faults as read faults on classic 32-bit PowerPC 2007-07-18 15:30 ` Segher Boessenkool @ 2007-07-19 0:00 ` Paul Mackerras 2007-07-19 16:44 ` Jon Loeliger 2007-07-19 18:46 ` Segher Boessenkool 0 siblings, 2 replies; 12+ messages in thread From: Paul Mackerras @ 2007-07-19 0:00 UTC (permalink / raw) To: Segher Boessenkool; +Cc: linuxppc-dev, Johannes Berg Segher Boessenkool writes: > Some old software on ppc32 executes from pages it hasn't marked > executable. Since "classic" hardware doesn't distinguish between > execute and read accesses, the do_page_fault() code shouldn't > either. This makes glibc-2.2 work again on such hardware. > > Signed-off-by: Segher Boessenkool <segher@kernel•crashing.org> > Cc: Scott Wood <scottwood@freescale•com> > Cc: Johannes Berg <johannes@sipsolutions•net> > --- > [Resend again, ozlabs' greylisting doesn't like me at all.] > > Tested by Scott on 32-bit, glibc-2.2.5 and glibc-2.3.3 (no new > failures and problem solved), and by Johannes on his glibc-2.4 > "---p" testcase. Could use testing on ppc64 and BookE too, for > good measure. Hmmm. The dangling else clauses are pretty gross, and in fact we have the same problem on POWER3 and RS64 processors (to be fair, we had the problem before and didn't notice, but we should still fix it). How about this instead? Could people test it please? (Note that CPU_FTR_NOEXECUTE is 0 in 32-bit kernels.) Paul. diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c index 0ece513..99c3093 100644 --- a/arch/powerpc/mm/fault.c +++ b/arch/powerpc/mm/fault.c @@ -283,7 +283,13 @@ good_area: /* protection fault */ if (error_code & DSISR_PROTFAULT) goto bad_area; - if (!(vma->vm_flags & VM_EXEC)) + /* + * Allow execution from readable areas if the MMU does not + * provide separate controls over reading and executing. + */ + if (!(vma->vm_flags & VM_EXEC) && + (cpu_has_feature(CPU_FTR_NOEXECUTE) || + !(vma->vm_flags & (VM_READ | VM_WRITE)))) goto bad_area; #else pte_t *ptep; ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] Treat ISI faults as read faults on classic 32-bit PowerPC 2007-07-19 0:00 ` Paul Mackerras @ 2007-07-19 16:44 ` Jon Loeliger 2007-07-19 17:16 ` Segher Boessenkool 2007-07-19 18:46 ` Segher Boessenkool 1 sibling, 1 reply; 12+ messages in thread From: Jon Loeliger @ 2007-07-19 16:44 UTC (permalink / raw) To: Paul Mackerras; +Cc: linuxppc-dev@ozlabs•org, Johannes Berg On Wed, 2007-07-18 at 19:00, Paul Mackerras wrote: > Hmmm. The dangling else clauses are pretty gross, and in fact we have > the same problem on POWER3 and RS64 processors (to be fair, we had > the problem before and didn't notice, but we should still fix it). > > How about this instead? Could people test it please? (Note that > CPU_FTR_NOEXECUTE is 0 in 32-bit kernels.) > > Paul. > > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c > index 0ece513..99c3093 100644 > --- a/arch/powerpc/mm/fault.c > +++ b/arch/powerpc/mm/fault.c Acked-by: Jon Loeliger <jdl@freescale•com> Tested on 8641HPCN. Thanks, jdl ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Treat ISI faults as read faults on classic 32-bit PowerPC 2007-07-19 16:44 ` Jon Loeliger @ 2007-07-19 17:16 ` Segher Boessenkool 2007-07-19 18:57 ` Jon Loeliger 0 siblings, 1 reply; 12+ messages in thread From: Segher Boessenkool @ 2007-07-19 17:16 UTC (permalink / raw) To: Jon Loeliger; +Cc: linuxppc-dev@ozlabs•org, Johannes Berg, Paul Mackerras >> Hmmm. The dangling else clauses are pretty gross, and in fact we >> have >> the same problem on POWER3 and RS64 processors (to be fair, we had >> the problem before and didn't notice, but we should still fix it). >> >> How about this instead? Could people test it please? (Note that >> CPU_FTR_NOEXECUTE is 0 in 32-bit kernels.) >> >> Paul. >> >> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c >> index 0ece513..99c3093 100644 >> --- a/arch/powerpc/mm/fault.c >> +++ b/arch/powerpc/mm/fault.c > > Acked-by: Jon Loeliger <jdl@freescale•com> > > Tested on 8641HPCN. Which glibc versions? glibc-2.4 and newer are fine without the patch already, glibc-2.3 seems to get away by accident; but 2.2 (and before) are the problematic ones. No other userland program has been identified as needing this fix, btw -- thankfully, or the testing matrix would be _huge_. Segher ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Treat ISI faults as read faults on classic 32-bit PowerPC 2007-07-19 17:16 ` Segher Boessenkool @ 2007-07-19 18:57 ` Jon Loeliger 2007-07-19 19:02 ` Scott Wood 0 siblings, 1 reply; 12+ messages in thread From: Jon Loeliger @ 2007-07-19 18:57 UTC (permalink / raw) To: Segher Boessenkool; +Cc: linuxppc-dev@ozlabs•org, Johannes Berg, Paul Mackerras On Thu, 2007-07-19 at 12:16, Segher Boessenkool wrote: > > Tested on 8641HPCN. > > Which glibc versions? glibc-2.4 and newer are fine without the > patch already, glibc-2.3 seems to get away by accident; but 2.2 > (and before) are the problematic ones. > > No other userland program has been identified as needing this > fix, btw -- thankfully, or the testing matrix would be _huge_. > > > Segher Segher, Needless to say, one that showed the problem beforehand. :-) But to be precise: [root:~] ls -lsa /usr/lib/libglib* 0 lrwxrwxrwx 1 18005314 24012 21 Aug 15 2005 /usr/lib/libglib-1.2.so.0 -> libglib-1.2.so.0.0.10* 536 -rwxrwxr-x 1 18005314 24012 540931 Apr 8 2003 /usr/lib/libglib-1.2.so.0.0.10* 1064 -rw-rw-r-- 1 18005314 24012 1084256 Apr 8 2003 /usr/lib/libglib.a 4 -rwxrwxr-x 1 18005314 24012 662 Apr 8 2003 /usr/lib/libglib.la* 0 lrwxrwxrwx 1 18005314 24012 21 Aug 15 2005 /usr/lib/libglib.so -> libglib-1.2.so.0.0.10* Thanks, jdl ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Treat ISI faults as read faults on classic 32-bit PowerPC 2007-07-19 18:57 ` Jon Loeliger @ 2007-07-19 19:02 ` Scott Wood 2007-07-19 19:10 ` Jon Loeliger 0 siblings, 1 reply; 12+ messages in thread From: Scott Wood @ 2007-07-19 19:02 UTC (permalink / raw) To: Jon Loeliger; +Cc: linuxppc-dev@ozlabs•org, Johannes Berg, Paul Mackerras On Thu, Jul 19, 2007 at 01:57:00PM -0500, Jon Loeliger wrote: > Needless to say, one that showed the problem beforehand. :-) > But to be precise: > > [root:~] ls -lsa /usr/lib/libglib* > 0 lrwxrwxrwx 1 18005314 24012 21 Aug 15 2005 /usr/lib/libglib-1.2.so.0 -> libglib-1.2.so.0.0.10* > 536 -rwxrwxr-x 1 18005314 24012 540931 Apr 8 2003 /usr/lib/libglib-1.2.so.0.0.10* > 1064 -rw-rw-r-- 1 18005314 24012 1084256 Apr 8 2003 /usr/lib/libglib.a > 4 -rwxrwxr-x 1 18005314 24012 662 Apr 8 2003 /usr/lib/libglib.la* > 0 lrwxrwxrwx 1 18005314 24012 21 Aug 15 2005 /usr/lib/libglib.so -> libglib-1.2.so.0.0.10* glib != glibc. -Scott ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Treat ISI faults as read faults on classic 32-bit PowerPC 2007-07-19 19:02 ` Scott Wood @ 2007-07-19 19:10 ` Jon Loeliger 2007-07-19 20:59 ` Kumar Gala 0 siblings, 1 reply; 12+ messages in thread From: Jon Loeliger @ 2007-07-19 19:10 UTC (permalink / raw) To: Scott Wood; +Cc: linuxppc-dev@ozlabs•org, Johannes Berg, Paul Mackerras On Thu, 2007-07-19 at 14:02, Scott Wood wrote: > glib != glibc. > > -Scott D'oh. So, It doesn't say what version it is. But it is also dated 8-Apr-2003. jdl ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Treat ISI faults as read faults on classic 32-bit PowerPC 2007-07-19 19:10 ` Jon Loeliger @ 2007-07-19 20:59 ` Kumar Gala 0 siblings, 0 replies; 12+ messages in thread From: Kumar Gala @ 2007-07-19 20:59 UTC (permalink / raw) To: Jon Loeliger; +Cc: linuxppc-dev@ozlabs•org, Johannes Berg, Paul Mackerras On Jul 19, 2007, at 2:10 PM, Jon Loeliger wrote: > On Thu, 2007-07-19 at 14:02, Scott Wood wrote: > >> glib != glibc. >> >> -Scott > > D'oh. > > So, It doesn't say what version it is. > But it is also dated 8-Apr-2003. The glibc jdl tested against is 2.2.5. - k ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Treat ISI faults as read faults on classic 32-bit PowerPC 2007-07-19 0:00 ` Paul Mackerras 2007-07-19 16:44 ` Jon Loeliger @ 2007-07-19 18:46 ` Segher Boessenkool 2007-07-19 23:40 ` Paul Mackerras 1 sibling, 1 reply; 12+ messages in thread From: Segher Boessenkool @ 2007-07-19 18:46 UTC (permalink / raw) To: Paul Mackerras; +Cc: linuxppc-dev, Johannes Berg > Hmmm. The dangling else clauses are pretty gross, I hoped you wouldn't notice. I guess I shouldn't have commented them :-) "It was the cleanest thing I could come up with". Every other thing I tried ended up as a maze of #ifdefs or some incomprehensible cross-jumping mess; and I was aiming for a minimal fix, too. Or, perhaps, it was just a ploy to trick you into writing a patch yourself. > and in fact we have > the same problem on POWER3 and RS64 processors Right. Too bad there is no public documentation for either :-/ > (to be fair, we had > the problem before and didn't notice, but we should still fix it). Yeah. > How about this instead? It's the better way forward, consider my patch withdrawn :-) > - if (!(vma->vm_flags & VM_EXEC)) > + /* > + * Allow execution from readable areas if the MMU does not > + * provide separate controls over reading and executing. > + */ > + if (!(vma->vm_flags & VM_EXEC) && > + (cpu_has_feature(CPU_FTR_NOEXECUTE) || > + !(vma->vm_flags & (VM_READ | VM_WRITE)))) > goto bad_area; Should you really be testing VM_READ|VM_WRITE, or should it just be VM_READ? Oh, and that conditional might benefit from being split into two separate "if" statements, it's a bit hard to read this way. Segher ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Treat ISI faults as read faults on classic 32-bit PowerPC 2007-07-19 18:46 ` Segher Boessenkool @ 2007-07-19 23:40 ` Paul Mackerras 2007-07-20 7:42 ` Segher Boessenkool 0 siblings, 1 reply; 12+ messages in thread From: Paul Mackerras @ 2007-07-19 23:40 UTC (permalink / raw) To: Segher Boessenkool; +Cc: linuxppc-dev, Johannes Berg Segher Boessenkool writes: > Should you really be testing VM_READ|VM_WRITE, or should it just > be VM_READ? We test VM_READ | VM_WRITE | VM_EXEC in the read case below, and that is because we have no HPTE encoding to say "writable but not readable" or "executable but not readable". Similarly we have no encoding to say "writable but not executable" on classic processors, so if you have just VM_WRITE set, you get a page that is readable, writable and executable. Paul. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Treat ISI faults as read faults on classic 32-bit PowerPC 2007-07-19 23:40 ` Paul Mackerras @ 2007-07-20 7:42 ` Segher Boessenkool 0 siblings, 0 replies; 12+ messages in thread From: Segher Boessenkool @ 2007-07-20 7:42 UTC (permalink / raw) To: Paul Mackerras; +Cc: linuxppc-dev, Johannes Berg >> Should you really be testing VM_READ|VM_WRITE, or should it just >> be VM_READ? > > We test VM_READ | VM_WRITE | VM_EXEC in the read case below, and that > is because we have no HPTE encoding to say "writable but not readable" > or "executable but not readable". Similarly we have no encoding to > say "writable but not executable" on classic processors, so if you > have just VM_WRITE set, you get a page that is readable, writable and > executable. Ah yes. I thought "executable requires readable", but that is with the CPU its flags, not the Linux flags. Would it be a good idea to map Linux flags to CPU flags somewhere early in this function? It might simplify some code, and things certainly would become more readable. Segher ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2007-07-20 7:45 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <5d56173bee3f9ea0050aa508e7f27cc932af7229.1184104284.git.segher@kernel.crashing.org>
2007-07-12 9:47 ` [PATCH] Treat ISI faults as read faults on classic 32-bit PowerPC Johannes Berg
2007-07-18 15:30 ` Segher Boessenkool
2007-07-19 0:00 ` Paul Mackerras
2007-07-19 16:44 ` Jon Loeliger
2007-07-19 17:16 ` Segher Boessenkool
2007-07-19 18:57 ` Jon Loeliger
2007-07-19 19:02 ` Scott Wood
2007-07-19 19:10 ` Jon Loeliger
2007-07-19 20:59 ` Kumar Gala
2007-07-19 18:46 ` Segher Boessenkool
2007-07-19 23:40 ` Paul Mackerras
2007-07-20 7:42 ` Segher Boessenkool
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox