| 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
| ||
|
Message-ID: <CAHQZ30BwXeZNS1BZCQa5Nyb6S7akXvAqnhXR8w2Cj6LnMG6kGg@mail.gmail.com>
Date: Wed, 21 Aug 2024 09:30:17 -0600
From: Raul Rangel <rrangel@...omium.org>
To: Petr Mladek <pmladek@...e.com>
Cc: Randy Dunlap <rdunlap@...radead.org>, Mario Limonciello <mario.limonciello@....com>,
linux-kernel@...r.kernel.org, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
kramasub@...omium.org, Alexander Potapenko <glider@...gle.com>,
Andrew Morton <akpm@...ux-foundation.org>, Ard Biesheuvel <ardb@...nel.org>,
"Jason A. Donenfeld" <Jason@...c4.com>, Li Zhe <lizhe.67@...edance.com>,
"Liam R. Howlett" <Liam.Howlett@...cle.com>, Mark Rutland <mark.rutland@....com>,
Will Deacon <will@...nel.org>, Wolfram Sang <wsa+renesas@...g-engineering.com>,
Zhou jie <zhoujie@...china.com>
Subject: Re: [PATCH] init: Don't proxy console= to earlycon
On Wed, Aug 21, 2024 at 8:38 AM Petr Mladek <pmladek@...e.com> wrote:
>
> On Thu 2024-08-08 11:30:53, Raul Rangel wrote:
> > On Fri, Jul 28, 2023 at 11:57 AM Raul Rangel <rrangel@...omium.org> wrote:
> > > > Your patch is good then. Well, would you mind to add a comment into
> > > > the code and make the commit message more clear even for dummies like
> > > > me?
> > > >
> > > > Something like the patch below. It would be better to split it into
> > > > two:
> > > >
> > > > + 1st shuffling the check and adding the first part of the comment
> > > > + 2nd fixing the case with empty console= options.
> > > >
> > > > I could prepare the patchset. I would keep your SOB for the 2nd patch
> > > > if you agreed.
> > >
> > > Yes, feel free. Thanks!
> > >
> >
> > Hey Petr,
> > I was just following up on this. Were you going to prepare the two patches?
>
> I have been quite overloaded last two years. It would help me a lot if
> you could prepare the two patches and send them for review.
>
> Thanks for re-opening this. It has already fallen through cracks on my
> side /o\.
>
> Best Regards,
> Petr
No worries, we were just going through our tech debt backlog and this
patch came up.
Thinking about this a little bit more, I'm wondering if we can clean
up the hack. Would something like the following patch work? My thought
is we declare an early handler for `console=`, so we can handle the
`console=uartXXXX` case, and leave the non-early handler in place to
handle the `console=ttyXX` cases.
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index a5f380584cdae7..7a48fe6fc0dffc 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -241,6 +241,21 @@ static int __init param_setup_earlycon(char *buf)
}
early_param("earlycon", param_setup_earlycon);
+static int __init param_setup_earlycon_console_alias(char *buf)
+{
+ /* We only want to handle `console=XXXX` in this handler. We leave
+ `console` and `console=` to be handled later by the non-early printk
+ handler.
+ */
+ if (!buf || !buf[0]) {
+ return 0;
+ }
+
+ /* `console=uartXXXX` is actually an alias for `earlycon=uartXXXX`. */
+ return param_setup_earlycon(buf);
+}
+early_param("console=", param_setup_earlycon_console_alias);
+
#ifdef CONFIG_OF_EARLY_FLATTREE
int __init of_setup_earlycon(const struct earlycon_id *match,
diff --git a/init/main.c b/init/main.c
index 8e1f6127e172c3..b6f0cb424176da 100644
--- a/init/main.c
+++ b/init/main.c
@@ -740,9 +740,7 @@ static int __init do_early_param(char *param, char *val,
const struct obs_kernel_param *p;
for (p = __setup_start; p < __setup_end; p++) {
- if ((p->early && parameq(param, p->str)) ||
- (strcmp(param, "console") == 0 &&
- strcmp(p->str, "earlycon") == 0 && val && val[0])) {
+ if ((p->early && parameq(param, p->str))) {
if (p->setup_func(val) != 0)
pr_warn("Malformed early option '%s'\n", param);
}
>
>
> > > > Here is the proposal:
> > > >
> > > > From d2fb7c54bed4c67df229c56fcc1b0af83ada5227 Mon Sep 17 00:00:00 2001
> > > > From: Raul E Rangel <rrangel@...omium.org>
> > > > Date: Fri, 7 Jul 2023 19:17:25 -0600
> > > > Subject: [PATCH] init: Don't proxy the empty console= options to earlycon
> > > >
> > > > Right now we are proxying the `console=XXX` command line args to the
> > > > param_setup_earlycon. This is done because the early consoles used to
> > > > be enabled via the `console` parameter.
> > > >
> > > > The `earlycon` parameter was added later by the commit 18a8bd949d6adb311
> > > > ("serial: convert early_uart to earlycon for 8250"). It allowed to
> > > > setup the early consoles using another callback. And it was indeed
> > > > more self-explanatory and cleaner approach.
> > > >
> > > > As a result, for example, the following parameters have the same effect:
> > > >
> > > > console=uart[8250],mmio,<addr>[,options]
> > > > earlycon=uart[8250],mmio,<addr>[,options]
> > > >
> > > > Nevertheless, `console` and `earlycon` parameters behave different when
> > > > the options do not match an early console. setup_earlycon() accepts only
> > > > console names in __earlycon_table. Also the empty options are handled
> > > > differently:
> > > >
> > > > + When `earlycon=` or just `earlycon` is specified on the command line,
> > > > param_setup_earlycon() tries to enable an early console via the SPCR
> > > > table or the device tree.
> > > >
> > > > + When `console=` is specified on the command line, it's intention is to
> > > > disable the console.
> > > >
> > > > Here comes the problem. The current code calls param_setup_earlycon()
> > > > even when `console=` is used with no options. As a result, it might
> > > > enable the early console when it is defined in the SPCR table or
> > > > a device tree. This is obviously not what users want.
> > > >
> > > > The early console should be enabled via SPCR or DT only when `earlycon`
> > > > is used explicitly with no options.
> > > >
> > > > Signed-off-by: Raul E Rangel <rrangel@...omium.org>
> > > > [pmladek@...e.com: Add comments and more details into the commit message]
> > > > ---
> > > > init/main.c | 20 ++++++++++++++++----
> > > > 1 file changed, 16 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/init/main.c b/init/main.c
> > > > index ad920fac325c..3b059e316e62 100644
> > > > --- a/init/main.c
> > > > +++ b/init/main.c
> > > > @@ -733,13 +733,25 @@ static int __init do_early_param(char *param, char *val,
> > > > const struct obs_kernel_param *p;
> > > >
> > > > for (p = __setup_start; p < __setup_end; p++) {
> > > > - if ((p->early && parameq(param, p->str)) ||
> > > > - (strcmp(param, "console") == 0 &&
> > > > - strcmp(p->str, "earlycon") == 0)
> > > > - ) {
> > > > + if (p->early && parameq(param, p->str)) {
> > > > if (p->setup_func(val) != 0)
> > > > pr_warn("Malformed early option '%s'\n", param);
> > > > }
> > > > +
> > > > + /*
> > > > + * Early consoles can be historically enabled also when earlycon
> > > > + * specific options are passed via console=[earlycon options]
> > > > + * parameter.
> > > > + *
> > > > + * Do not try it with an empty value. "console=" is used to
> > > > + * disable real normal consoles. While "earlycon" is used to
> > > > + * enable an early console via SPCR or DT.
> > > > + */
> > > > + if (strcmp(param, "console") == 0 && val && val[0] &&
> > > > + strcmp(p->str, "earlycon") == 0) {
> > > > + if (p->setup_func(val) != 0)
> > > > + pr_warn("Failed to setup earlycon via console=%s\n", val);
> > > > + }
> > > > }
> > > > /* We accept everything at this stage. */
> > > > return 0;
> > > > --
> > > > 2.35.3
> > > >
> > >
> > > Signed-off-by: Raul E Rangel <rrangel@...omium.org>
> >
> > Thanks,
> > Raul
Powered by blists - more mailing lists