linux-kernel - Re: [PATCH v3 2/2] Handle flushing of CPU backtraces during panic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <877cc38exx.fsf@jogness.linutronix.de>
Date: Mon, 26 Aug 2024 17:52:34 +0206
From: John Ogness <john.ogness@...utronix.de>
To: takakura@...inux.co.jp, pmladek@...e.com
Cc: akpm@...ux-foundation.org, bhe@...hat.com, feng.tang@...el.com,
 j.granados@...sung.com, linux-kernel@...r.kernel.org, lukas@...ner.de,
 nishimura@...inux.co.jp, rostedt@...dmis.org, senozhatsky@...omium.org,
 stephen.s.brennan@...cle.com, taka@...inux.co.jp, takakura@...inux.co.jp,
 ubizjak@...il.com, wangkefeng.wang@...wei.com
Subject: Re: [PATCH v3 2/2] Handle flushing of CPU backtraces during panic

On 2024-08-21, takakura@...inux.co.jp wrote:
>>> /**
>>>  * console_try_or_trigger_flush - try to flush consoles directly when
>>>  *   safe or the trigger deferred flush.
>>>  *
>>>  * Context: Any
>>>  */
>>> void console_try_or_trigger_flush(void)
>>> {
>>>      if (!is_printk_legacy_deferred() && console_trylock())
>>>              console_unlock();
>>>      else
>>>              defer_console_output();
>>> }
>>>
>>> and use it instead of printk_trigger_flush() in
>>> nmi_trigger_cpumask_backtrace().
>>
>> Just to be clear, you are talking about removing
>> printk_trigger_flush() entirely and instead provide the new
>> console_try_or_trigger_flush()?  Which then also involves updating
>> the call sites:
>>
>> lib/nmi_backtrace.c:nmi_trigger_cpumask_backtrace()
>> arch/powerpc/kernel/watchdog.c:watchdog_timer_interrupt()
>>
>
> Taking a look at [0], in addition to the mentioned call sites, 
> nbcon_device_release() will also be calling printk_trigger_flush()?
> For nbcon_device_release(), I thought its better not to be replaced as 
> it calles for @legacy_off, in which case printk_trigger_flush() seems 
> more suitable as it always defers printing.

The @legacy_off logic would be within console_try_or_trigger_flush(),
so the result would be the same.

> Also taking a look at the [1], for nmi_trigger_cpumask_backtrace(), 
> I thought that it will not comply with the syncing of 
> legacy_allow_panic_sync. I believe it will allow flushing of legacy consoles 
> before printk_legacy_allow_panic_sync() which is out of sync.

But isn't your patch also causing that violation?

printk_legacy_allow_panic_sync() performs a trylock/unlock. Isn't that
good enough?

John