[PATCH AUTOSEL 4.19 049/167] ARC: show_regs: lockdep: re-enable preemption

[PATCH AUTOSEL 4.19 049/167] ARC: show_regs: lockdep: re-enable preemption

[Sasha Levin]

From: Vineet Gupta <vgupta@synopsys.com>

[ Upstream commit f731a8e89f8c78985707c626680f3e24c7a60772 ]

signal handling core calls show_regs() with preemption disabled which
on ARC takes mmap_sem for mm/vma access, causing lockdep splat.

| [ARCLinux]# ./segv-null-ptr
| potentially unexpected fatal signal 11.
| BUG: sleeping function called from invalid context at kernel/fork.c:1011
| in_atomic(): 1, irqs_disabled(): 0, pid: 70, name: segv-null-ptr
| no locks held by segv-null-ptr/70.
| CPU: 0 PID: 70 Comm: segv-null-ptr Not tainted 4.18.0+ #69
|
| Stack Trace:
|  arc_unwind_core+0xcc/0x100
|  ___might_sleep+0x17a/0x190
|  mmput+0x16/0xb8
|  show_regs+0x52/0x310
|  get_signal+0x5ee/0x610
|  do_signal+0x2c/0x218
|  resume_user_mode_begin+0x90/0xd8

Workaround by re-enabling preemption temporarily.

Note that the preemption disabling in core code around show_regs()
was introduced by commit 3a9f84d354ce ("signals, debug: fix BUG: using
smp_processor_id() in preemptible code in print_fatal_signal()")

to silence a differnt lockdep seen on x86 bakc in 2009.

Cc: <stable at vger.kernel.org>
Signed-off-by: Vineet Gupta <vgupta at synopsys.com>
Signed-off-by: Sasha Levin <sashal at kernel.org>
---
 arch/arc/kernel/troubleshoot.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arc/kernel/troubleshoot.c b/arch/arc/kernel/troubleshoot.c
index 5c6663321e873..215f515442e03 100644
--- a/arch/arc/kernel/troubleshoot.c
+++ b/arch/arc/kernel/troubleshoot.c
@@ -179,6 +179,12 @@ void show_regs(struct pt_regs *regs)
 	struct task_struct *tsk = current;
 	struct callee_regs *cregs;
 
+	/*
+	 * generic code calls us with preemption disabled, but some calls
+	 * here could sleep, so re-enable to avoid lockdep splat
+	 */
+	preempt_enable();
+
 	print_task_path_n_nm(tsk);
 	show_regs_print_info(KERN_INFO);
 
@@ -221,6 +227,8 @@ void show_regs(struct pt_regs *regs)
 	cregs = (struct callee_regs *)current->thread.callee_reg;
 	if (cregs)
 		show_callee_regs(cregs);
+
+	preempt_disable();
 }
 
 void show_kernel_fault_diag(const char *str, struct pt_regs *regs,
-- 
2.20.1

[PATCH AUTOSEL 4.19 050/167] ARC: mm: do_page_fault fixes #1: relinquish mmap_sem if signal arrives while handle_mm_fault

[Sasha Levin]

From: Vineet Gupta <vgupta@synopsys.com>

[ Upstream commit 4d447455e73b47c43dd35fcc38ed823d3182a474 ]

do_page_fault() forgot to relinquish mmap_sem if a signal came while
handling handle_mm_fault() - due to say a ctl+c or oom etc.
This would later cause a deadlock by acquiring it twice.

This came to light when running libc testsuite tst-tls3-malloc test but
is likely also the cause for prior seen LTP failures. Using lockdep
clearly showed what the issue was.

| # while true; do ./tst-tls3-malloc ; done
| Didn't expect signal from child: got `Segmentation fault'
| ^C
| ============================================
| WARNING: possible recursive locking detected
| 4.17.0+ #25 Not tainted
| --------------------------------------------
| tst-tls3-malloc/510 is trying to acquire lock:
| 606c7728 (&mm->mmap_sem){++++}, at: __might_fault+0x28/0x5c
|
|but task is already holding lock:
|606c7728 (&mm->mmap_sem){++++}, at: do_page_fault+0x9c/0x2a0
|
| other info that might help us debug this:
|  Possible unsafe locking scenario:
|
|       CPU0
|       ----
|  lock(&mm->mmap_sem);
|  lock(&mm->mmap_sem);
|
| *** DEADLOCK ***
|

------------------------------------------------------------
What the change does is not obvious (note to myself)

prior code was

| do_page_fault
|
|   down_read()		<-- lock taken
|   handle_mm_fault	<-- signal pending as this runs
|   if fatal_signal_pending
|       if VM_FAULT_ERROR
|           up_read
|       if user_mode
|          return	<-- lock still held, this was the BUG

New code

| do_page_fault
|
|   down_read()		<-- lock taken
|   handle_mm_fault	<-- signal pending as this runs
|   if fatal_signal_pending
|       if VM_FAULT_RETRY
|          return       <-- not same case as above, but still OK since
|                           core mm already relinq lock for FAULT_RETRY
|    ...
|
|   < Now falls through for bug case above >
|
|   up_read()		<-- lock relinquished

Cc: stable at vger.kernel.org
Signed-off-by: Vineet Gupta <vgupta at synopsys.com>
Signed-off-by: Sasha Levin <sashal at kernel.org>
---
 arch/arc/mm/fault.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c
index db6913094be3c..f28db0b112a30 100644
--- a/arch/arc/mm/fault.c
+++ b/arch/arc/mm/fault.c
@@ -143,12 +143,17 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
 	 */
 	fault = handle_mm_fault(vma, address, flags);
 
-	/* If Pagefault was interrupted by SIGKILL, exit page fault "early" */
 	if (unlikely(fatal_signal_pending(current))) {
-		if ((fault & VM_FAULT_ERROR) && !(fault & VM_FAULT_RETRY))
-			up_read(&mm->mmap_sem);
-		if (user_mode(regs))
+
+		/*
+		 * if fault retry, mmap_sem already relinquished by core mm
+		 * so OK to return to user mode (with signal handled first)
+		 */
+		if (fault & VM_FAULT_RETRY) {
+			if (!user_mode(regs))
+				goto no_context;
 			return;
+		}
 	}
 
 	perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address);
-- 
2.20.1

[PATCH AUTOSEL 4.19 111/167] signal/arc: Use force_sig_fault where appropriate

[Sasha Levin]

From: "Eric W. Biederman" <ebiederm@xmission.com>

[ Upstream commit 15773ae938d8d93d982461990bebad6e1d7a1830 ]

Acked-by: Vineet Gupta <vgupta at synopsys.com>
Signed-off-by: "Eric W. Biederman" <ebiederm at xmission.com>
Signed-off-by: Sasha Levin <sashal at kernel.org>
---
 arch/arc/mm/fault.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c
index f28db0b112a30..a0366f9dca051 100644
--- a/arch/arc/mm/fault.c
+++ b/arch/arc/mm/fault.c
@@ -66,14 +66,12 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
 	struct vm_area_struct *vma = NULL;
 	struct task_struct *tsk = current;
 	struct mm_struct *mm = tsk->mm;
-	siginfo_t info;
+	int si_code;
 	int ret;
 	vm_fault_t fault;
 	int write = regs->ecr_cause & ECR_C_PROTV_STORE;  /* ST/EX */
 	unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
 
-	clear_siginfo(&info);
-
 	/*
 	 * We fault-in kernel-space virtual memory on-demand. The
 	 * 'reference' page table is init_mm.pgd.
@@ -91,7 +89,7 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
 			return;
 	}
 
-	info.si_code = SEGV_MAPERR;
+	si_code = SEGV_MAPERR;
 
 	/*
 	 * If we're in an interrupt or have no user
@@ -119,7 +117,7 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
 	 * we can handle it..
 	 */
 good_area:
-	info.si_code = SEGV_ACCERR;
+	si_code = SEGV_ACCERR;
 
 	/* Handle protection violation, execute on heap or stack */
 
@@ -204,11 +202,7 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
 	/* User mode accesses just cause a SIGSEGV */
 	if (user_mode(regs)) {
 		tsk->thread.fault_address = address;
-		info.si_signo = SIGSEGV;
-		info.si_errno = 0;
-		/* info.si_code has been set above */
-		info.si_addr = (void __user *)address;
-		force_sig_info(SIGSEGV, &info, tsk);
+		force_sig_fault(SIGSEGV, si_code, (void __user *)address, tsk);
 		return;
 	}
 
@@ -243,9 +237,5 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
 		goto no_context;
 
 	tsk->thread.fault_address = address;
-	info.si_signo = SIGBUS;
-	info.si_errno = 0;
-	info.si_code = BUS_ADRERR;
-	info.si_addr = (void __user *)address;
-	force_sig_info(SIGBUS, &info, tsk);
+	force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *)address, tsk);
 }
-- 
2.20.1

[PATCH AUTOSEL 4.19 112/167] ARC: mm: fix uninitialised signal code in do_page_fault

[Sasha Levin]

From: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>

[ Upstream commit 121e38e5acdc8e1e4cdb750fcdcc72f94e420968 ]

Commit 15773ae938d8 ("signal/arc: Use force_sig_fault where
appropriate") introduced undefined behaviour by leaving si_code
unitiailized and leaking random kernel values to user space.

Fixes: 15773ae938d8 ("signal/arc: Use force_sig_fault where appropriate")
Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev at synopsys.com>
Signed-off-by: Vineet Gupta <vgupta at synopsys.com>
Signed-off-by: Sasha Levin <sashal at kernel.org>
---
 arch/arc/mm/fault.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c
index a0366f9dca051..535cf18e8bf2c 100644
--- a/arch/arc/mm/fault.c
+++ b/arch/arc/mm/fault.c
@@ -66,7 +66,7 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
 	struct vm_area_struct *vma = NULL;
 	struct task_struct *tsk = current;
 	struct mm_struct *mm = tsk->mm;
-	int si_code;
+	int si_code = 0;
 	int ret;
 	vm_fault_t fault;
 	int write = regs->ecr_cause & ECR_C_PROTV_STORE;  /* ST/EX */
-- 
2.20.1

[PATCH AUTOSEL 4.19 113/167] ARC: mm: SIGSEGV userspace trying to access kernel virtual memory

[Sasha Levin]

From: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>

[ Upstream commit a8c715b4dd73c26a81a9cc8dc792aa715d8b4bb2 ]

As of today if userspace process tries to access a kernel virtual addres
(0x7000_0000 to 0x7ffff_ffff) such that a legit kernel mapping already
exists, that process hangs instead of being killed with SIGSEGV

Fix that by ensuring that do_page_fault() handles kenrel vaddr only if
in kernel mode.

And given this, we can also simplify the code a bit. Now a vmalloc fault
implies kernel mode so its failure (for some reason) can reuse the
@no_context label and we can remove @bad_area_nosemaphore.

Reproduce user test for original problem:

------------------------>8-----------------
 #include <stdlib.h>
 #include <stdint.h>

 int main(int argc, char *argv[])
 {
 	volatile uint32_t temp;

 	temp = *(uint32_t *)(0x70000000);
 }
------------------------>8-----------------

Cc: <stable at vger.kernel.org>
Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev at synopsys.com>
Signed-off-by: Vineet Gupta <vgupta at synopsys.com>
Signed-off-by: Sasha Levin <sashal at kernel.org>
---
 arch/arc/mm/fault.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c
index 535cf18e8bf2c..4e8143de32e70 100644
--- a/arch/arc/mm/fault.c
+++ b/arch/arc/mm/fault.c
@@ -66,7 +66,7 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
 	struct vm_area_struct *vma = NULL;
 	struct task_struct *tsk = current;
 	struct mm_struct *mm = tsk->mm;
-	int si_code = 0;
+	int si_code = SEGV_MAPERR;
 	int ret;
 	vm_fault_t fault;
 	int write = regs->ecr_cause & ECR_C_PROTV_STORE;  /* ST/EX */
@@ -81,16 +81,14 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
 	 * only copy the information from the master page table,
 	 * nothing more.
 	 */
-	if (address >= VMALLOC_START) {
+	if (address >= VMALLOC_START && !user_mode(regs)) {
 		ret = handle_kernel_vaddr_fault(address);
 		if (unlikely(ret))
-			goto bad_area_nosemaphore;
+			goto no_context;
 		else
 			return;
 	}
 
-	si_code = SEGV_MAPERR;
-
 	/*
 	 * If we're in an interrupt or have no user
 	 * context, we must not take the fault..
@@ -198,7 +196,6 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
 bad_area:
 	up_read(&mm->mmap_sem);
 
-bad_area_nosemaphore:
 	/* User mode accesses just cause a SIGSEGV */
 	if (user_mode(regs)) {
 		tsk->thread.fault_address = address;
-- 
2.20.1

[PATCH AUTOSEL 4.19 111/167] signal/arc: Use force_sig_fault where appropriate

[Eric W. Biederman]

Sasha Levin <sashal at kernel.org> writes:

> From: "Eric W. Biederman" <ebiederm at xmission.com>
>
> [ Upstream commit 15773ae938d8d93d982461990bebad6e1d7a1830 ]

To the best of my knowledge this is just a clean up, no changes in
behavior are present.

The only reason I can see to backport this is so that later fixes could
be applied cleanly.

So while I have no objections to this patch being backported I don't see
why you would want to either.

> Acked-by: Vineet Gupta <vgupta at synopsys.com>
> Signed-off-by: "Eric W. Biederman" <ebiederm at xmission.com>
> Signed-off-by: Sasha Levin <sashal at kernel.org>
> ---
>  arch/arc/mm/fault.c | 20 +++++---------------
>  1 file changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c
> index f28db0b112a30..a0366f9dca051 100644
> --- a/arch/arc/mm/fault.c
> +++ b/arch/arc/mm/fault.c
> @@ -66,14 +66,12 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
>  	struct vm_area_struct *vma = NULL;
>  	struct task_struct *tsk = current;
>  	struct mm_struct *mm = tsk->mm;
> -	siginfo_t info;
> +	int si_code;
>  	int ret;
>  	vm_fault_t fault;
>  	int write = regs->ecr_cause & ECR_C_PROTV_STORE;  /* ST/EX */
>  	unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
>  
> -	clear_siginfo(&info);
> -
>  	/*
>  	 * We fault-in kernel-space virtual memory on-demand. The
>  	 * 'reference' page table is init_mm.pgd.
> @@ -91,7 +89,7 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
>  			return;
>  	}
>  
> -	info.si_code = SEGV_MAPERR;
> +	si_code = SEGV_MAPERR;
>  
>  	/*
>  	 * If we're in an interrupt or have no user
> @@ -119,7 +117,7 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
>  	 * we can handle it..
>  	 */
>  good_area:
> -	info.si_code = SEGV_ACCERR;
> +	si_code = SEGV_ACCERR;
>  
>  	/* Handle protection violation, execute on heap or stack */
>  
> @@ -204,11 +202,7 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
>  	/* User mode accesses just cause a SIGSEGV */
>  	if (user_mode(regs)) {
>  		tsk->thread.fault_address = address;
> -		info.si_signo = SIGSEGV;
> -		info.si_errno = 0;
> -		/* info.si_code has been set above */
> -		info.si_addr = (void __user *)address;
> -		force_sig_info(SIGSEGV, &info, tsk);
> +		force_sig_fault(SIGSEGV, si_code, (void __user *)address, tsk);
>  		return;
>  	}
>  
> @@ -243,9 +237,5 @@ void do_page_fault(unsigned long address, struct pt_regs *regs)
>  		goto no_context;
>  
>  	tsk->thread.fault_address = address;
> -	info.si_signo = SIGBUS;
> -	info.si_errno = 0;
> -	info.si_code = BUS_ADRERR;
> -	info.si_addr = (void __user *)address;
> -	force_sig_info(SIGBUS, &info, tsk);
> +	force_sig_fault(SIGBUS, BUS_ADRERR, (void __user *)address, tsk);
>  }

[PATCH AUTOSEL 4.19 111/167] signal/arc: Use force_sig_fault where appropriate

[Sasha Levin]

On Tue, Sep 03, 2019@11:49:16AM -0500, Eric W. Biederman wrote:
>Sasha Levin <sashal at kernel.org> writes:
>
>> From: "Eric W. Biederman" <ebiederm at xmission.com>
>>
>> [ Upstream commit 15773ae938d8d93d982461990bebad6e1d7a1830 ]
>
>To the best of my knowledge this is just a clean up, no changes in
>behavior are present.
>
>The only reason I can see to backport this is so that later fixes could
>be applied cleanly.
>
>So while I have no objections to this patch being backported I don't see
>why you would want to either.

This patch along with the next one came in as a dependency for
a8c715b4dd73c ("ARC: mm: SIGSEGV userspace trying to access kernel
virtual memory").

--
Thanks,
Sasha

[PATCH AUTOSEL 4.19 111/167] signal/arc: Use force_sig_fault where appropriate

[Eric W. Biederman]

Sasha Levin <sashal at kernel.org> writes:

> On Tue, Sep 03, 2019@11:49:16AM -0500, Eric W. Biederman wrote:
>>Sasha Levin <sashal at kernel.org> writes:
>>
>>> From: "Eric W. Biederman" <ebiederm at xmission.com>
>>>
>>> [ Upstream commit 15773ae938d8d93d982461990bebad6e1d7a1830 ]
>>
>>To the best of my knowledge this is just a clean up, no changes in
>>behavior are present.
>>
>>The only reason I can see to backport this is so that later fixes could
>>be applied cleanly.
>>
>>So while I have no objections to this patch being backported I don't see
>>why you would want to either.
>
> This patch along with the next one came in as a dependency for
> a8c715b4dd73c ("ARC: mm: SIGSEGV userspace trying to access kernel
> virtual memory").

Thanks for providing the rest of the context.

That looks like a perfect reason for backporting this patch.

Eric