linux-kernel - Re: [PATCH RFT 4/6] serial: sh-sci: Do not probe the serial port if its slot in sci

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

Message-ID: <CAMuHMdX8Sg4heCHuJXnMknXkY3iirjnTvTZ5fPLSN4kZ66dA2Q@mail.gmail.com>
Date: Thu, 19 Dec 2024 15:21:41 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Claudiu <claudiu.beznea@...on.dev>
Cc: gregkh@...uxfoundation.org, jirislaby@...nel.org, 
	wsa+renesas@...g-engineering.com, prabhakar.mahadev-lad.rj@...renesas.com, 
	lethal@...ux-sh.org, g.liakhovetski@....de, groeck@...omium.org, 
	mka@...omium.org, ulrich.hecht+renesas@...il.com, ysato@...rs.sourceforge.jp, 
	linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org, 
	linux-renesas-soc@...r.kernel.org, 
	Claudiu Beznea <claudiu.beznea.uj@...renesas.com>, stable@...r.kernel.org
Subject: Re: [PATCH RFT 4/6] serial: sh-sci: Do not probe the serial port if
 its slot in sci_ports[] is in use

Hi Claudiu,

On Wed, Dec 4, 2024 at 4:58 PM Claudiu <claudiu.beznea@...on.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
>
> In the sh-sci driver, sci_ports[0] is used by earlycon. If the earlycon is
> still active when sci_probe() is called and the new serial port is supposed
> to map to sci_ports[0], return -EBUSY to prevent breaking the earlycon.
>
> This situation should occurs in debug scenarios, and users should be
> aware of the potential conflict.
>
> Fixes: 0b0cced19ab1 ("serial: sh-sci: Add CONFIG_SERIAL_EARLYCON support")
> Cc: stable@...r.kernel.org
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>

Thanks for your patch!

> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -158,6 +158,7 @@ struct sci_port {
>         bool has_rtscts;
>         bool autorts;
>         bool tx_occurred;
> +       bool earlycon;

This is only used in sci_ports[0], so it can be a single global flag,
instead of a flag embedded in each sci_port structure.

>  };
>
>  #define SCI_NPORTS CONFIG_SERIAL_SH_SCI_NR_UARTS
> @@ -3443,6 +3444,7 @@ static int sci_probe_single(struct platform_device *dev,
>  static int sci_probe(struct platform_device *dev)
>  {
>         struct plat_sci_port *p;
> +       struct resource *res;
>         struct sci_port *sp;
>         unsigned int dev_id;
>         int ret;
> @@ -3472,6 +3474,26 @@ static int sci_probe(struct platform_device *dev)
>         }
>
>         sp = &sci_ports[dev_id];
> +
> +       /*
> +        * In case:
> +        * - the probed port alias is zero (as the one used by earlycon), and
> +        * - the earlycon is still active (e.g., "earlycon keep_bootcon" in
> +        *   bootargs)

This is even true without "keep_bootcon", as nothing ever clears the
sci_port.earlycon flag once it is set.

> +        *
> +        * defer the probe of this serial. This is a debug scenario and the user
> +        * must be aware of it.
> +        *
> +        * Except when the probed port is the same as the earlycon port.
> +        */
> +
> +       res = platform_get_resource(dev, IORESOURCE_MEM, 0);
> +       if (!res)
> +               return -ENODEV;
> +
> +       if (sp->earlycon && res->start != sp->port.mapbase)
> +               return dev_err_probe(&dev->dev, -EBUSY, "sci_port[0] is used by earlycon!\n");
> +
>         platform_set_drvdata(dev, sp);
>
>         ret = sci_probe_single(dev, dev_id, p, sp);
> @@ -3568,6 +3590,7 @@ static int __init early_console_setup(struct earlycon_device *device,
>         port_cfg.type = type;
>         sci_ports[0].cfg = &port_cfg;
>         sci_ports[0].params = sci_probe_regmap(&port_cfg);
> +       sci_ports[0].earlycon = true;
>         port_cfg.scscr = sci_serial_in(&sci_ports[0].port, SCSCR);
>         sci_serial_out(&sci_ports[0].port, SCSCR,
>                        SCSCR_RE | SCSCR_TE | port_cfg.scscr);

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds