All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: linuxppc-dev@lists.ozlabs.org
Cc: aneesh.kumar@linux.vnet.ibm.com,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>
Subject: [PATCH 23/24] powerpc/mm: Cleanup check for stack expansion
Date: Wed, 19 Jul 2017 14:49:45 +1000	[thread overview]
Message-ID: <20170719044946.22030-23-benh@kernel.crashing.org> (raw)
In-Reply-To: <20170719044946.22030-1-benh@kernel.crashing.org>

When hitting below a VM_GROWSDOWN vma (typically growing the stack),
we check whether it's a valid stack-growing instruction and we
check the distance to GPR1. This is largely open coded with lots
of comments, so move it out to a helper.

While at it, make store_update_sp a boolean.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/mm/fault.c | 84 ++++++++++++++++++++++++++++---------------------
 1 file changed, 48 insertions(+), 36 deletions(-)

diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index a229fd2d82d6..c2720ebb6a62 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -71,15 +71,15 @@ static inline bool notify_page_fault(struct pt_regs *regs)
  * Check whether the instruction at regs->nip is a store using
  * an update addressing form which will update r1.
  */
-static int store_updates_sp(struct pt_regs *regs)
+static bool store_updates_sp(struct pt_regs *regs)
 {
 	unsigned int inst;
 
 	if (get_user(inst, (unsigned int __user *)regs->nip))
-		return 0;
+		return false;
 	/* check for 1 in the rA field */
 	if (((inst >> 16) & 0x1f) != 1)
-		return 0;
+		return false;
 	/* check major opcode */
 	switch (inst >> 26) {
 	case 37:	/* stwu */
@@ -87,7 +87,7 @@ static int store_updates_sp(struct pt_regs *regs)
 	case 45:	/* sthu */
 	case 53:	/* stfsu */
 	case 55:	/* stfdu */
-		return 1;
+		return true;
 	case 62:	/* std or stdu */
 		return (inst & 3) == 1;
 	case 31:
@@ -99,10 +99,10 @@ static int store_updates_sp(struct pt_regs *regs)
 		case 439:	/* sthux */
 		case 695:	/* stfsux */
 		case 759:	/* stfdux */
-			return 1;
+			return true;
 		}
 	}
-	return 0;
+	return false;
 }
 /*
  * do_page_fault error handling helpers
@@ -222,6 +222,43 @@ static bool bad_kernel_fault(bool is_exec, unsigned long error_code,
 	return is_exec || (address >= TASK_SIZE);
 }
 
+static bool bad_stack_expansion(struct pt_regs *regs, unsigned long address,
+				struct vm_area_struct *vma,
+				bool store_update_sp)
+{
+	/*
+	 * N.B. The POWER/Open ABI allows programs to access up to
+	 * 288 bytes below the stack pointer.
+	 * The kernel signal delivery code writes up to about 1.5kB
+	 * below the stack pointer (r1) before decrementing it.
+	 * The exec code can write slightly over 640kB to the stack
+	 * before setting the user r1.  Thus we allow the stack to
+	 * expand to 1MB without further checks.
+	 */
+	if (address + 0x100000 < vma->vm_end) {
+		/* get user regs even if this fault is in kernel mode */
+		struct pt_regs *uregs = current->thread.regs;
+		if (uregs == NULL)
+			return true;
+
+		/*
+		 * A user-mode access to an address a long way below
+		 * the stack pointer is only valid if the instruction
+		 * is one which would update the stack pointer to the
+		 * address accessed if the instruction completed,
+		 * i.e. either stwu rs,n(r1) or stwux rs,r1,rb
+		 * (or the byte, halfword, float or double forms).
+		 *
+		 * If we don't check this then any write to the area
+		 * between the last mapped region and the stack will
+		 * expand the stack rather than segfaulting.
+		 */
+		if (address + 2048 < uregs->gpr[1] && !store_update_sp)
+			return true;
+	}
+	return false;
+}
+
 static bool access_error(bool is_write, bool is_exec,
 			 struct vm_area_struct *vma)
 {
@@ -350,7 +387,7 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
 	int is_user = user_mode(regs);
 	int is_write = page_fault_is_write(error_code);
 	int fault, major = 0;
-	int store_update_sp = 0;
+	bool store_update_sp = false;
 
 #ifdef CONFIG_PPC_ICSWX
 	/*
@@ -458,36 +495,11 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
 	if (unlikely(!(vma->vm_flags & VM_GROWSDOWN)))
 		return bad_area(regs, address);
 
-	/*
-	 * N.B. The POWER/Open ABI allows programs to access up to
-	 * 288 bytes below the stack pointer.
-	 * The kernel signal delivery code writes up to about 1.5kB
-	 * below the stack pointer (r1) before decrementing it.
-	 * The exec code can write slightly over 640kB to the stack
-	 * before setting the user r1.  Thus we allow the stack to
-	 * expand to 1MB without further checks.
-	 */
-	if (address + 0x100000 < vma->vm_end) {
-		/* get user regs even if this fault is in kernel mode */
-		struct pt_regs *uregs = current->thread.regs;
-		if (uregs == NULL)
-			return bad_area(regs, address);
+	/* The stack is being expanded, check if it's valid */
+	if (unlikely(bad_stack_expansion(regs, address, vma, store_update_sp)))
+		return bad_area(regs, address);
 
-		/*
-		 * A user-mode access to an address a long way below
-		 * the stack pointer is only valid if the instruction
-		 * is one which would update the stack pointer to the
-		 * address accessed if the instruction completed,
-		 * i.e. either stwu rs,n(r1) or stwux rs,r1,rb
-		 * (or the byte, halfword, float or double forms).
-		 *
-		 * If we don't check this then any write to the area
-		 * between the last mapped region and the stack will
-		 * expand the stack rather than segfaulting.
-		 */
-		if (address + 2048 < uregs->gpr[1] && !store_update_sp)
-			return bad_area(regs, address);
-	}
+	/* Try to expand it */
 	if (unlikely(expand_stack(vma, address)))
 		return bad_area(regs, address);
 
-- 
2.13.3

  parent reply	other threads:[~2017-07-19  4:57 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-19  4:49 [PATCH 01/24] powerpc/mm: Move exception_enter/exit to a do_page_fault wrapper Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 02/24] powerpc/mm: Pre-filter SRR1 bits before do_page_fault() Benjamin Herrenschmidt
2017-07-22 16:43   ` LEROY Christophe
2017-07-23  1:10     ` Benjamin Herrenschmidt
2017-07-24 13:48     ` Michael Ellerman
2017-07-19  4:49 ` [PATCH 03/24] powerpc/6xx: Handle DABR match before calling do_page_fault Benjamin Herrenschmidt
2017-08-03  0:19   ` Michael Ellerman
2017-08-03  1:00     ` Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 04/24] powerpc/mm: Update definitions of DSISR bits Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 05/24] powerpc/mm: Update bits used to skip hash_page Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 06/24] powerpc/mm: Use symbolic constants for filtering SRR1 bits on ISIs Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 07/24] powerpc/mm: Move out definition of CPU specific is_write bits Benjamin Herrenschmidt
2017-07-22 16:40   ` LEROY Christophe
2017-07-23  1:06     ` Benjamin Herrenschmidt
2017-07-24 11:58     ` Michael Ellerman
2017-07-19  4:49 ` [PATCH 08/24] powerpc/mm: Move error_code checks for bad faults earlier Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 09/24] powerpc/mm: Overhaul handling of bad page faults Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 10/24] powerpc/mm: Move debugger check to notify_page_fault() Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 11/24] powerpc/mm: Simplify returns from __do_page_fault Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 12/24] powerpc/mm: Fix reporting of kernel execute faults Benjamin Herrenschmidt
2018-11-07  8:35   ` Christophe LEROY
2018-11-07 10:39     ` Benjamin Herrenschmidt
     [not found]     ` <87zhtr5d1v.fsf@linux.ibm.com>
2018-11-30  6:08       ` Christophe LEROY
2017-07-19  4:49 ` [PATCH 13/24] powerpc/mm: Make bad_area* helper functions Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 14/24] powerpc/mm: Rework mm_fault_error() Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 15/24] powerpc/mm: Move CMO accounting out of do_page_fault into a helper Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 16/24] powerpc/mm: Cosmetic fix to page fault accounting Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 17/24] powerpc/mm: Move the DSISR_PROTFAULT sanity check Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 18/24] powerpc/mm: Move/simplify faulthandler_disabled() and !mm check Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 19/24] powerpc/mm: Add a bunch of (un)likely annotations to do_page_fault Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 20/24] powerpc/mm: Set fault flags earlier Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 21/24] powerpc/mm: Move page fault VMA access checks to a helper Benjamin Herrenschmidt
2017-07-19  4:49 ` [PATCH 22/24] powerpc/mm: Don't lose "major" fault indication on retry Benjamin Herrenschmidt
2017-07-19  4:49 ` Benjamin Herrenschmidt [this message]
2017-07-21 16:59   ` [PATCH 23/24] powerpc/mm: Cleanup check for stack expansion LEROY Christophe
2017-07-24 10:47     ` Michael Ellerman
2017-07-24 17:34       ` LEROY Christophe
2017-07-25 11:19         ` Michael Ellerman
2017-07-31 11:37           ` Christophe LEROY
2017-07-19  4:49 ` [PATCH 24/24] powerpc: Remove old unused icswx based coprocessor support Benjamin Herrenschmidt
2017-08-07 10:41 ` [01/24] powerpc/mm: Move exception_enter/exit to a do_page_fault wrapper Michael Ellerman
2017-08-07 16:37   ` Christophe LEROY
2017-08-08  2:16     ` Michael Ellerman
2017-08-08  6:45       ` Christophe LEROY
2017-08-08 10:00         ` 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=20170719044946.22030-23-benh@kernel.crashing.org \
    --to=benh@kernel.crashing.org \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=linuxppc-dev@lists.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.