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

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

[Matt Redfearn]

Commit 4c30c6f566c0 ("kernel/printk: do not turn off bootconsole in
printk_late_init() if keep_bootcon") added a check on keep_bootcon to
ensure that boot consoles were kept around until the real console is
registered.
This can lead to problems if the boot console data and code are in the
init section, since it can be freed before the boot console is
deregistered. This was fixed by commit 81cc26f2bd11 ("printk: only
unregister boot consoles when necessary").
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.

Fixes 81cc26f2bd11 ("printk: only unregister boot consoles when necessary").
Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>

---

 kernel/printk/printk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index a1db38abac5b..b5411f44eed4 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2660,7 +2660,7 @@ static int __init printk_late_init(void)
 	int ret;
 
 	for_each_console(con) {
-		if (!keep_bootcon && con->flags & CON_BOOT) {
+		if (con->flags & CON_BOOT) {
 			/*
 			 * Make sure to unregister boot consoles whose data
 			 * resides in the init section before the init section
-- 
2.7.4

[PATCH 2/2] serial: earlycon: Make early_con as __initdata

[Matt Redfearn]

All early console drivers that may be registered as the earlycon are
marked __init to be placed in the init section. The drivers' code and
data are freed during free_initmem_default() but the early console is
not unregistered in printk_late_init() as the init_section_intersects()
test fails. This leads to the earlycon still being active, potentially
with dangling pointers into the freed init section, which may have been
poisoned by the slab debugger. Attempting to use the boot console after
this will likely lead to weird behaviour and kernel crashes.

Fix this by marking the early_con struct __initdata so that the
init_section_intersects() test succeeds and the console is unregistered
in printk_late_init() before the init section is freed.

The 8250 earlycon, on which the generic earlycon was based, had these
attributes on it's console struct. The switch to the generic
implementation in commit d2fd6810a823 ("tty/serial: convert 8250 to
generic earlycon") appears to have broken unregistraton of the boot
console when its code and data are in __init.

Fixes: d2fd6810a823 ("tty/serial: convert 8250 to generic earlycon")
Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>

---

 drivers/tty/serial/earlycon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index c3651540e1ba..388aaafcca2b 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -29,13 +29,13 @@
 
 #include <asm/serial.h>
 
-static struct console early_con = {
+static struct console early_con __initdata = {
 	.name =		"uart",		/* fixed up at earlycon registration */
 	.flags =	CON_PRINTBUFFER | CON_BOOT,
 	.index =	0,
 };
 
-static struct earlycon_device early_console_dev = {
+static struct earlycon_device early_console_dev __initdata = {
 	.con = &early_con,
 };
 
-- 
2.7.4

Re: [PATCH 2/2] serial: earlycon: Make early_con as __initdata

[Sergey Senozhatsky]

On (07/06/17 11:38), Matt Redfearn wrote:
> All early console drivers that may be registered as the earlycon are
> marked __init to be placed in the init section. The drivers' code and
> data are freed during free_initmem_default() but the early console is
> not unregistered in printk_late_init() as the init_section_intersects()
> test fails. This leads to the earlycon still being active, potentially
> with dangling pointers into the freed init section, which may have been
> poisoned by the slab debugger. Attempting to use the boot console after
> this will likely lead to weird behaviour and kernel crashes.

people want to use early-con as a panic() console fallback.

	-ss

> Fix this by marking the early_con struct __initdata so that the
> init_section_intersects() test succeeds and the console is unregistered
> in printk_late_init() before the init section is freed.
> 
> The 8250 earlycon, on which the generic earlycon was based, had these
> attributes on it's console struct. The switch to the generic
> implementation in commit d2fd6810a823 ("tty/serial: convert 8250 to
> generic earlycon") appears to have broken unregistraton of the boot
> console when its code and data are in __init.
> 
> Fixes: d2fd6810a823 ("tty/serial: convert 8250 to generic earlycon")
> Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
> 
> ---
> 
>  drivers/tty/serial/earlycon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index c3651540e1ba..388aaafcca2b 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -29,13 +29,13 @@
>  
>  #include <asm/serial.h>
>  
> -static struct console early_con = {
> +static struct console early_con __initdata = {
>  	.name =		"uart",		/* fixed up at earlycon registration */
>  	.flags =	CON_PRINTBUFFER | CON_BOOT,
>  	.index =	0,
>  };
>  
> -static struct earlycon_device early_console_dev = {
> +static struct earlycon_device early_console_dev __initdata = {
>  	.con = &early_con,
>  };
>  
> -- 
> 2.7.4
> 

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

[Sergey Senozhatsky]

On (07/06/17 11:38), Matt Redfearn wrote:
> Commit 4c30c6f566c0 ("kernel/printk: do not turn off bootconsole in
> printk_late_init() if keep_bootcon") added a check on keep_bootcon to
> ensure that boot consoles were kept around until the real console is
> registered.
> This can lead to problems if the boot console data and code are in the
> init section, since it can be freed before the boot console is
> deregistered. This was fixed by commit 81cc26f2bd11 ("printk: only
> unregister boot consoles when necessary").
> 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.
> 
> Fixes 81cc26f2bd11 ("printk: only unregister boot consoles when necessary").
> Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>

well... hm. don't specify `keep_bootcon' if your bootcon access initdata?
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()).

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")?


> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index a1db38abac5b..b5411f44eed4 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2660,7 +2660,7 @@ static int __init printk_late_init(void)
>  	int ret;
>  
>  	for_each_console(con) {
> -		if (!keep_bootcon && con->flags & CON_BOOT) {
> +		if (con->flags & CON_BOOT) {
>  			/*
>  			 * Make sure to unregister boot consoles whose data
>  			 * resides in the init section before the init section

what about bootconsoles that are still not getting de-registered due
to keep_bootcon in register_console()?

	-ss

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

[Matt Redfearn]

On 07/07/17 05:45, Sergey Senozhatsky wrote:
> On (07/06/17 11:38), Matt Redfearn wrote:
>> Commit 4c30c6f566c0 ("kernel/printk: do not turn off bootconsole in
>> printk_late_init() if keep_bootcon") added a check on keep_bootcon to
>> ensure that boot consoles were kept around until the real console is
>> registered.
>> This can lead to problems if the boot console data and code are in the
>> init section, since it can be freed before the boot console is
>> deregistered. This was fixed by commit 81cc26f2bd11 ("printk: only
>> unregister boot consoles when necessary").
>> 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.
>>
>> Fixes 81cc26f2bd11 ("printk: only unregister boot consoles when necessary").
>> Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
> well... hm. don't specify `keep_bootcon' if your bootcon access initdata?
> 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()).
>
> 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")?

Sure, but as a user, how are you supposed to know that? And if your 
kernel is configured with SLAB poisoning, when the earlycon resident in 
initdata is freed all of it's text and data is replaced with a poison 
value, meaning it definitely won't work anymore anyway, and will instead 
likely just panic the kernel, often with a completely meaningless 
message (I know, because that's how I tracked down the need for this patch).
With this patch, the earlycon will only be unregistered if it is in the 
init section and will no longer be functional anyway. Perhaps it would 
be better to change the structure to make that more explicit, something 
like:

                 if ((con->flags & CON_BOOT) && 
init_section_intersects(con, sizeof(*con))) {
/*
                          * Make sure to unregister boot consoles whose 
data
                          * resides in the init section before the init 
section
                          * is discarded. Boot consoles whose data will 
stick
                          * around will automatically be unregistered 
when the
                          * proper console replaces them.
*/
unregister_console(con);
                 }

Thanks,
Matt

>
>
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index a1db38abac5b..b5411f44eed4 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -2660,7 +2660,7 @@ static int __init printk_late_init(void)
>>   	int ret;
>>   
>>   	for_each_console(con) {
>> -		if (!keep_bootcon && con->flags & CON_BOOT) {
>> +		if (con->flags & CON_BOOT) {
>>   			/*
>>   			 * Make sure to unregister boot consoles whose data
>>   			 * resides in the init section before the init section
> what about bootconsoles that are still not getting de-registered due
> to keep_bootcon in register_console()?
>
> 	-ss

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

[Petr Mladek]

Hi all,

let's first make sure that we understand the code the same way.

On Fri 2017-07-07 08:58:01, Matt Redfearn wrote:
> On 07/07/17 05:45, Sergey Senozhatsky wrote:
> >On (07/06/17 11:38), Matt Redfearn wrote:
> >>Commit 4c30c6f566c0 ("kernel/printk: do not turn off bootconsole in
> >>printk_late_init() if keep_bootcon") added a check on keep_bootcon to
> >>ensure that boot consoles were kept around until the real console is
> >>registered.
> >>
> >>This can lead to problems if the boot console data and code are in the
> >>init section, since it can be freed before the boot console is
> >>deregistered.

Yes.

> >>This was fixed by commit 81cc26f2bd11 ("printk: only
> >>unregister boot consoles when necessary").

This does not make sense to me in this context. This commit has an
effect only when keep_bootcon is false. While the commit 4c30c6f566c0
("kernel/printk: do not turn off boot console in printk_late_init()
if keep_bootcon") _causes problems only_ when keep_bootcon is true.

What I want to say is that the two commits have effect when
keep_bootcon has different value. Therefore they could not fix
each other.

> >>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'.


> >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".

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

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.

Matt, is there any chance that you look into this possibility?

Best Regards,
Petr

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

[Matt Redfearn]

Hi Petr,


On 11/07/17 13:43, Petr Mladek wrote:
> Hi all,
>
> let's first make sure that we understand the code the same way.
>
> On Fri 2017-07-07 08:58:01, Matt Redfearn wrote:
>> On 07/07/17 05:45, Sergey Senozhatsky wrote:
>>> On (07/06/17 11:38), Matt Redfearn wrote:
>>>> Commit 4c30c6f566c0 ("kernel/printk: do not turn off bootconsole in
>>>> printk_late_init() if keep_bootcon") added a check on keep_bootcon to
>>>> ensure that boot consoles were kept around until the real console is
>>>> registered.
>>>>
>>>> This can lead to problems if the boot console data and code are in the
>>>> init section, since it can be freed before the boot console is
>>>> deregistered.
> Yes.
>
>>>> This was fixed by commit 81cc26f2bd11 ("printk: only
>>>> unregister boot consoles when necessary").
> This does not make sense to me in this context. This commit has an
> effect only when keep_bootcon is false. While the commit 4c30c6f566c0
> ("kernel/printk: do not turn off boot console in printk_late_init()
> if keep_bootcon") _causes problems only_ when keep_bootcon is true.
>
> What I want to say is that the two commits have effect when
> keep_bootcon has different value. Therefore they could not fix
> each other.

Yeah, what I meant was the situation of the boot console being freed 
while still active (if keep_bootcon is false) was fixed by 81cc26f2bd11. 
After 4c30c6f566c0, if keep_bootcon is true, then you run in to the 
issue I had. The boot console is still active. It's code & data are 
freed and poisoned with the SLAB poison. Some of that section is then 
executed when the boot console is accessed. By happy coincidence(!), on 
MIPS, the poison value 0xCCCCCCCC happens to be a valid opcode with no 
side effect (it's a memory prefetch), so the CPU merrily runs off into 
the weeds executing Mb's worth of them before finally executing 
something that causes an exception of some kind. You get a kernel panic 
with a cause and address that make no sense and have to backtrack trying 
to figure out at what point the actual error occurred, and get nicely 
sidetracked form the issue you were actually trying to debug that 
necessitated keep_bootcon ;-).

>
>>>> 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'.
>
>
>>> 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".
>
> I am not comfortable with removing the only way to debug some type
> of bugs. But the current state is broken as well.
>
> 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.

This seems like the most reasonable way forward to me as well, though 
sadly will lead to some post-init kernel bloat.

I still think, however, that this patch is a reasonable change to make. 
Though perhaps some warning (if keep_bootcon is active?) would be 
appropriate so that a user knows that the boot console is being 
unregistered (because it may no longer be functional after being freed) 
and therefore silence can be expected - and we know that a driver needs 
fixing. We can then proceed as you suggest and migrate away from placing 
early console drivers which will not already have an active real console 
in the init section.
My other patch, placing serial earlycon in the init section is then the 
wrong way round and we need to remove __init from e.g. early8250 and so on.

>
> Matt, is there any chance that you look into this possibility?

Sure, I can look at fixing up console drivers which we use on the MIPS 
architecture.

Thanks,
Matt

>
> Best Regards,
> Petr

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

[Petr Mladek]

On Tue 2017-07-11 15:41:50, Matt Redfearn wrote:
> On 11/07/17 13:43, Petr Mladek wrote:
> >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.
> 
> This seems like the most reasonable way forward to me as well,
> though sadly will lead to some post-init kernel bloat.
> 
> I still think, however, that this patch is a reasonable change to
> make.

The thing is that this patch "silently" makes the keep_bootcon
option almost unusable.

The remaining use in register_console() does not make much sense.
It keeps the boot console after the preferred real one was registered.


> My other patch, placing serial earlycon in the init section is then
> the wrong way round and we need to remove __init from e.g. early8250
> and so on.

Yup.

> >Matt, is there any chance that you look into this possibility?
> 
> Sure, I can look at fixing up console drivers which we use on the
> MIPS architecture.

I have discussed this with my colleagues. It seems that implementation
of keep_bootcon option is pretty broken. It allows to debug broken
system using unreliable consoles (already freed). Therefore enabling
the option adds more problems on its own.

It would be nice to fix it. But it looks rather complicated. There
are several possibilities that are more or less hacky. It is hard
to guess the best solution if we do not know who is using it, how
it helps or sucks for them.

Before we spend a lot of time on this. Let me first try to send an RFC
to remove the feature and put more people into CC. It will either
be accepted or we will get arguments that would justify the needed
changes to fix it.

Best Regards,
Petr


PS: If you already stared migrating some consoles from the init
section, I would be interested into feedback about what problems
you did meet so far.

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

[Petr Mladek]

On Wed 2017-07-12 13:11:17, Petr Mladek wrote:
> On Tue 2017-07-11 15:41:50, Matt Redfearn wrote:
> > On 11/07/17 13:43, Petr Mladek wrote:
> > >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.
> > 
> > This seems like the most reasonable way forward to me as well,
> > though sadly will lead to some post-init kernel bloat.
> > 
> > I still think, however, that this patch is a reasonable change to
> > make.
> 
> The thing is that this patch "silently" makes the keep_bootcon
> option almost unusable.

I was wrong here. I thought that most early consoles used the init
section. It was mentioned somewhere and I looked a wrong way.
But this is not true. In fact, it seems that there are
only few of them. Most early consoles have struct console
and the write() callback in the normal section that is preserved.

Matt's patch and the keep_bootcon option makes sense to me
after all. Let me to resend Matt's patch with some small
improvements and one more patch that improves the check
of early consoles that use init section. I'll keep Matt
as the author of the first patch.

Best Regards,
Petr
 

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

[Matt Redfearn]

On 14/07/17 13:40, Petr Mladek wrote:
> On Wed 2017-07-12 13:11:17, Petr Mladek wrote:
>> On Tue 2017-07-11 15:41:50, Matt Redfearn wrote:
>>> On 11/07/17 13:43, Petr Mladek wrote:
>>>> 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.
>>> This seems like the most reasonable way forward to me as well,
>>> though sadly will lead to some post-init kernel bloat.
>>>
>>> I still think, however, that this patch is a reasonable change to
>>> make.
>> The thing is that this patch "silently" makes the keep_bootcon
>> option almost unusable.
> I was wrong here. I thought that most early consoles used the init
> section. It was mentioned somewhere and I looked a wrong way.
> But this is not true. In fact, it seems that there are
> only few of them. Most early consoles have struct console
> and the write() callback in the normal section that is preserved.
>
> Matt's patch and the keep_bootcon option makes sense to me
> after all. Let me to resend Matt's patch with some small
> improvements and one more patch that improves the check
> of early consoles that use init section. I'll keep Matt
> as the author of the first patch.

Thanks for looking into this and your fix Petr!

Matt

>
> Best Regards,
> Petr
>   

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

[Sergey Senozhatsky]

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