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: <20180912133347.23ulng4njaa3uzsa@pathway.suse.cz>
Date:   Wed, 12 Sep 2018 15:33:47 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Cc:     Hans de Goede <hdegoede@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H . Peter Anvin" <hpa@...or.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        x86@...nel.org, linux-kernel@...r.kernel.org,
        Steven Rostedt <rostedt@...dmis.org>,
        Maninder Singh <maninder1.s@...sung.com>
Subject: Re: [PATCH 4.19 regression fix] printk: For early boot messages
 check loglevel when flushing the buffer

On Wed 2018-09-12 16:49:30, Sergey Senozhatsky wrote:
> On (09/11/18 10:47), Petr Mladek wrote:
> > > > Most problems should probably be solved when we store console_seq
> > > > before setting exclusive_console. Then we could clear
> > > > exclusive_console when reaching the stored sequence number.
> 
> Hmm, wouldn't the same "it might be problematic with slow consoles"
> argument apply to this solution as well?

It would just fix a clear bug.

> So maybe we can do the following:
> 
> - store console_seq when we register exclusive console
> - in flush_on_panic, if we have exclusive console set
>     - clear exclusive console
>     - rollback console_seq to the value it had before we set exclusive console
 
> ---
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 53c94cbce0af..4ef199572df7 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -424,6 +424,7 @@ static u32 log_next_idx;
>  /* the next printk record to write to the console */
>  static u64 console_seq;
>  static u32 console_idx;
> +static u64 rollback_console_seq;
>  
>  /* the next printk record to read after the last 'clear' command */
>  static u64 clear_seq;
> @@ -2592,6 +2593,10 @@ void console_flush_on_panic(void)
>  	 */
>  	console_trylock();
>  	console_may_schedule = 0;
> +	if (exclusive_console) {
> +		exclusive_console = NULL;
> +		console_seq = rollback_console_seq;

This might be controversial. Yes, it might help in some situations
(slow exclusive console). But it might be counterproductive
when the exclusive console is the one that is readable
after the crash, used for debugging and this change causes
an incomplete log.

I would do this change only when people complains about
the current behavior.

I think that more important is to do:

@@ -2415,6 +2415,9 @@ void console_unlock(void)
                console_seq++;
                raw_spin_unlock(&logbuf_lock);
 
+               if (exclusive_console && console_seq >= rollback_console_seq)
+                       exclusive_console = NULL;
+
                /*
                 * While actively printing out messages, if another printk()
                 * were to occur on another CPU, it may wait for this one to

IMHO, this is a clear win. It fixes a clear mistake.

I would just rename the variable to exclusive_console_seq_stop or so.

Best Regards,
Petr

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ