linux-kernel - Re: [PATCH v6 02/11] printk: Use struct console for suppression and extended console state

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: <Zz1i78qGL02oF8Zl@chrisdown.name>
Date: Wed, 20 Nov 2024 04:17:51 +0000
From: Chris Down <chris@...isdown.name>
To: Petr Mladek <pmladek@...e.com>, linux-kernel@...r.kernel.org
Cc: John Ogness <john.ogness@...utronix.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	Tony Lindgren <tony.lindgren@...ux.intel.com>, kernel-team@...com
Subject: Re: [PATCH v6 02/11] printk: Use struct console for suppression and
 extended console state

John Ogness writes:
>On 2024-10-28, Chris Down <chris@...isdown.name> wrote:
>> In preparation for supporting per-console loglevels, modify
>> printk_get_next_message() to accept the console itself instead of
>> individual arguments that mimic its fields.
>
>Sorry, this is not allowed. printk_get_next_message() was created
>specifically to locklessly retrieve and format arbitrary records. It
>must never be tied to a console because it has nothing to do with
>consoles (as can bee seen with the devkmsg_read() hack you added in the
>function).
>
>I recommend adding an extra argument specifying the level.
>
>The extra argument would be redundant if may_suppress=false. So perhaps
>as an alternative change "bool may_suppress" to "u32 supress_level". The
>loglevels are only 3 bits. So you could easily define a special value
>NO_SUPPRESS to represent the may_suppress=false case.
>
>#define NO_SUPPRESS BIT(31)
>
>bool printk_get_next_message(struct printk_message *pmsg, u64 seq,
>                             bool is_extended, u32 suppress_level);
>
>Then in devkmsg_read():
>
>printk_get_next_message(&pmsg, atomic64_read(&user->seq), true, NO_SUPRRESS)

Petr, what do you think about this? I remember when we discussed this before we 
talked about either determining state via `struct console` (which seems to turn 
out not to be feasible) or passing another argument.

Do you prefer to have another argument or do the bit dance?

Personally I prefer the simpler solution with more arguments instead of bit 
stuffing if we have to go this way, but I'm up for whichever sounds good to 
you.