x86: fault_{32|64}.c unify do_page_fault

Begin to unify do_page_fault(), easy code movement first.

Signed-off-by Harvey Harrison <[email protected]>

---
Ingo, similar to the kprobes unification patches I did, it gets a bit uglier before it gets better ;-)

 arch/x86/mm/fault_32.c |   38 +++++++++++++++++++++++++++++---------
 arch/x86/mm/fault_64.c |   23 ++++++++++++++++++-----
 2 files changed, 47 insertions(+), 14 deletions(-)

arch/x86/mm/fault_32.c +0 -0

index b1893eb..051a4ec 100644

arch/x86/mm/fault_32.c +1 -0

+	int si_code;
 
 	/*
 	 * We can fault from pretty much anywhere, with unknown IRQ state.
 	 */
 	trace_hardirqs_fixup();

arch/x86/mm/fault_32.c +9 -1

+
+	/* get the address */
+	address = read_cr2();
 	si_code = SEGV_MAPERR;
 
 	/*

@@ -403,9 +410,24 @@ void __kprobes do_page_fault(struct pt_regs *regs, unsigned long error_code)
 	 * (error_code & 4) == 0, and that the fault was not a
 	 * protection error (error_code & 9) == 0.
 	 */
+#ifdef CONFIG_X86_32
 	if (unlikely(address >= TASK_SIZE)) {
-		if (!(error_code & 0x0000000d) && vmalloc_fault(address) >= 0)
+		if (!(error_code & (PF_RSVD|PF_USER|PF_PROT)) &&
+		    vmalloc_fault(address) >= 0)
 			return;
+#else
+	if (unlikely(address >= TASK_SIZE64)) {
+		/*

arch/x86/mm/fault_32.c +1 -3

 		if (notify_page_fault(regs))
 			return;
 		/*

@@ -423,8 +445,6 @@ void __kprobes do_page_fault(struct pt_regs *regs, unsigned long error_code)
 	if (regs->flags & (X86_EFLAGS_IF|VM_MASK))
 		local_irq_enable();
 
-	mm = tsk->mm;
-
 	/*
 	 * If we're in an interrupt, have no user context or are running in an
 	 * atomic region then we must not take the fault.

@@ -495,7 +515,7 @@ good_area:
 			goto bad_area;
 	}
 
- survive:
+survive:
 	/*
 	 * If for any reason at all we couldn't handle the fault,
 	 * make sure we exit gracefully rather than endlessly redo

arch/x86/mm/fault_64.c +0 -0

index 357a3e0..97b92b6 100644

arch/x86/mm/fault_64.c +6 -2

 	/* get the address */
 	address = read_cr2();
-
 	si_code = SEGV_MAPERR;
 
-
 	/*
 	 * We fault-in kernel-space virtual memory on-demand. The
 	 * 'reference' page table is init_mm.pgd.

@@ -457,6 +460,12 @@ asmlinkage void __kprobes do_page_fault(struct pt_regs *regs,
 	 * (error_code & 4) == 0, and that the fault was not a
 	 * protection error (error_code & 9) == 0.
 	 */
+#ifdef CONFIG_X86_32
+	if (unlikely(address >= TASK_SIZE)) {
+		if (!(error_code & (PF_RSVD|PF_USER|PF_PROT)) &&
+		    vmalloc_fault(address) >= 0)
+			return;
+#else
 	if (unlikely(address >= TASK_SIZE64)) {
 		/*
 		 * Don't check for the module range here: its PML4

arch/x86/mm/fault_64.c +6 -3

 		if (notify_page_fault(regs))
 			return;
 		/*

@@ -500,7 +510,7 @@ asmlinkage void __kprobes do_page_fault(struct pt_regs *regs,
 	if (user_mode_vm(regs))
 		error_code |= PF_USER;
 
- again:
+again:
 	/* When running in the kernel we expect faults to occur only to
 	 * addresses in user space.  All other faults represent errors in the
 	 * kernel and should generate an OOPS.  Unfortunately, in the case of an

@@ -531,8 +541,11 @@ asmlinkage void __kprobes do_page_fault(struct pt_regs *regs,
 	if (!(vma->vm_flags & VM_GROWSDOWN))
 		goto bad_area;
 	if (error_code & PF_USER) {
-		/* Allow userspace just enough access below the stack pointer
-		 * to let the 'enter' instruction work.
+		/*
+		 * Accessing the stack below %sp is always a bug.
+		 * The large cushion allows instructions like enter
+		 * and pusha to work.  ("enter $65535,$31" pushes
+		 * 32 pointers and then decrements %sp by 65535.)
 		 */
 		if (address + 65536 + 32 * sizeof(unsigned long) < regs->sp)
 			goto bad_area;

Quoted 11 lines · Harvey Harrison

Begin to unify do_page_fault(), easy code movement first.

Signed-off-by Harvey Harrison <[email protected]>

---
Ingo, similar to the kprobes unification patches I did, it gets a bit uglier before it gets better ;-)

 arch/x86/mm/fault_32.c |   38 +++++++++++++++++++++++++++++---------
 arch/x86/mm/fault_64.c |   23 ++++++++++++++++++-----
 2 files changed, 47 insertions(+), 14 deletions(-)

arch/x86/mm/fault_32.c void __kprobes do_page_fault(struct pt_regs *regs, unsigned long error_code) · line 379

Quoted 12 lines

arch/x86/mm/fault_32.c +2 -2

index b1893eb..051a4ec 100644
@@ -375,19 +375,26 @@ void __kprobes do_page_fault(struct pt_regs *regs, unsigned long error_code)
 	struct mm_struct *mm;
 	struct vm_area_struct *vma;
 	unsigned long address;
-	int write, si_code;
-	int fault;
+	int write, fault;
+#ifdef CONFIG_x86_64

There is no such thing as CONFIG_x86_64 .

Do you seriously think code is getting better and more readable because of this liberal #ifdef sprinkling in every possible direction?

Quoted 1 line

There is no such thing as CONFIG_x86_64 .

My apologies, testing/compiling on X86_32 here.

My apologies, testing/compiling on X86_32 here.

Quoted 3 lines

Do you seriously think code is getting better and more readable because of this liberal #ifdef sprinkling in every possible direction?

Well, this of course is not the end of the road, but it makes it obvious where the differences between 32/64 bit lie and allows further cleanups to unify these areas over time. This is meant as a no functionality change path at first.....and it does point out that for the most part the files are _very_ similar to each other.

So my plan for now was to move forward with no functional changes and esentially ifdef or reorder code until we get to identical fault_32/64.c which then gets moved to a single fault.c

Then the cleanups happen in one place in one file and it should be easy to audit the series at the end. But for further patches I'll wait until the series is further along and tested before submitting. This was how the kprobes unification went and I think it works fairly well this way.