linux-kernel - Re: [PATCH 4.19 regression fix] printk: For early boot messages check loglevel when flushing the buffer

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: <20180907042132.GC2394@jagdpanzerIV>
Date:   Fri, 7 Sep 2018 13:21:32 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
To:     Petr Mladek <pmladek@...e.com>
Cc:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        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 (09/06/18 16:28), Petr Mladek wrote:
> On Thu 2018-09-06 16:29:40, Sergey Senozhatsky wrote:
> > On (09/05/18 13:02), Petr Mladek wrote:
> > > Note that the first registered console prints all messages
> > > even without this flag.
> > 
> > Hmm, OK, interesting point.
> > 
> > I assumed that the first console usually has CON_PRINTBUFFER bit set.
> > Or even a CON_PRINTBUFFER | CON_ANYTIME combo. E.g. 8250. It sort of
> > makes sense to have CON_PRINTBUFFER for the first console. Any later
> > consoles [e.g. fbcon, netcon] don't necessarily have CON_PRINTBUFFER.
> > 
> > And the first console has CON_PRINTBUFFER bit set. Well, just because
> > it sounds reasonable. Those were the main assumptions behind my code
> > snippet. Was any of those assumptions wrong?
> 
> This assumption makes sense. In fact, I was wrong. I thought that
> console_seq/console_idx were not updated until the first console
> was registered. But it is not the case.
> 
> It means that the hack with exclusive_console might be usable.

Yeah, it is a hack. But not as dirty as it might appear, I think. In some
sense it's aligned with what we do for exlusive_consoles - we treat exclusive
consoles specially. So specially that even if the system panics while we
re-flush logbuf messages to a new exclusive console, we flush_on_panic() only
to that exclusive console, ignoring the rest of them.

Not sure if it's totally right. There can be a netcon, for instance,
available, which will not see panic flush() because of a exclusive
console:

---

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index c036f128cdc3..ede29a7ba6db 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2545,6 +2545,7 @@ void console_flush_on_panic(void)
 	 * ensure may_schedule is cleared.
 	 */
 	console_trylock();
+	exclusive_console = NULL;
 	console_may_schedule = 0;
 	console_unlock();
 }

---

Opinions?

> But I would prefer to do it a cleaner way.

OK.

> But it is rather complicated, still hacky, ...

Right.

> > 
> > I can agree, definitely. That's one of the options.
> 
> I prefer the revert for now.

OK, agreed.
IIRC I didn't see any upstream code which would have been fixed
by the commit in question.

	-ss