linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Michael Ellerman <mpe@ellerman.id.au>
To: linuxppc-dev@ozlabs.org
Subject: [PATCH v6 10/10] powerpc/mm: Detect bad KUAP faults
Date: Thu, 18 Apr 2019 16:51:25 +1000	[thread overview]
Message-ID: <20190418065125.2687-10-mpe@ellerman.id.au> (raw)
In-Reply-To: <20190418065125.2687-1-mpe@ellerman.id.au>

When KUAP is enabled we have logic to detect page faults that occur
outside of a valid user access region and are blocked by the AMR.

What we don't have at the moment is logic to detect a fault *within* a
valid user access region, that has been incorrectly blocked by AMR.
This is not meant to ever happen, but it can if we incorrectly
save/restore the AMR, or if the AMR was overwritten for some other
reason.

Currently if that happens we assume it's just a regular fault that
will be corrected by handling the fault normally, so we just return.
But there is nothing the fault handling code can do to fix it, so the
fault just happens again and we spin forever, leading to soft lockups.

So add some logic to detect that case and WARN() if we ever see it.
Arguably it should be a BUG(), but it's more polite to fail the access
and let the kernel continue, rather than taking down the box. There
should be no data integrity issue with failing the fault rather than
BUG'ing, as we're just going to disallow an access that should have
been allowed.

To make the code a little easier to follow, unroll the condition at
the end of bad_kernel_fault() and comment each case, before adding the
call to bad_kuap_fault().

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---

v6: Simplify bad_kuap_fault() as suggested by Christophe.
---
 .../powerpc/include/asm/book3s/64/kup-radix.h |  6 +++++
 arch/powerpc/include/asm/kup.h                |  1 +
 arch/powerpc/mm/fault.c                       | 25 ++++++++++++++++---
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h b/arch/powerpc/include/asm/book3s/64/kup-radix.h
index 6d6628424134..7679bd0c5af0 100644
--- a/arch/powerpc/include/asm/book3s/64/kup-radix.h
+++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h
@@ -95,6 +95,12 @@ static inline void prevent_user_access(void __user *to, const void __user *from,
 	set_kuap(AMR_KUAP_BLOCKED);
 }
 
+static inline bool bad_kuap_fault(struct pt_regs *regs, bool is_write)
+{
+	return WARN(mmu_has_feature(MMU_FTR_RADIX_KUAP) &&
+		    (regs->kuap & (is_write ? AMR_KUAP_BLOCK_WRITE : AMR_KUAP_BLOCK_READ)),
+		    "Bug: %s fault blocked by AMR!", is_write ? "Write" : "Read");
+}
 #endif /* CONFIG_PPC_KUAP */
 
 #endif /* __ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
index d7312defbe1c..28ad4654eed2 100644
--- a/arch/powerpc/include/asm/kup.h
+++ b/arch/powerpc/include/asm/kup.h
@@ -26,6 +26,7 @@ static inline void allow_user_access(void __user *to, const void __user *from,
 				     unsigned long size) { }
 static inline void prevent_user_access(void __user *to, const void __user *from,
 				       unsigned long size) { }
+static inline bool bad_kuap_fault(struct pt_regs *regs, bool is_write) { return false; }
 #endif /* CONFIG_PPC_KUAP */
 
 static inline void allow_read_from_user(const void __user *from, unsigned long size)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 463d1e9d026e..b5d3578d9f65 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -44,6 +44,7 @@
 #include <asm/mmu_context.h>
 #include <asm/siginfo.h>
 #include <asm/debug.h>
+#include <asm/kup.h>
 
 static inline bool notify_page_fault(struct pt_regs *regs)
 {
@@ -224,7 +225,7 @@ static int mm_fault_error(struct pt_regs *regs, unsigned long addr,
 
 /* Is this a bad kernel fault ? */
 static bool bad_kernel_fault(struct pt_regs *regs, unsigned long error_code,
-			     unsigned long address)
+			     unsigned long address, bool is_write)
 {
 	int is_exec = TRAP(regs) == 0x400;
 
@@ -235,6 +236,9 @@ static bool bad_kernel_fault(struct pt_regs *regs, unsigned long error_code,
 				    address >= TASK_SIZE ? "exec-protected" : "user",
 				    address,
 				    from_kuid(&init_user_ns, current_uid()));
+
+		// Kernel exec fault is always bad
+		return true;
 	}
 
 	if (!is_exec && address < TASK_SIZE && (error_code & DSISR_PROTFAULT) &&
@@ -244,7 +248,22 @@ static bool bad_kernel_fault(struct pt_regs *regs, unsigned long error_code,
 				    from_kuid(&init_user_ns, current_uid()));
 	}
 
-	return is_exec || (address >= TASK_SIZE) || !search_exception_tables(regs->nip);
+	// Kernel fault on kernel address is bad
+	if (address >= TASK_SIZE)
+		return true;
+
+	// Fault on user outside of certain regions (eg. copy_tofrom_user()) is bad
+	if (!search_exception_tables(regs->nip))
+		return true;
+
+	// Read/write fault in a valid region (the exception table search passed
+	// above), but blocked by KUAP is bad, it can never succeed.
+	if (bad_kuap_fault(regs, is_write))
+		return true;
+
+	// What's left? Kernel fault on user in well defined regions (extable
+	// matched), and allowed by KUAP in the faulting context.
+	return false;
 }
 
 static bool bad_stack_expansion(struct pt_regs *regs, unsigned long address,
@@ -467,7 +486,7 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
 	 * take a page fault to a kernel address or a page fault to a user
 	 * address outside of dedicated places
 	 */
-	if (unlikely(!is_user && bad_kernel_fault(regs, error_code, address)))
+	if (unlikely(!is_user && bad_kernel_fault(regs, error_code, address, is_write)))
 		return SIGSEGV;
 
 	/*
-- 
2.20.1


  parent reply	other threads:[~2019-04-18  7:06 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-18  6:51 [PATCH v6 01/10] powerpc/powernv/idle: Restore IAMR after idle Michael Ellerman
2019-04-18  6:51 ` [PATCH v6 02/10] powerpc/powernv/idle: Restore AMR/UAMOR/AMOR " Michael Ellerman
2019-04-18  6:51 ` [PATCH v6 03/10] powerpc: Add framework for Kernel Userspace Protection Michael Ellerman
2019-04-18  6:51 ` [PATCH v6 04/10] powerpc: Add skeleton for Kernel Userspace Execution Prevention Michael Ellerman
2019-04-18  6:51 ` [PATCH v6 05/10] powerpc: Add a framework for Kernel Userspace Access Protection Michael Ellerman
2019-04-18  6:51 ` [PATCH v6 06/10] powerpc/64: Setup KUP on secondary CPUs Michael Ellerman
2019-04-24  6:03   ` Christophe Leroy
2019-04-18  6:51 ` [PATCH v6 07/10] powerpc/mm/radix: Use KUEP API for Radix MMU Michael Ellerman
2019-04-18  6:51 ` [PATCH v6 08/10] powerpc/lib: Refactor __patch_instruction() to use __put_user_asm() Michael Ellerman
2019-04-18  6:51 ` [PATCH v6 09/10] powerpc/64s: Implement KUAP for Radix MMU Michael Ellerman
2020-04-17 10:10   ` Christophe Leroy
2020-04-17 10:39     ` Nicholas Piggin
2020-04-17 12:02       ` Christophe Leroy
2019-04-18  6:51 ` Michael Ellerman [this message]
2019-04-21 14:19 ` [v6,01/10] powerpc/powernv/idle: Restore IAMR after idle Michael Ellerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190418065125.2687-10-mpe@ellerman.id.au \
    --to=mpe@ellerman.id.au \
    --cc=linuxppc-dev@ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).