linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] [backport for 5.10] powerpc/603: Fix protection of user pages mapped with PROT_NONE
@ 2021-03-11 18:24 Christophe Leroy
  2021-03-12 10:03 ` Greg KH
  0 siblings, 1 reply; 2+ messages in thread
From: Christophe Leroy @ 2021-03-11 18:24 UTC (permalink / raw)
  To: stable, gregkh; +Cc: linux-kernel, linuxppc-dev

(cherry picked from commit c119565a15a628efdfa51352f9f6c5186e506a1c)

On book3s/32, page protection is defined by the PP bits in the PTE
which provide the following protection depending on the access
keys defined in the matching segment register:
- PP 00 means RW with key 0 and N/A with key 1.
- PP 01 means RW with key 0 and RO with key 1.
- PP 10 means RW with both key 0 and key 1.
- PP 11 means RO with both key 0 and key 1.

Since the implementation of kernel userspace access protection,
PP bits have been set as follows:
- PP00 for pages without _PAGE_USER
- PP01 for pages with _PAGE_USER and _PAGE_RW
- PP11 for pages with _PAGE_USER and without _PAGE_RW

For kernelspace segments, kernel accesses are performed with key 0
and user accesses are performed with key 1. As PP00 is used for
non _PAGE_USER pages, user can't access kernel pages not flagged
_PAGE_USER while kernel can.

For userspace segments, both kernel and user accesses are performed
with key 0, therefore pages not flagged _PAGE_USER are still
accessible to the user.

This shouldn't be an issue, because userspace is expected to be
accessible to the user. But unlike most other architectures, powerpc
implements PROT_NONE protection by removing _PAGE_USER flag instead of
flagging the page as not valid. This means that pages in userspace
that are not flagged _PAGE_USER shall remain inaccessible.

To get the expected behaviour, just mimic other architectures in the
TLB miss handler by checking _PAGE_USER permission on userspace
accesses as if it was the _PAGE_PRESENT bit.

Note that this problem only is only for 603 cores. The 604+ have
an hash table, and hash_page() function already implement the
verification of _PAGE_USER permission on userspace pages.

Fixes: f342adca3afc ("powerpc/32s: Prepare Kernel Userspace Access Protection")
Change-Id: I68bc5e5ff4542bdfcdcd12923fa96a5811707475
Cc: stable@vger.kernel.org # v5.2+
Reported-by: Christoph Plattner <christoph.plattner@thalesgroup.com>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/4a0c6e3bb8f0c162457bf54d9bc6fd8d7b55129f.1612160907.git.christophe.leroy@csgroup.eu
---
 arch/powerpc/kernel/head_book3s_32.S | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/head_book3s_32.S b/arch/powerpc/kernel/head_book3s_32.S
index 2729d8fa6e77..96b45901da64 100644
--- a/arch/powerpc/kernel/head_book3s_32.S
+++ b/arch/powerpc/kernel/head_book3s_32.S
@@ -461,10 +461,11 @@ InstructionTLBMiss:
 	cmplw	0,r1,r3
 #endif
 	mfspr	r2, SPRN_SPRG_PGDIR
-	li	r1,_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_EXEC
+	li	r1,_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_EXEC | _PAGE_USER
 #if defined(CONFIG_MODULES) || defined(CONFIG_DEBUG_PAGEALLOC)
 	bgt-	112f
 	lis	r2, (swapper_pg_dir - PAGE_OFFSET)@ha	/* if kernel address, use */
+	li	r1,_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_EXEC
 	addi	r2, r2, (swapper_pg_dir - PAGE_OFFSET)@l	/* kernel page table */
 #endif
 112:	rlwimi	r2,r3,12,20,29		/* insert top 10 bits of address */
@@ -523,9 +524,10 @@ DataLoadTLBMiss:
 	lis	r1, TASK_SIZE@h		/* check if kernel address */
 	cmplw	0,r1,r3
 	mfspr	r2, SPRN_SPRG_PGDIR
-	li	r1, _PAGE_PRESENT | _PAGE_ACCESSED
+	li	r1, _PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_USER
 	bgt-	112f
 	lis	r2, (swapper_pg_dir - PAGE_OFFSET)@ha	/* if kernel address, use */
+	li	r1, _PAGE_PRESENT | _PAGE_ACCESSED
 	addi	r2, r2, (swapper_pg_dir - PAGE_OFFSET)@l	/* kernel page table */
 112:	rlwimi	r2,r3,12,20,29		/* insert top 10 bits of address */
 	lwz	r2,0(r2)		/* get pmd entry */
@@ -599,9 +601,10 @@ DataStoreTLBMiss:
 	lis	r1, TASK_SIZE@h		/* check if kernel address */
 	cmplw	0,r1,r3
 	mfspr	r2, SPRN_SPRG_PGDIR
-	li	r1, _PAGE_RW | _PAGE_DIRTY | _PAGE_PRESENT | _PAGE_ACCESSED
+	li	r1, _PAGE_RW | _PAGE_DIRTY | _PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_USER
 	bgt-	112f
 	lis	r2, (swapper_pg_dir - PAGE_OFFSET)@ha	/* if kernel address, use */
+	li	r1, _PAGE_RW | _PAGE_DIRTY | _PAGE_PRESENT | _PAGE_ACCESSED
 	addi	r2, r2, (swapper_pg_dir - PAGE_OFFSET)@l	/* kernel page table */
 112:	rlwimi	r2,r3,12,20,29		/* insert top 10 bits of address */
 	lwz	r2,0(r2)		/* get pmd entry */
-- 
2.25.0


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

* Re: [PATCH] [backport for 5.10] powerpc/603: Fix protection of user pages mapped with PROT_NONE
  2021-03-11 18:24 [PATCH] [backport for 5.10] powerpc/603: Fix protection of user pages mapped with PROT_NONE Christophe Leroy
@ 2021-03-12 10:03 ` Greg KH
  0 siblings, 0 replies; 2+ messages in thread
From: Greg KH @ 2021-03-12 10:03 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: stable, linux-kernel, linuxppc-dev

On Thu, Mar 11, 2021 at 06:24:30PM +0000, Christophe Leroy wrote:
> (cherry picked from commit c119565a15a628efdfa51352f9f6c5186e506a1c)
> 
> On book3s/32, page protection is defined by the PP bits in the PTE
> which provide the following protection depending on the access
> keys defined in the matching segment register:
> - PP 00 means RW with key 0 and N/A with key 1.
> - PP 01 means RW with key 0 and RO with key 1.
> - PP 10 means RW with both key 0 and key 1.
> - PP 11 means RO with both key 0 and key 1.
> 
> Since the implementation of kernel userspace access protection,
> PP bits have been set as follows:
> - PP00 for pages without _PAGE_USER
> - PP01 for pages with _PAGE_USER and _PAGE_RW
> - PP11 for pages with _PAGE_USER and without _PAGE_RW
> 
> For kernelspace segments, kernel accesses are performed with key 0
> and user accesses are performed with key 1. As PP00 is used for
> non _PAGE_USER pages, user can't access kernel pages not flagged
> _PAGE_USER while kernel can.
> 
> For userspace segments, both kernel and user accesses are performed
> with key 0, therefore pages not flagged _PAGE_USER are still
> accessible to the user.
> 
> This shouldn't be an issue, because userspace is expected to be
> accessible to the user. But unlike most other architectures, powerpc
> implements PROT_NONE protection by removing _PAGE_USER flag instead of
> flagging the page as not valid. This means that pages in userspace
> that are not flagged _PAGE_USER shall remain inaccessible.
> 
> To get the expected behaviour, just mimic other architectures in the
> TLB miss handler by checking _PAGE_USER permission on userspace
> accesses as if it was the _PAGE_PRESENT bit.
> 
> Note that this problem only is only for 603 cores. The 604+ have
> an hash table, and hash_page() function already implement the
> verification of _PAGE_USER permission on userspace pages.
> 
> Fixes: f342adca3afc ("powerpc/32s: Prepare Kernel Userspace Access Protection")
> Change-Id: I68bc5e5ff4542bdfcdcd12923fa96a5811707475
> Cc: stable@vger.kernel.org # v5.2+
> Reported-by: Christoph Plattner <christoph.plattner@thalesgroup.com>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Link: https://lore.kernel.org/r/4a0c6e3bb8f0c162457bf54d9bc6fd8d7b55129f.1612160907.git.christophe.leroy@csgroup.eu
> ---
>  arch/powerpc/kernel/head_book3s_32.S | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Both backports applied, thanks.

greg k-h

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

end of thread, other threads:[~2021-03-12 10:14 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-11 18:24 [PATCH] [backport for 5.10] powerpc/603: Fix protection of user pages mapped with PROT_NONE Christophe Leroy
2021-03-12 10:03 ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).