linux-kernel - Re: [PATCH V6] panic: Move 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: <5eeba935-d73a-5539-4eb1-298fb6ee0578@igalia.com>
Date:   Wed, 23 Feb 2022 10:15:05 -0300
From:   "Guilherme G. Piccoli" <gpiccoli@...lia.com>
To:     Sergey Senozhatsky <senozhatsky@...omium.org>
Cc:     linux-kernel@...r.kernel.org, bhe@...hat.com, pmladek@...e.com,
        akpm@...ux-foundation.org, anton@...msg.org, ccross@...roid.com,
        dyoung@...hat.com, feng.tang@...el.com, john.ogness@...utronix.de,
        keescook@...omium.org, kernel@...ccoli.net,
        kexec@...ts.infradead.org, rostedt@...dmis.org,
        tony.luck@...el.com, vgoyal@...hat.com
Subject: Re: [PATCH V6] panic: Move panic_print before kmsg dumpers

On 22/02/2022 22:27, Sergey Senozhatsky wrote:
> [...]
> Hmm. Yeah, well, that's a bit of a tricky interface now
> 
> 	panic()
> 		// everything (if corresponding bits set), no console flush
> 		panic_print_sys_info(false)
> 		...
> 		// console flush only if corresponding bit set
> 		panic_print_sys_info(true)
> 
> 
> 
> If everyone is fine then OK.
> 
> But I _personally_ would look into changing this to something like this:
> 
> 	#define EARLY_PANIC_MASK (PANIC_PRINT_FOO | PANIC_PRINT_BAR | ...)
> 	#define LATE_PANIC_MASK (PANIC_PRINT_ALL_PRINTK_MSG)
> 	panic()
> 		panic_print_sys_info(panic_print & EARLY_PANIC_MASK)
> 		...
> 		panic_print_sys_info(panic_print & LATE_PANIC_MASK)

Hi Sergey, notice that panic_print_sys_info() currently doesn't have a
parameter! The prototype (without this patch) is:

static void panic_print_sys_info(void);

So, it consumes the "panic_print" global variable (which matches the
command-line parameter / sysctl), hence to implement your suggestion
either we need a refactor in panic_print_sys_info(), adding a parameter
(more or less what the patch is already doing, but with a bit more
changes) or we override the global variable twice in panic(), before the
function calls.

As you said, it's possible and a matter of personal coding style. I'd be
fine if more people ask that, but if everyone is fine with the current
implementation, I'd rather get this patch merged as is, since we need it
and couldn't even make it for 5.17 heh

Cheers,

Guilherme