linux-kernel - Re: [PATCH 1/2] printk: Unconditionally unregister boot consoles if in init section

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] [day] [month] [year] [list]

Message-ID: <20170714215441.GA10437@tigerII.localdomain>
Date:   Sat, 15 Jul 2017 06:54:41 +0900
From:   Sergey Senozhatsky <sergey.senozhatsky@...il.com>
To:     Petr Mladek <pmladek@...e.com>
Cc:     Matt Redfearn <matt.redfearn@...tec.com>,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>, linux-serial@...r.kernel.org,
        Steven Rostedt <rostedt@...dmis.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] printk: Unconditionally unregister boot consoles if
 in init section

Hello,

sorry, I'm on a sick leave and can't check emails that often.

On (07/11/17 14:43), Petr Mladek wrote:
[..]
> > >>The keep_bootcon flag prevents the unregistration of a boot console,
> > >>even if it's data and code reside in the init section and are about to
> > >>be freed. This can lead to crashes and weird panics when the bootconsole
> > >>is accessed after free, especially if page poisoning is in use and the
> > >>code / data have been overwritten with a poison value.
> > >>To prevent this, always free the boot console if it is within the init
> > >>section.
> 
> > >if someone asked to `keep_bootcon' but we actually don't keep it, then
> > >what's the purpose of the flag and
> > >	pr_info("debug: skip boot console de-registration.\n")?
> 
> Exactly. The important information is in the commit 7bf693951a8e5f7e
> ("console: allow to retain boot console via boot option keep_bootcon"):
> 
>     On some architectures, the boot process involves de-registering the boot
>     console (early boot), initialize drivers and then re-register the console.
> 
>     This mechanism introduces a window in which no printk can happen on the
>     console and messages are buffered and then printed once the new console is
>     available.
> 
>     If a kernel crashes during this window, all it's left on the boot console
>     is "console [foo] enabled, boot console disabled" making debug of the crash
>     rather 'interesting'.

sure. no objections here.
I probably mis-worded my thoughts. I agree with Matt's conclusions and
he made valid points I just disagree with the fix.

> > >keeping `early_printk' sometimes can be helpful and people even want to
> > >use `early_printk' as a panic() console fallback (because of those nasty
> > >deadlocks that spoil printk->call_console_drivers()).
> > >
> > 
> > Sure, but as a user, how are you supposed to know that?
> 
> Good point! I wonder if the authors of the keep_bootcon option
> actually knew about it. I do not see this risk mentioned anywhere
> and the early consoles might work long enough by chance.
> 
> One problem is that real consoles might be registered much later
> when it is done using an async probe calls. It might open a big window
> when there is no visible output and debugging is "impossible".

yes. besides, Peter wants to have early consoles as panic fallback.

> I am not comfortable with removing the only way to debug some type
> of bugs. But the current state is broken as well.

yes and yes.

> IMHO, the reasonable solution is to move early console code and data
> out of the init sections. We should do this for the early consoles
> where the corresponding real console is registered using a deferred
> probe. Others should be already replaced by the real console when
> printk_late_init() is called. At least this is how I understand it.

so I was thinking about two options:

a) do not keep consoles in init section

b) have a special init section for consoles code and avoid offloading
   the corresponding pages when we see that keep flag


"b" seems like a crazy idea at glance, but it kinda makes some sense at
the same time. dunno.

	-ss