Signal Handler Interrupt Management
(too old to reply)
Mark Schulte schultetwin@gmail.com [nuttx]
2018-01-29 20:50:34 UTC
Hi All,

Signal handlers maybe run with interrupts enabled or disabled,
depending on how the task the received the signal was blocked. (i.e.:
If sem_wait() is called, then we disable interrupts, then block the
currently running task). I've noticed this on armv7-m, although I
believe it also is true on armv7-a, armv6-m, and potentially other
platforms. I think this could be dangerous, because we're running user
code with interrupts disabled. I think this is especially true if a
page fault may occur in the signal handler (although I'm not too
familiar with the armv7-a architecture, so I think this is okay.)

My proposed solution is to force enable interrupts in up_sigdeliver(),
calling up_irq_enable() instead of
leave_critical_section(regs[REG_PRIMASK]). I believe this is safe
because, when we return to normal execution, interrupts will be
restored to their previous state when the signal handler returns.
However, I'm not too familiar with the synchronization primatives in
NuttX, and want to make sure that I'm not violating some critical
section by doing so. Any thoughts on this?

I'm happy to provide the patch if this seems like a good solution.
Just want to make sure it makes sense.

spudarnia@yahoo.com [nuttx]
2018-01-29 22:36:48 UTC
Yes, it does not make sense for user code to be running with interrupts disabled. I can't think of any problem that having interrupts disabled would cause off the top of my head other than some unexpected performance issues because the signal handler might keep interrupts disabled for some time. If some logic were to spin waiting for a timer to change, that logic could hang with no timer interrupts.

This change, of course, would affect all architectures. I wouldn't want one behaving in one way or and others in another way. I can help with the conversion of any architectures that you are uncomfortable with. Not all define up_irq_enable. Only ARMv6-M, ARMv7-A, ARMv7-M, ARMv7-R, AVR, SH1, i486, Xtensa do. RISC-V has up_enable_irq(). All other architectures should support some way of disabling or enabling interrupts unconditionally (those would be AVR32, all MIPS/PIC32, MISOC LM3, M16C, ZNeo, eZ80, z8, and Z80).

On ARMv7-A, disable interrupts sets the 'I' bit in the CPSR which, I believe, will effect only external interrupts and not other exceptions like page faults (data aborts or prefetch abort), FIQs, SVCs, or undefined instructions. So other than the above-mentioned performance risk (and the general aesthetics of it all), everything else should run normally.