riscv: Fix sleeping in invalid context in die()

[PATCH] riscv: Fix sleeping in invalid context in die()

Posted by Nam Cao 1 year, 2 months ago

die() can be called in exception handler, and therefore cannot sleep.
However, die() takes spinlock_t which can sleep with PREEMPT_RT enabled.
That causes the following warning:

BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 285, name: mutex
preempt_count: 110001, expected: 0
RCU nest depth: 0, expected: 0
CPU: 0 UID: 0 PID: 285 Comm: mutex Not tainted 6.12.0-rc7-00022-ge19049cf7d56-dirty #234
Hardware name: riscv-virtio,qemu (DT)
Call Trace:
    dump_backtrace+0x1c/0x24
    show_stack+0x2c/0x38
    dump_stack_lvl+0x5a/0x72
    dump_stack+0x14/0x1c
    __might_resched+0x130/0x13a
    rt_spin_lock+0x2a/0x5c
    die+0x24/0x112
    do_trap_insn_illegal+0xa0/0xea
    _new_vmalloc_restore_context_a0+0xcc/0xd8
Oops - illegal instruction [#1]

Switch to use raw_spinlock_t, which does not sleep even with PREEMPT_RT
enabled.

Fixes: 76d2a0493a17 ("RISC-V: Init and Halt Code")
Signed-off-by: Nam Cao <namcao@linutronix.de>
Cc: stable@vger.kernel.org
---
stable backport is probably not needed for versions earlier than 6.12
because PREEMPT_RT is not enabled. But it doesn't hurt..
---
 arch/riscv/kernel/traps.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index 51ebfd23e007..8ff8e8b36524 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -35,7 +35,7 @@
 
 int show_unhandled_signals = 1;
 
-static DEFINE_SPINLOCK(die_lock);
+static DEFINE_RAW_SPINLOCK(die_lock);
 
 static int copy_code(struct pt_regs *regs, u16 *val, const u16 *insns)
 {
@@ -81,7 +81,7 @@ void die(struct pt_regs *regs, const char *str)
 
 	oops_enter();
 
-	spin_lock_irqsave(&die_lock, flags);
+	raw_spin_lock_irqsave(&die_lock, flags);
 	console_verbose();
 	bust_spinlocks(1);
 
@@ -100,7 +100,7 @@ void die(struct pt_regs *regs, const char *str)
 
 	bust_spinlocks(0);
 	add_taint(TAINT_DIE, LOCKDEP_NOW_UNRELIABLE);
-	spin_unlock_irqrestore(&die_lock, flags);
+	raw_spin_unlock_irqrestore(&die_lock, flags);
 	oops_exit();
 
 	if (in_interrupt())
-- 
2.39.5

Re: [PATCH] riscv: Fix sleeping in invalid context in die()

Posted by Sebastian Andrzej Siewior 1 year, 2 months ago

On 2024-11-18 10:13:33 [+0100], Nam Cao wrote:
> die() can be called in exception handler, and therefore cannot sleep.
> However, die() takes spinlock_t which can sleep with PREEMPT_RT enabled.
> That causes the following warning:
> 
> BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
> in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 285, name: mutex
> preempt_count: 110001, expected: 0
> RCU nest depth: 0, expected: 0
> CPU: 0 UID: 0 PID: 285 Comm: mutex Not tainted 6.12.0-rc7-00022-ge19049cf7d56-dirty #234
> Hardware name: riscv-virtio,qemu (DT)
> Call Trace:
>     dump_backtrace+0x1c/0x24
>     show_stack+0x2c/0x38
>     dump_stack_lvl+0x5a/0x72
>     dump_stack+0x14/0x1c
>     __might_resched+0x130/0x13a
>     rt_spin_lock+0x2a/0x5c
>     die+0x24/0x112
>     do_trap_insn_illegal+0xa0/0xea
>     _new_vmalloc_restore_context_a0+0xcc/0xd8
> Oops - illegal instruction [#1]
> 
> Switch to use raw_spinlock_t, which does not sleep even with PREEMPT_RT
> enabled.
> 
> Fixes: 76d2a0493a17 ("RISC-V: Init and Halt Code")
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> Cc: stable@vger.kernel.org

Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

The die_lock() is probably do let one CPU die at a time. On x86 there is
support for for recursive die so if it happens, you don't spin on the
die_lock and see nothing. Not sure if this is a thing.

Sebastian

Re: [PATCH] riscv: Fix sleeping in invalid context in die()

Posted by Palmer Dabbelt 1 year, 1 month ago

On Wed, 20 Nov 2024 00:50:45 PST (-0800), bigeasy@linutronix.de wrote:
> On 2024-11-18 10:13:33 [+0100], Nam Cao wrote:
>> die() can be called in exception handler, and therefore cannot sleep.
>> However, die() takes spinlock_t which can sleep with PREEMPT_RT enabled.
>> That causes the following warning:
>>
>> BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
>> in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 285, name: mutex
>> preempt_count: 110001, expected: 0
>> RCU nest depth: 0, expected: 0
>> CPU: 0 UID: 0 PID: 285 Comm: mutex Not tainted 6.12.0-rc7-00022-ge19049cf7d56-dirty #234
>> Hardware name: riscv-virtio,qemu (DT)
>> Call Trace:
>>     dump_backtrace+0x1c/0x24
>>     show_stack+0x2c/0x38
>>     dump_stack_lvl+0x5a/0x72
>>     dump_stack+0x14/0x1c
>>     __might_resched+0x130/0x13a
>>     rt_spin_lock+0x2a/0x5c
>>     die+0x24/0x112
>>     do_trap_insn_illegal+0xa0/0xea
>>     _new_vmalloc_restore_context_a0+0xcc/0xd8
>> Oops - illegal instruction [#1]
>>
>> Switch to use raw_spinlock_t, which does not sleep even with PREEMPT_RT
>> enabled.
>>
>> Fixes: 76d2a0493a17 ("RISC-V: Init and Halt Code")
>> Signed-off-by: Nam Cao <namcao@linutronix.de>
>> Cc: stable@vger.kernel.org
>
> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>
> The die_lock() is probably do let one CPU die at a time. On x86 there is
> support for for recursive die so if it happens, you don't spin on the
> die_lock and see nothing. Not sure if this is a thing.

Looks like the RISC-V code is pretty much the same as the arm64 code, so 
it probably just came from there.  I don't really know what the right 
answer is here...

>
> Sebastian