Messages in this thread | | | From | Doug Anderson <> | Date | Thu, 31 Aug 2023 09:16:31 -0700 | Subject | Re: [PATCH] lkdtm/bugs: add test for panic() with stuck secondary CPUs |
| |
Hi,
On Thu, Aug 31, 2023 at 3:10 AM Mark Rutland <mark.rutland@arm.com> wrote: > > Upon a panic() the kernel will use either smp_send_stop() or > crash_smp_send_stop() to attempt to stop secondary CPUs via an IPI, > which may or may not be an NMI. Generally it's preferable that this is an > NMI so that CPUs can be stopped in as many situations as possible, but > it's not always possible to provide an NMI, and there are cases where > CPUs may be unable to handle the NMI regardless. > > This patch adds a test for panic() where all other CPUs are stuck with > interrupts disabled, which can be used to check whether the kernel > gracefully handles CPUs failing to respond to a stop, and whe NMIs stops > work. > > For example, on arm64 *without* an NMI, this results in: > > | # echo PANIC_STOP_IRQOFF > /sys/kernel/debug/provoke-crash/DIRECT > | lkdtm: Performing direct entry PANIC_STOP_IRQOFF > | Kernel panic - not syncing: panic stop irqoff test > | CPU: 2 PID: 24 Comm: migration/2 Not tainted 6.5.0-rc3-00077-ge6c782389895-dirty #4 > | Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015 > | Stopper: multi_cpu_stop+0x0/0x1a0 <- stop_machine_cpuslocked+0x158/0x1a4 > | Call trace: > | dump_backtrace+0x94/0xec > | show_stack+0x18/0x24 > | dump_stack_lvl+0x74/0xc0 > | dump_stack+0x18/0x24 > | panic+0x358/0x3e8 > | lkdtm_PANIC+0x0/0x18 > | multi_cpu_stop+0x9c/0x1a0 > | cpu_stopper_thread+0x84/0x118 > | smpboot_thread_fn+0x224/0x248 > | kthread+0x114/0x118 > | ret_from_fork+0x10/0x20 > | SMP: stopping secondary CPUs > | SMP: failed to stop secondary CPUs 0-3 > | Kernel Offset: 0x401cf3490000 from 0xffff800080000000 > | PHYS_OFFSET: 0x40000000 > | CPU features: 0x00000000,68c167a1,cce6773f > | Memory Limit: none > | ---[ end Kernel panic - not syncing: panic stop irqoff test ]--- > > On arm64 *with* an NMI, this results in: > > | # echo PANIC_STOP_IRQOFF > /sys/kernel/debug/provoke-crash/DIRECT > | lkdtm: Performing direct entry PANIC_STOP_IRQOFF > | Kernel panic - not syncing: panic stop irqoff test > | CPU: 1 PID: 19 Comm: migration/1 Not tainted 6.5.0-rc3-00077-ge6c782389895-dirty #4 > | Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015 > | Stopper: multi_cpu_stop+0x0/0x1a0 <- stop_machine_cpuslocked+0x158/0x1a4 > | Call trace: > | dump_backtrace+0x94/0xec > | show_stack+0x18/0x24 > | dump_stack_lvl+0x74/0xc0 > | dump_stack+0x18/0x24 > | panic+0x358/0x3e8 > | lkdtm_PANIC+0x0/0x18 > | multi_cpu_stop+0x9c/0x1a0 > | cpu_stopper_thread+0x84/0x118 > | smpboot_thread_fn+0x224/0x248 > | kthread+0x114/0x118 > | ret_from_fork+0x10/0x20 > | SMP: stopping secondary CPUs > | Kernel Offset: 0x55a9c0bc0000 from 0xffff800080000000 > | PHYS_OFFSET: 0x40000000 > | CPU features: 0x00000000,68c167a1,fce6773f > | Memory Limit: none > | ---[ end Kernel panic - not syncing: panic stop irqoff test ]---
Since Sumit didn't notice it and I had to use a "diff" tool to help me spot the difference in your two examples, maybe telegraph for the reader of the commit message that the important thing is the lack of "SMP: failed to stop secondary CPUs" in your example.
> +static void lkdtm_PANIC_STOP_IRQOFF(void) > +{ > + atomic_t v = ATOMIC_INIT(0); > + > + cpus_read_lock(); > + stop_machine(panic_stop_irqoff_fn, &v, cpu_online_mask);
It broke my brain just a little bit that stop_machine() isn't backed by "IPI_CPU_STOP" since the point of this was to test the patch modifying how "IPI_CPU_STOP" worked. ;-) Assuming I didn't mess up, I guess stop_machine() is just used to get all the CPUs running with interrupts disabled and then the last CPU does a panic. The _panic_ then tests "IPI_CPU_STOP" / "IPI_CPU_CRASH_STOP".
Probably this is obvious to everyone but me, so no action is needed unless you agree that it's confusing and can think of a good comment / commit message addition to help.
In any case, after Sumit's feedback is addressed:
Reviewed-by: Douglas Anderson <dianders@chromium.org>
| |