[PATCH 3.14.x-rt] ARM: enable irq in translation/section permission fault handlers

[PATCH 3.14.x-rt] ARM: enable irq in translation/section permission fault handlers

[Yadi Hu]

[PATCH 3.14.x-rt] ARM: enable irq in translation/section permission fault handlers

_might_sleep() is used to check whether current context is atomic 
in preempt-rt kernel. According to discussion on http://www.spinics.net/
lists/linux-rt-users/msg11398.html, this case is a bug in RT. 

local_irq_save();
    some_function();
        rt_spin_lock() --> this calls __might_sleep
local_irq_restore(); 

----
arch/arm/mm/fault.c |    6 ++++++
 1 file changed, 6 insertions(+)

[PATCH 3.14.x-rt] ARM: enable irq in translation/section permission fault handlers

[Yadi Hu]

From: Yadi.hu <yadi.hu@windriver.com>

Probably happens on all ARM, with
CONFIG_PREEMPT_RT_FULL
CONFIG_DEBUG_ATOMIC_SLEEP

This simple program....

int main() {
   *((char*)0xc0001000) = 0;
};

[ 512.742724] BUG: sleeping function called from invalid context at kernel/rtmutex.c:658
[ 512.743000] in_atomic(): 0, irqs_disabled(): 128, pid: 994, name: a
[ 512.743217] INFO: lockdep is turned off.
[ 512.743360] irq event stamp: 0
[ 512.743482] hardirqs last enabled at (0): [< (null)>] (null)
[ 512.743714] hardirqs last disabled at (0): [<c0426370>] copy_process+0x3b0/0x11c0
[ 512.744013] softirqs last enabled at (0): [<c0426370>] copy_process+0x3b0/0x11c0
[ 512.744303] softirqs last disabled at (0): [< (null)>] (null)
[ 512.744631] [<c041872c>] (unwind_backtrace+0x0/0x104)
[ 512.745001] [<c09af0c4>] (dump_stack+0x20/0x24)
[ 512.745355] [<c0462490>] (__might_sleep+0x1dc/0x1e0)
[ 512.745717] [<c09b6770>] (rt_spin_lock+0x34/0x6c)
[ 512.746073] [<c0441bf0>] (do_force_sig_info+0x34/0xf0)
[ 512.746457] [<c0442668>] (force_sig_info+0x18/0x1c)
[ 512.746829] [<c041d880>] (__do_user_fault+0x9c/0xd8)
[ 512.747185] [<c041d938>] (do_bad_area+0x7c/0x94)
[ 512.747536] [<c041d990>] (do_sect_fault+0x40/0x48)
[ 512.747898] [<c040841c>] (do_DataAbort+0x40/0xa0)
[ 512.748181] Exception stack(0xecaa1fb0 to 0xecaa1ff8)

Oxc0000000 belongs to kernel address space, user task can not be
allowed to access it. For above condition, correct result is that
test case should receive a “segment fault” and exits but not stacks.

the root cause is commit 02fe2845d6a8 ("avoid enabling interrupts in
prefetch/data abort handlers"),it deletes irq enable block in Data
abort assemble code and move them into page/breakpiont/alignment fault
handlers instead. But author does not enable irq in translation/section
permission fault handlers. ARM disables irq when it enters exception/
interrupt mode, if kernel doesn't enable irq, it would be still disabled
during translation/section permission fault.

We see the above splat because do_force_sig_info is still called with
IRQs off, and that code eventually does a:

        spin_lock_irqsave(&t->sighand->siglock, flags);

As this is architecture independent code, and we've not seen any other
need for other arch to have the siglock converted to raw lock, we can
conclude that we should enable irq for ARM translation/section
permission exception.

Signed-off-by: Yadi.hu <yadi.hu@windriver.com>
---
 arch/arm/mm/fault.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 5dbf13f..272519f 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -428,6 +428,9 @@ do_translation_fault(unsigned long addr, unsigned int fsr,
 	if (addr < TASK_SIZE)
 		return do_page_fault(addr, fsr, regs);
 
+	if (interrupts_enabled(regs))
+		local_irq_enable();
+
 	if (user_mode(regs))
 		goto bad_area;
 
@@ -494,6 +497,9 @@ do_translation_fault(unsigned long addr, unsigned int fsr,
 static int
 do_sect_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 {
+	if (interrupts_enabled(regs))
+		local_irq_enable();
+
 	do_bad_area(addr, fsr, regs);
 	return 0;
 }
-- 
1.7.3.5

--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3.14.x-rt] ARM: enable irq in translation/section permission fault handlers

[yhu2]

Any comments would be appreciated!@

On 12/10/2014 10:32 AM, Yadi Hu wrote:
> From: Yadi.hu <yadi.hu@windriver.com>
>
> Probably happens on all ARM, with
> CONFIG_PREEMPT_RT_FULL
> CONFIG_DEBUG_ATOMIC_SLEEP
>
> This simple program....
>
> int main() {
>     *((char*)0xc0001000) = 0;
> };
>
> [ 512.742724] BUG: sleeping function called from invalid context at kernel/rtmutex.c:658
> [ 512.743000] in_atomic(): 0, irqs_disabled(): 128, pid: 994, name: a
> [ 512.743217] INFO: lockdep is turned off.
> [ 512.743360] irq event stamp: 0
> [ 512.743482] hardirqs last enabled at (0): [< (null)>] (null)
> [ 512.743714] hardirqs last disabled at (0): [<c0426370>] copy_process+0x3b0/0x11c0
> [ 512.744013] softirqs last enabled at (0): [<c0426370>] copy_process+0x3b0/0x11c0
> [ 512.744303] softirqs last disabled at (0): [< (null)>] (null)
> [ 512.744631] [<c041872c>] (unwind_backtrace+0x0/0x104)
> [ 512.745001] [<c09af0c4>] (dump_stack+0x20/0x24)
> [ 512.745355] [<c0462490>] (__might_sleep+0x1dc/0x1e0)
> [ 512.745717] [<c09b6770>] (rt_spin_lock+0x34/0x6c)
> [ 512.746073] [<c0441bf0>] (do_force_sig_info+0x34/0xf0)
> [ 512.746457] [<c0442668>] (force_sig_info+0x18/0x1c)
> [ 512.746829] [<c041d880>] (__do_user_fault+0x9c/0xd8)
> [ 512.747185] [<c041d938>] (do_bad_area+0x7c/0x94)
> [ 512.747536] [<c041d990>] (do_sect_fault+0x40/0x48)
> [ 512.747898] [<c040841c>] (do_DataAbort+0x40/0xa0)
> [ 512.748181] Exception stack(0xecaa1fb0 to 0xecaa1ff8)
>
> Oxc0000000 belongs to kernel address space, user task can not be
> allowed to access it. For above condition, correct result is that
> test case should receive a “segment fault” and exits but not stacks.
>
> the root cause is commit 02fe2845d6a8 ("avoid enabling interrupts in
> prefetch/data abort handlers"),it deletes irq enable block in Data
> abort assemble code and move them into page/breakpiont/alignment fault
> handlers instead. But author does not enable irq in translation/section
> permission fault handlers. ARM disables irq when it enters exception/
> interrupt mode, if kernel doesn't enable irq, it would be still disabled
> during translation/section permission fault.
>
> We see the above splat because do_force_sig_info is still called with
> IRQs off, and that code eventually does a:
>
>          spin_lock_irqsave(&t->sighand->siglock, flags);
>
> As this is architecture independent code, and we've not seen any other
> need for other arch to have the siglock converted to raw lock, we can
> conclude that we should enable irq for ARM translation/section
> permission exception.
>
> Signed-off-by: Yadi.hu <yadi.hu@windriver.com>
> ---
>   arch/arm/mm/fault.c |    6 ++++++
>   1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> index 5dbf13f..272519f 100644
> --- a/arch/arm/mm/fault.c
> +++ b/arch/arm/mm/fault.c
> @@ -428,6 +428,9 @@ do_translation_fault(unsigned long addr, unsigned int fsr,
>   	if (addr < TASK_SIZE)
>   		return do_page_fault(addr, fsr, regs);
>   
> +	if (interrupts_enabled(regs))
> +		local_irq_enable();
> +
>   	if (user_mode(regs))
>   		goto bad_area;
>   
> @@ -494,6 +497,9 @@ do_translation_fault(unsigned long addr, unsigned int fsr,
>   static int
>   do_sect_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
>   {
> +	if (interrupts_enabled(regs))
> +		local_irq_enable();
> +
>   	do_bad_area(addr, fsr, regs);
>   	return 0;
>   }

--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3.14.x-rt] ARM: enable irq in translation/section permission fault handlers

[Sebastian Andrzej Siewior]

* Yadi Hu | 2014-12-10 10:32:09 [+0800]:

>This simple program....
>
>int main() {
>   *((char*)0xc0001000) = 0;
>};

Thanks for the test programm and the well done description. I applied
the patch.

Sebastian