[Bug 1001160] New: FIQ can trash stack when interrupting IRQ

[bugzilla-daemon at bugs dot ecos dot sourceware dot org]

Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001160

           Summary: FIQ can trash stack when interrupting IRQ
           Product: eCos
           Version: 3.0
          Platform: All
        OS/Version: ARM
            Status: UNCONFIRMED
          Severity: critical
          Priority: low
         Component: HAL
        AssignedTo: unassigned@bugs.ecos.sourceware.org
        ReportedBy: ml@tctechnologies.tc
                CC: ecos-bugs@ecos.sourceware.org
             Class: Advice Request


There is a small unprotected critical section in vectors.s. The opening is
exactly one instruction in size so it is hard to reproduce unless there is a
high frequency of FIQ;s and IRQ's.
We have seen various reports about this dating as far back as 2004 but we have
seen no solutions. A solution is suggested in this bug report.

Please note that this bug is only relevant for implementations using both FIQ
and IRQ interrupt sources.

Scenario:
IRQ event arrives.
Handler will execute:
IRQ:
        // Note: I use this exception stack while saving the context because
        // the current SP does not seem to be always valid in this CPU mode.
        ldr     sp,.__exception_stack   // get good stack
        stmfd   sp!,{r0-r5}             // save some supervisor regs
        sub     r0,lr,#4                // PC at time of interrupt
        mrs     r1,spsr
        mov     r2,#CYGNUM_HAL_VECTOR_IRQ
        mov     r3,sp

At this point r0-r5 of the interrupted code is stored in __exception_stack.
If any FIQ happens the FIQ handler will detect that the mode is CPSR_IRQ_MODE
and disable itself. That protects this region so far.

The IRQ handler continues:
        mrs     r4,cpsr                 // switch to Supervisor Mode
        bic     r4,r4,#CPSR_MODE_BITS
        // When handling an IRQ we must disable FIQ unless the current 
        // mode in CPSR is IRQ. If we were to get a FIQ while in another 
        // mode, the FIQ handling code would transform the FIQ into an 
        // IRQ and call the non-reentrant IRQ handler again. As a result, 
        // for example, the stack pointer would be set to the beginning 
        // of the exception_stack clobbering the registers we have just 
        // saved.
        orr     r4,r4,#CPSR_SUPERVISOR_MODE|CPSR_FIQ_DISABLE
        msr     cpsr,r4

The problem is in the last instruction above. The ARM7 architecture
documentation states:
"If an interrupt is received by the core during execution of an instruction
that disables interrupts, the ARM7 family will still take the interrupt. This
occurs for both IRQ and FIQ interrupts."

So assume the FIQ is happening while executing msr     cpsr,r4. The instruction
will complete and then the FIQ will run. Unfortunately the processor is not in
CPSR_IRQ_MODE any more so the FIQ will fall through into the IRQ, trashing the
stored r0-r5.

After the FIQ is done its work, the original IRQ will continue and eventually
return to the interrupted code with r0-r5 corrupted.

This can obviously result in all sorts of crashes and usually the code will
eventually end up in an exception handler or run wild.

We would like to propose a fix for this problem as well. This fix has been
tested and it solves the problem. The fix is to first disable FIQ and then
change mode. The following code snippet does that:

IRQ:
        // Note: I use this exception stack while saving the context because
        // the current SP does not seem to be always valid in this CPU mode.
        ldr     sp,.__exception_stack   // get good stack
        stmfd   sp!,{r0-r5}             // save some supervisor regs
        sub     r0,lr,#4                // PC at time of interrupt
        mrs     r1,spsr
        mov     r2,#CYGNUM_HAL_VECTOR_IRQ
        mov     r3,sp

        mrs     r4,cpsr                 // switch to Supervisor Mode

        // ML/BK18-01-2011, small chance of contention with FIQ
        //   we should disable FIQ while still in IRQ mode and then
        //   change to SVC mode
        orr     r4,r4,#CPSR_FIQ_DISABLE        
        msr     cpsr,r4

        bic     r4,r4,#CPSR_MODE_BITS
        // When handling an IRQ we must disable FIQ unless the current 
        // mode in CPSR is IRQ. If we were to get a FIQ while in another 
        // mode, the FIQ handling code would transform the FIQ into an 
        // IRQ and call the non-reentrant IRQ handler again. As a result, 
        // for example, the stack pointer would be set to the beginning 
        // of the exception_stack clobbering the registers we have just 
        // saved.
        orr     r4,r4,#CPSR_SUPERVISOR_MODE|CPSR_FIQ_DISABLE
        msr     cpsr,r4


Brian Karr and Morten Lave
TC applied Technologies.

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.