Message ID | 7e7b0688c907e54f3b11ddfb9a8f44511d475fd7.1634983809.git.christophe.leroy@csgroup.eu (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [v2,01/10] powerpc/nohash: Fix __ptep_set_access_flags() and ptep_set_wrprotect() | expand |
Related | show |
On 23/10/2021 13:47, Christophe Leroy wrote: > set_memory_x() calls pte_mkexec() which sets _PAGE_EXEC. > set_memory_nx() calls pte_exprotec() which clears _PAGE_EXEC. > > Book3e has 2 bits, UX and SX, which defines the exec rights > resp. for user (PR=1) and for kernel (PR=0). > > _PAGE_EXEC is defined as UX only. > > An executable kernel page is set with either _PAGE_KERNEL_RWX > or _PAGE_KERNEL_ROX, which both have SX set and UX cleared. > > So set_memory_nx() call for an executable kernel page does > nothing because UX is already cleared. > > And set_memory_x() on a non-executable kernel page makes it > executable for the user and keeps it non-executable for kernel. > > Also, pte_exec() always returns 'false' on kernel pages, because > it checks _PAGE_EXEC which doesn't include SX, so for instance > the W+X check doesn't work. > > To fix this: > - change tlb_low_64e.S to use _PAGE_BAP_UX instead of _PAGE_USER > - sets both UX and SX in _PAGE_EXEC so that pte_user() returns > true whenever one of the two bits is set and pte_exprotect() > clears both bits. > - Define a book3e specific version of pte_mkexec() which sets > either SX or UX based on UR. > > Fixes: 1f9ad21c3b38 ("powerpc/mm: Implement set_memory() routines") > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > v2: New pte_mkexec() in nohash/64/pgtable.h conflicts with the one in nohash/pte_book3e.h Should guard it with #ifndef pte_mkexec(), but as pte_book3e is the only user in 64 bits, then just remove it from there. Send v3 with only that change compared to v2. Christophe > --- > arch/powerpc/include/asm/nohash/32/pgtable.h | 2 ++ > arch/powerpc/include/asm/nohash/pte-book3e.h | 18 ++++++++++++++---- > arch/powerpc/mm/nohash/tlb_low_64e.S | 8 ++++---- > 3 files changed, 20 insertions(+), 8 deletions(-) > > diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h > index ac0a5ff48c3a..d6ba821a56ce 100644 > --- a/arch/powerpc/include/asm/nohash/32/pgtable.h > +++ b/arch/powerpc/include/asm/nohash/32/pgtable.h > @@ -193,10 +193,12 @@ static inline pte_t pte_wrprotect(pte_t pte) > } > #endif > > +#ifndef pte_mkexec > static inline pte_t pte_mkexec(pte_t pte) > { > return __pte(pte_val(pte) | _PAGE_EXEC); > } > +#endif > > #define pmd_none(pmd) (!pmd_val(pmd)) > #define pmd_bad(pmd) (pmd_val(pmd) & _PMD_BAD) > diff --git a/arch/powerpc/include/asm/nohash/pte-book3e.h b/arch/powerpc/include/asm/nohash/pte-book3e.h > index 813918f40765..f798640422c2 100644 > --- a/arch/powerpc/include/asm/nohash/pte-book3e.h > +++ b/arch/powerpc/include/asm/nohash/pte-book3e.h > @@ -48,7 +48,7 @@ > #define _PAGE_WRITETHRU 0x800000 /* W: cache write-through */ > > /* "Higher level" linux bit combinations */ > -#define _PAGE_EXEC _PAGE_BAP_UX /* .. and was cache cleaned */ > +#define _PAGE_EXEC (_PAGE_BAP_SX | _PAGE_BAP_UX) /* .. and was cache cleaned */ > #define _PAGE_RW (_PAGE_BAP_SW | _PAGE_BAP_UW) /* User write permission */ > #define _PAGE_KERNEL_RW (_PAGE_BAP_SW | _PAGE_BAP_SR | _PAGE_DIRTY) > #define _PAGE_KERNEL_RO (_PAGE_BAP_SR) > @@ -93,11 +93,11 @@ > /* Permission masks used to generate the __P and __S table */ > #define PAGE_NONE __pgprot(_PAGE_BASE) > #define PAGE_SHARED __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_RW) > -#define PAGE_SHARED_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_RW | _PAGE_EXEC) > +#define PAGE_SHARED_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_RW | _PAGE_BAP_UX) > #define PAGE_COPY __pgprot(_PAGE_BASE | _PAGE_USER) > -#define PAGE_COPY_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_EXEC) > +#define PAGE_COPY_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_BAP_UX) > #define PAGE_READONLY __pgprot(_PAGE_BASE | _PAGE_USER) > -#define PAGE_READONLY_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_EXEC) > +#define PAGE_READONLY_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_BAP_UX) > > #ifndef __ASSEMBLY__ > static inline pte_t pte_mkprivileged(pte_t pte) > @@ -113,6 +113,16 @@ static inline pte_t pte_mkuser(pte_t pte) > } > > #define pte_mkuser pte_mkuser > + > +static inline pte_t pte_mkexec(pte_t pte) > +{ > + if (pte_val(pte) & _PAGE_BAP_UR) > + return __pte((pte_val(pte) & ~_PAGE_BAP_SX) | _PAGE_BAP_UX); > + else > + return __pte((pte_val(pte) & ~_PAGE_BAP_UX) | _PAGE_BAP_SX); > +} > +#define pte_mkexec pte_mkexec > + > #endif /* __ASSEMBLY__ */ > > #endif /* __KERNEL__ */ > diff --git a/arch/powerpc/mm/nohash/tlb_low_64e.S b/arch/powerpc/mm/nohash/tlb_low_64e.S > index bf24451f3e71..9235e720e357 100644 > --- a/arch/powerpc/mm/nohash/tlb_low_64e.S > +++ b/arch/powerpc/mm/nohash/tlb_low_64e.S > @@ -222,7 +222,7 @@ tlb_miss_kernel_bolted: > > tlb_miss_fault_bolted: > /* We need to check if it was an instruction miss */ > - andi. r10,r11,_PAGE_EXEC|_PAGE_BAP_SX > + andi. r10,r11,_PAGE_BAP_UX|_PAGE_BAP_SX > bne itlb_miss_fault_bolted > dtlb_miss_fault_bolted: > tlb_epilog_bolted > @@ -239,7 +239,7 @@ itlb_miss_fault_bolted: > srdi r15,r16,60 /* get region */ > bne- itlb_miss_fault_bolted > > - li r11,_PAGE_PRESENT|_PAGE_EXEC /* Base perm */ > + li r11,_PAGE_PRESENT|_PAGE_BAP_UX /* Base perm */ > > /* We do the user/kernel test for the PID here along with the RW test > */ > @@ -614,7 +614,7 @@ itlb_miss_fault_e6500: > > /* We do the user/kernel test for the PID here along with the RW test > */ > - li r11,_PAGE_PRESENT|_PAGE_EXEC /* Base perm */ > + li r11,_PAGE_PRESENT|_PAGE_BAP_UX /* Base perm */ > oris r11,r11,_PAGE_ACCESSED@h > > cmpldi cr0,r15,0 /* Check for user region */ > @@ -734,7 +734,7 @@ normal_tlb_miss_done: > > normal_tlb_miss_access_fault: > /* We need to check if it was an instruction miss */ > - andi. r10,r11,_PAGE_EXEC > + andi. r10,r11,_PAGE_BAP_UX > bne 1f > ld r14,EX_TLB_DEAR(r12) > ld r15,EX_TLB_ESR(r12) >
Le 25/10/2021 à 23:53, Christophe Leroy a écrit : > > > On 23/10/2021 13:47, Christophe Leroy wrote: >> set_memory_x() calls pte_mkexec() which sets _PAGE_EXEC. >> set_memory_nx() calls pte_exprotec() which clears _PAGE_EXEC. >> >> Book3e has 2 bits, UX and SX, which defines the exec rights >> resp. for user (PR=1) and for kernel (PR=0). >> >> _PAGE_EXEC is defined as UX only. >> >> An executable kernel page is set with either _PAGE_KERNEL_RWX >> or _PAGE_KERNEL_ROX, which both have SX set and UX cleared. >> >> So set_memory_nx() call for an executable kernel page does >> nothing because UX is already cleared. >> >> And set_memory_x() on a non-executable kernel page makes it >> executable for the user and keeps it non-executable for kernel. >> >> Also, pte_exec() always returns 'false' on kernel pages, because >> it checks _PAGE_EXEC which doesn't include SX, so for instance >> the W+X check doesn't work. >> >> To fix this: >> - change tlb_low_64e.S to use _PAGE_BAP_UX instead of _PAGE_USER >> - sets both UX and SX in _PAGE_EXEC so that pte_user() returns >> true whenever one of the two bits is set and pte_exprotect() >> clears both bits. >> - Define a book3e specific version of pte_mkexec() which sets >> either SX or UX based on UR. >> >> Fixes: 1f9ad21c3b38 ("powerpc/mm: Implement set_memory() routines") >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> v2: New > > pte_mkexec() in nohash/64/pgtable.h conflicts with the one in > nohash/pte_book3e.h > > Should guard it with #ifndef pte_mkexec(), but as pte_book3e is the > only user in 64 bits, then just remove it from there. > > Send v3 with only that change compared to v2. Series v1 was merged into next so I submitted followup series with the three fixes. Christophe > > Christophe > >> --- >> arch/powerpc/include/asm/nohash/32/pgtable.h | 2 ++ >> arch/powerpc/include/asm/nohash/pte-book3e.h | 18 ++++++++++++++---- >> arch/powerpc/mm/nohash/tlb_low_64e.S | 8 ++++---- >> 3 files changed, 20 insertions(+), 8 deletions(-) >> >> diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h >> b/arch/powerpc/include/asm/nohash/32/pgtable.h >> index ac0a5ff48c3a..d6ba821a56ce 100644 >> --- a/arch/powerpc/include/asm/nohash/32/pgtable.h >> +++ b/arch/powerpc/include/asm/nohash/32/pgtable.h >> @@ -193,10 +193,12 @@ static inline pte_t pte_wrprotect(pte_t pte) >> } >> #endif >> +#ifndef pte_mkexec >> static inline pte_t pte_mkexec(pte_t pte) >> { >> return __pte(pte_val(pte) | _PAGE_EXEC); >> } >> +#endif >> #define pmd_none(pmd) (!pmd_val(pmd)) >> #define pmd_bad(pmd) (pmd_val(pmd) & _PMD_BAD) >> diff --git a/arch/powerpc/include/asm/nohash/pte-book3e.h >> b/arch/powerpc/include/asm/nohash/pte-book3e.h >> index 813918f40765..f798640422c2 100644 >> --- a/arch/powerpc/include/asm/nohash/pte-book3e.h >> +++ b/arch/powerpc/include/asm/nohash/pte-book3e.h >> @@ -48,7 +48,7 @@ >> #define _PAGE_WRITETHRU 0x800000 /* W: cache write-through */ >> /* "Higher level" linux bit combinations */ >> -#define _PAGE_EXEC _PAGE_BAP_UX /* .. and was cache cleaned */ >> +#define _PAGE_EXEC (_PAGE_BAP_SX | _PAGE_BAP_UX) /* .. and was >> cache cleaned */ >> #define _PAGE_RW (_PAGE_BAP_SW | _PAGE_BAP_UW) /* User write >> permission */ >> #define _PAGE_KERNEL_RW (_PAGE_BAP_SW | _PAGE_BAP_SR | >> _PAGE_DIRTY) >> #define _PAGE_KERNEL_RO (_PAGE_BAP_SR) >> @@ -93,11 +93,11 @@ >> /* Permission masks used to generate the __P and __S table */ >> #define PAGE_NONE __pgprot(_PAGE_BASE) >> #define PAGE_SHARED __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_RW) >> -#define PAGE_SHARED_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_RW >> | _PAGE_EXEC) >> +#define PAGE_SHARED_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_RW >> | _PAGE_BAP_UX) >> #define PAGE_COPY __pgprot(_PAGE_BASE | _PAGE_USER) >> -#define PAGE_COPY_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_EXEC) >> +#define PAGE_COPY_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_BAP_UX) >> #define PAGE_READONLY __pgprot(_PAGE_BASE | _PAGE_USER) >> -#define PAGE_READONLY_X __pgprot(_PAGE_BASE | _PAGE_USER | >> _PAGE_EXEC) >> +#define PAGE_READONLY_X __pgprot(_PAGE_BASE | _PAGE_USER | >> _PAGE_BAP_UX) >> #ifndef __ASSEMBLY__ >> static inline pte_t pte_mkprivileged(pte_t pte) >> @@ -113,6 +113,16 @@ static inline pte_t pte_mkuser(pte_t pte) >> } >> #define pte_mkuser pte_mkuser >> + >> +static inline pte_t pte_mkexec(pte_t pte) >> +{ >> + if (pte_val(pte) & _PAGE_BAP_UR) >> + return __pte((pte_val(pte) & ~_PAGE_BAP_SX) | _PAGE_BAP_UX); >> + else >> + return __pte((pte_val(pte) & ~_PAGE_BAP_UX) | _PAGE_BAP_SX); >> +} >> +#define pte_mkexec pte_mkexec >> + >> #endif /* __ASSEMBLY__ */ >> #endif /* __KERNEL__ */ >> diff --git a/arch/powerpc/mm/nohash/tlb_low_64e.S >> b/arch/powerpc/mm/nohash/tlb_low_64e.S >> index bf24451f3e71..9235e720e357 100644 >> --- a/arch/powerpc/mm/nohash/tlb_low_64e.S >> +++ b/arch/powerpc/mm/nohash/tlb_low_64e.S >> @@ -222,7 +222,7 @@ tlb_miss_kernel_bolted: >> tlb_miss_fault_bolted: >> /* We need to check if it was an instruction miss */ >> - andi. r10,r11,_PAGE_EXEC|_PAGE_BAP_SX >> + andi. r10,r11,_PAGE_BAP_UX|_PAGE_BAP_SX >> bne itlb_miss_fault_bolted >> dtlb_miss_fault_bolted: >> tlb_epilog_bolted >> @@ -239,7 +239,7 @@ itlb_miss_fault_bolted: >> srdi r15,r16,60 /* get region */ >> bne- itlb_miss_fault_bolted >> - li r11,_PAGE_PRESENT|_PAGE_EXEC /* Base perm */ >> + li r11,_PAGE_PRESENT|_PAGE_BAP_UX /* Base perm */ >> /* We do the user/kernel test for the PID here along with the RW >> test >> */ >> @@ -614,7 +614,7 @@ itlb_miss_fault_e6500: >> /* We do the user/kernel test for the PID here along with the RW >> test >> */ >> - li r11,_PAGE_PRESENT|_PAGE_EXEC /* Base perm */ >> + li r11,_PAGE_PRESENT|_PAGE_BAP_UX /* Base perm */ >> oris r11,r11,_PAGE_ACCESSED@h >> cmpldi cr0,r15,0 /* Check for user region */ >> @@ -734,7 +734,7 @@ normal_tlb_miss_done: >> normal_tlb_miss_access_fault: >> /* We need to check if it was an instruction miss */ >> - andi. r10,r11,_PAGE_EXEC >> + andi. r10,r11,_PAGE_BAP_UX >> bne 1f >> ld r14,EX_TLB_DEAR(r12) >> ld r15,EX_TLB_ESR(r12) >>
diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h index ac0a5ff48c3a..d6ba821a56ce 100644 --- a/arch/powerpc/include/asm/nohash/32/pgtable.h +++ b/arch/powerpc/include/asm/nohash/32/pgtable.h @@ -193,10 +193,12 @@ static inline pte_t pte_wrprotect(pte_t pte) } #endif +#ifndef pte_mkexec static inline pte_t pte_mkexec(pte_t pte) { return __pte(pte_val(pte) | _PAGE_EXEC); } +#endif #define pmd_none(pmd) (!pmd_val(pmd)) #define pmd_bad(pmd) (pmd_val(pmd) & _PMD_BAD) diff --git a/arch/powerpc/include/asm/nohash/pte-book3e.h b/arch/powerpc/include/asm/nohash/pte-book3e.h index 813918f40765..f798640422c2 100644 --- a/arch/powerpc/include/asm/nohash/pte-book3e.h +++ b/arch/powerpc/include/asm/nohash/pte-book3e.h @@ -48,7 +48,7 @@ #define _PAGE_WRITETHRU 0x800000 /* W: cache write-through */ /* "Higher level" linux bit combinations */ -#define _PAGE_EXEC _PAGE_BAP_UX /* .. and was cache cleaned */ +#define _PAGE_EXEC (_PAGE_BAP_SX | _PAGE_BAP_UX) /* .. and was cache cleaned */ #define _PAGE_RW (_PAGE_BAP_SW | _PAGE_BAP_UW) /* User write permission */ #define _PAGE_KERNEL_RW (_PAGE_BAP_SW | _PAGE_BAP_SR | _PAGE_DIRTY) #define _PAGE_KERNEL_RO (_PAGE_BAP_SR) @@ -93,11 +93,11 @@ /* Permission masks used to generate the __P and __S table */ #define PAGE_NONE __pgprot(_PAGE_BASE) #define PAGE_SHARED __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_RW) -#define PAGE_SHARED_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_RW | _PAGE_EXEC) +#define PAGE_SHARED_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_RW | _PAGE_BAP_UX) #define PAGE_COPY __pgprot(_PAGE_BASE | _PAGE_USER) -#define PAGE_COPY_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_EXEC) +#define PAGE_COPY_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_BAP_UX) #define PAGE_READONLY __pgprot(_PAGE_BASE | _PAGE_USER) -#define PAGE_READONLY_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_EXEC) +#define PAGE_READONLY_X __pgprot(_PAGE_BASE | _PAGE_USER | _PAGE_BAP_UX) #ifndef __ASSEMBLY__ static inline pte_t pte_mkprivileged(pte_t pte) @@ -113,6 +113,16 @@ static inline pte_t pte_mkuser(pte_t pte) } #define pte_mkuser pte_mkuser + +static inline pte_t pte_mkexec(pte_t pte) +{ + if (pte_val(pte) & _PAGE_BAP_UR) + return __pte((pte_val(pte) & ~_PAGE_BAP_SX) | _PAGE_BAP_UX); + else + return __pte((pte_val(pte) & ~_PAGE_BAP_UX) | _PAGE_BAP_SX); +} +#define pte_mkexec pte_mkexec + #endif /* __ASSEMBLY__ */ #endif /* __KERNEL__ */ diff --git a/arch/powerpc/mm/nohash/tlb_low_64e.S b/arch/powerpc/mm/nohash/tlb_low_64e.S index bf24451f3e71..9235e720e357 100644 --- a/arch/powerpc/mm/nohash/tlb_low_64e.S +++ b/arch/powerpc/mm/nohash/tlb_low_64e.S @@ -222,7 +222,7 @@ tlb_miss_kernel_bolted: tlb_miss_fault_bolted: /* We need to check if it was an instruction miss */ - andi. r10,r11,_PAGE_EXEC|_PAGE_BAP_SX + andi. r10,r11,_PAGE_BAP_UX|_PAGE_BAP_SX bne itlb_miss_fault_bolted dtlb_miss_fault_bolted: tlb_epilog_bolted @@ -239,7 +239,7 @@ itlb_miss_fault_bolted: srdi r15,r16,60 /* get region */ bne- itlb_miss_fault_bolted - li r11,_PAGE_PRESENT|_PAGE_EXEC /* Base perm */ + li r11,_PAGE_PRESENT|_PAGE_BAP_UX /* Base perm */ /* We do the user/kernel test for the PID here along with the RW test */ @@ -614,7 +614,7 @@ itlb_miss_fault_e6500: /* We do the user/kernel test for the PID here along with the RW test */ - li r11,_PAGE_PRESENT|_PAGE_EXEC /* Base perm */ + li r11,_PAGE_PRESENT|_PAGE_BAP_UX /* Base perm */ oris r11,r11,_PAGE_ACCESSED@h cmpldi cr0,r15,0 /* Check for user region */ @@ -734,7 +734,7 @@ normal_tlb_miss_done: normal_tlb_miss_access_fault: /* We need to check if it was an instruction miss */ - andi. r10,r11,_PAGE_EXEC + andi. r10,r11,_PAGE_BAP_UX bne 1f ld r14,EX_TLB_DEAR(r12) ld r15,EX_TLB_ESR(r12)
set_memory_x() calls pte_mkexec() which sets _PAGE_EXEC. set_memory_nx() calls pte_exprotec() which clears _PAGE_EXEC. Book3e has 2 bits, UX and SX, which defines the exec rights resp. for user (PR=1) and for kernel (PR=0). _PAGE_EXEC is defined as UX only. An executable kernel page is set with either _PAGE_KERNEL_RWX or _PAGE_KERNEL_ROX, which both have SX set and UX cleared. So set_memory_nx() call for an executable kernel page does nothing because UX is already cleared. And set_memory_x() on a non-executable kernel page makes it executable for the user and keeps it non-executable for kernel. Also, pte_exec() always returns 'false' on kernel pages, because it checks _PAGE_EXEC which doesn't include SX, so for instance the W+X check doesn't work. To fix this: - change tlb_low_64e.S to use _PAGE_BAP_UX instead of _PAGE_USER - sets both UX and SX in _PAGE_EXEC so that pte_user() returns true whenever one of the two bits is set and pte_exprotect() clears both bits. - Define a book3e specific version of pte_mkexec() which sets either SX or UX based on UR. Fixes: 1f9ad21c3b38 ("powerpc/mm: Implement set_memory() routines") Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- v2: New --- arch/powerpc/include/asm/nohash/32/pgtable.h | 2 ++ arch/powerpc/include/asm/nohash/pte-book3e.h | 18 ++++++++++++++---- arch/powerpc/mm/nohash/tlb_low_64e.S | 8 ++++---- 3 files changed, 20 insertions(+), 8 deletions(-)