| Message ID | 20230215144828.3370316-1-mnissler@rivosinc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 130aee3fd9981297ff9354e5d5609cd59aafbbea |
| Delegated to: | Palmer Dabbelt |
| Headers | show |
| Series | riscv: Avoid enabling interrupts in die() | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Single patches do not need cover letters |
| conchuod/tree_selection | success | Guessed tree name to be fixes |
| conchuod/fixes_present | success | Fixes tag present in non-next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 13 and now 13 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 3 this patch: 3 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | success | Errors and warnings before: 20 this patch: 20 |
| conchuod/alphanumeric_selects | success | Out of order selects before the patch: 729 and now 729 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 2 this patch: 2 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 20 lines checked |
| conchuod/source_inline | success | Was 0 now: 0 |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | Fixes tag looks correct |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
Mattias Nissler <mnissler@rivosinc.com> writes: > While working on something else, I noticed that the kernel would start > accepting interrupts again after crashing in an interrupt handler. Since > the kernel is already in inconsistent state, enabling interrupts is > dangerous and opens up risk of kernel state deteriorating further. > Interrupts do get enabled via what looks like an unintended side effect of > spin_unlock_irq, so switch to the more cautious > spin_lock_irqsave/spin_unlock_irqrestore instead. > > Fixes: 76d2a0493a17 ("RISC-V: Init and Halt Code") > Signed-off-by: Mattias Nissler <mnissler@rivosinc.com> Nice catch! Reviewed-by: Björn Töpel <bjorn@kernel.org>
Hello: This patch was applied to riscv/linux.git (for-next) by Palmer Dabbelt <palmer@rivosinc.com>: On Wed, 15 Feb 2023 14:48:28 +0000 you wrote: > While working on something else, I noticed that the kernel would start > accepting interrupts again after crashing in an interrupt handler. Since > the kernel is already in inconsistent state, enabling interrupts is > dangerous and opens up risk of kernel state deteriorating further. > Interrupts do get enabled via what looks like an unintended side effect of > spin_unlock_irq, so switch to the more cautious > spin_lock_irqsave/spin_unlock_irqrestore instead. > > [...] Here is the summary with links: - riscv: Avoid enabling interrupts in die() https://git.kernel.org/riscv/c/130aee3fd998 You are awesome, thank you!
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index 549bde5c970a..70c98ce23be2 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -34,10 +34,11 @@ void die(struct pt_regs *regs, const char *str) static int die_counter; int ret; long cause; + unsigned long flags; oops_enter(); - spin_lock_irq(&die_lock); + spin_lock_irqsave(&die_lock, flags); console_verbose(); bust_spinlocks(1); @@ -54,7 +55,7 @@ void die(struct pt_regs *regs, const char *str) bust_spinlocks(0); add_taint(TAINT_DIE, LOCKDEP_NOW_UNRELIABLE); - spin_unlock_irq(&die_lock); + spin_unlock_irqrestore(&die_lock, flags); oops_exit(); if (in_interrupt())
While working on something else, I noticed that the kernel would start accepting interrupts again after crashing in an interrupt handler. Since the kernel is already in inconsistent state, enabling interrupts is dangerous and opens up risk of kernel state deteriorating further. Interrupts do get enabled via what looks like an unintended side effect of spin_unlock_irq, so switch to the more cautious spin_lock_irqsave/spin_unlock_irqrestore instead. Fixes: 76d2a0493a17 ("RISC-V: Init and Halt Code") Signed-off-by: Mattias Nissler <mnissler@rivosinc.com> --- arch/riscv/kernel/traps.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)