linux-kernel - Re: [PATCH printk v4 5/8] printk: introduce printk_get_next_message() and printk

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]

Date:   Thu, 5 Jan 2023 16:24:55 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     John Ogness <john.ogness@...utronix.de>
Cc:     Sergey Senozhatsky <senozhatsky@...omium.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH printk v4 5/8] printk: introduce
 printk_get_next_message() and printk_message

On Thu 2023-01-05 11:43:32, John Ogness wrote:
> Code for performing the console output is intermixed with code that
> is formatting the output for that console. Introduce a new helper
> function printk_get_next_message() to handle the reading and
> formatting of the printk text. The helper does not require any
> locking so that in the future it can be used for other printing
> contexts as well.
> 
> This also introduces a new struct printk_message to wrap the struct
> printk_buffers, adding metadata about its contents. This allows
> users of printk_get_next_message() to receive all relevant
> information about the message that was read and formatted.
> 
> Why is struct printk_message a wrapper struct?
> 
> It is intentional that a wrapper struct is introduced instead of
> adding the metadata directly to struct printk_buffers. The upcoming
> atomic consoles support multiple printing contexts per CPU. This
> means that while a CPU is formatting a message, it can be
> interrupted and the interrupting context may also format a (possibly
> different) message. Since the printk buffers are rather large,
> there will only be one struct printk_buffers per CPU and it must be
> shared by the possible contexts of that CPU.
> 
> If the metadata was part of struct printk_buffers, interrupting
> contexts would clobber the metadata being prepared by the
> interrupted context. This could be handled by robustifying the
> message formatting functions to cope with metadata unexpectedly
> changing. However, this would require significant amounts of extra
> data copying, also adding significant complexity to the code.
> 
> Instead, the metadata can live on the stack of the formatting
> context and the message formatting functions do not need to be
> concerned about the metadata changing underneath them.
> 
> Note that the message formatting functions can handle unexpected
> text buffer changes. So it is perfectly OK if a shared text buffer
> is clobbered by an interrupting context. The atomic console
> implementation will recognize the interruption and avoid printing
> the (probably garbage) text buffer.

Great description!

> Signed-off-by: John Ogness <john.ogness@...utronix.de>

Reviewed-by: Petr Mladek <pmladek@...e.com>

Best Regards,
Petr