[<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
Powered by blists - more mailing lists