All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
To: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Guenter Roeck <linux@roeck-us.net>,
	Shreyas Joshi <shreyas.joshi@biamp.com>,
	rostedt@goodmis.org, shreyasjoshi15@gmail.com,
	linux-kernel@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH] printk: handle blank console arguments passed in.
Date: Wed, 7 Oct 2020 21:30:44 +0900	[thread overview]
Message-ID: <20201007123044.GA509@jagdpanzerIV.localdomain> (raw)
In-Reply-To: <20201007072853.GF32369@alley>

On (20/10/07 09:28), Petr Mladek wrote:
> 
> 		/*
> 		 * Dirty hack to prevent using any console with tty
> 		 * binding as a fallback and adding the empty
> 		 * name into console_cmdline array.
> 		 */
> 		preferred_console = MAX_CMDLINECONSOLES;

Let me dump my findings so far. I still don't understand what exactly
crashes the laptop (blank screen is not very helpful).

So, things start with the "preferred_console = -1". In console_setup()
we call __add_preferred_console(). Since we have no consoles, the
name matching loop is not executed, and console selection counter remains
at 0. After the loop, despite the fact that we don't have the console
(`name' is empty), we still set `preferred_console', to 0. This affects
register_console(). Since we have `preferred_console >= 0' we don't
execute the newcon->setup(), but, more importantly, we don't set the
newcon->flags |= CON_ENABLED. Now, we call try_enable_new_console():
since there are no consoles, the ->match() loop is not executed.
newcone does not have CON_ENABLED set, so try_enable_new_console()
returns -ENOENT. Both for user_specified=true and for fallback
user_specified=false cases. At this point we hit error-return path
from register_console() - we don't add newcon to the list of console
drivers. The console drivers list, thus, remains empty. So far so good.

Now. Things get strange in init/main.c

We have that kernel_init_freeable()->console_on_rootfs() control path.

console_on_rootfs() attempts to filp_open()->tty_open() /dev/console.
This ends up in printk's console_device(), which iterates the list of
console drivers and returns associated console->device back to tty. The
problem is that console drivers list is empty, so the function returns
NULL, and filp_open("/dev/console") fails. But the console_on_rootfs()
comment says that this function should never fail (!). This sort of
makes me wonder if "console=" is actually legal.

What this filp_open() failure means in particular, is that we never
create stdin/out/err fds, because we error-out and don't invoke
init_dup(file).

Things look different in older kernels. For instance, even in 5.4
the corresponding code looks as follows:

	/* Open the /dev/console on the rootfs, this should never fail */
	if (ksys_open((const char __user *) "/dev/console", O_RDWR, 0) < 0)
		pr_err("Warning: unable to open an initial console.\n");

	(void) ksys_dup(0);
	(void) ksys_dup(0);

Somehow, the fact that we don't init_dup(file) causes problems on my
laptop, but, at the moment, I can't tell exactly where. Perhaps more
experienced people will be like "darn, this is trivial, the problem is
here, here and there".

Hint: I can crash my laptop when I remove the "console=" boot param and
comment out init_dup(file) calls in console_on_rootfs().

I guess the problem is somewhat related to missing stdin/out/err fds.

Any ideas?

	-ss

  reply	other threads:[~2020-10-07 12:30 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-09  5:29 [PATCH] printk: handle blank console arguments passed in Shreyas Joshi
2020-03-17  1:34 ` Shreyas Joshi
2020-03-17  1:39   ` Steven Rostedt
2020-03-17  2:01     ` Sergey Senozhatsky
2020-03-17  2:17       ` Shreyas Joshi
2020-03-17  8:22         ` Sergey Senozhatsky
2020-05-22  6:46   ` Shreyas Joshi
2020-05-22  6:53   ` Shreyas Joshi
2020-05-22 10:00     ` Petr Mladek
2020-10-06  2:59       ` Sergey Senozhatsky
2020-10-06  3:35         ` Guenter Roeck
2020-10-06  5:08           ` Greg Kroah-Hartman
2020-10-06 11:17             ` Guenter Roeck
2020-10-06  6:59           ` Sergey Senozhatsky
2020-10-06  9:54             ` Petr Mladek
2020-10-06 13:33               ` Sergey Senozhatsky
2020-10-06 14:22                 ` Guenter Roeck
2020-10-06 16:08                   ` Sergey Senozhatsky
2020-10-06  9:52           ` Petr Mladek
2020-10-06 10:45             ` Guenter Roeck
2020-10-06 13:43               ` Petr Mladek
2020-10-06 16:35                 ` Petr Mladek
2020-10-06 17:15                   ` Sergey Senozhatsky
2020-10-07  7:28                     ` Petr Mladek
2020-10-07 12:30                       ` Sergey Senozhatsky [this message]
2020-10-07 14:39                         ` Sergey Senozhatsky
2020-10-07 15:57                         ` Guenter Roeck
2020-10-07 16:29                           ` Sergey Senozhatsky
2020-10-08  5:52                             ` Sergey Senozhatsky
2020-10-08  9:01                               ` Petr Mladek
2020-10-08 10:56                                 ` Sergey Senozhatsky
2020-10-08 12:05                                 ` Sergey Senozhatsky
2020-10-22 11:38                               ` Petr Mladek
2020-10-22 13:32                                 ` Sergey Senozhatsky
2020-10-08  8:50                         ` Petr Mladek
2020-10-08 12:20                           ` Sergey Senozhatsky
2020-10-08 12:37                             ` Sergey Senozhatsky
2020-05-15 15:24 ` Petr Mladek
2020-05-20 11:50   ` Sergey Senozhatsky
2020-05-22  6:40     ` Shreyas Joshi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201007123044.GA509@jagdpanzerIV.localdomain \
    --to=sergey.senozhatsky@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=shreyas.joshi@biamp.com \
    --cc=shreyasjoshi15@gmail.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.