linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] serial: core: Initialise spin lock before use in uart_configure_port()
@ 2020-07-06 21:49 Andy Shevchenko
  2020-07-07  7:51 ` Prabhakar Mahadev Lad
  2020-07-10 13:12 ` Anatoly Pugachev
  0 siblings, 2 replies; 3+ messages in thread
From: Andy Shevchenko @ 2020-07-06 21:49 UTC (permalink / raw)
  To: Greg Kroah-Hartman, linux-serial
  Cc: Andy Shevchenko, Marc Zyngier, Lad Prabhakar, Guenter Roeck,
	Anatoly Pugachev, Tony Lindgren, Geert Uytterhoeven

The comment near to uart_port_spin_lock_init() says:

  Ensure that the serial console lock is initialised early.
  If this port is a console, then the spinlock is already initialised.

and there is nothing about enabled or disabled consoles. The commit
a3cb39d258ef ("serial: core: Allow detach and attach serial device
for console") made a change, which follows the comment, and also to
prevent reinitialisation of the lock in use, when user detaches and
attaches back the same console device. But this change discovers
another issue, that uart_add_one_port() tries to access a spin lock
that now may be uninitialised. This happens when a driver expects
the serial core to register a console on its behalf. In this case
we must initialise a spin lock before use.

Fixes: a3cb39d258ef ("serial: core: Allow detach and attach serial device for console")
Reported-by: Marc Zyngier <maz@kernel.org>
Reported-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reported-by: Guenter Roeck <linux@roeck-us.net>
Reported-by: Anatoly Pugachev <matorola@gmail.com>
Acked-by: Marc Zyngier <maz@kernel.org>
Tested-by: Tony Lindgren <tony@atomide.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v3: renamed lock init primitive and dropped inline (Marc), added tags (Marc, Tony)
 drivers/tty/serial/serial_core.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 3cc183acf7ba..309fa42b0861 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1915,6 +1915,12 @@ static inline bool uart_console_enabled(struct uart_port *port)
 	return uart_console(port) && (port->cons->flags & CON_ENABLED);
 }
 
+static void __uart_port_spin_lock_init(struct uart_port *port)
+{
+	spin_lock_init(&port->lock);
+	lockdep_set_class(&port->lock, &port_lock_key);
+}
+
 /*
  * Ensure that the serial console lock is initialised early.
  * If this port is a console, then the spinlock is already initialised.
@@ -1924,8 +1930,7 @@ static inline void uart_port_spin_lock_init(struct uart_port *port)
 	if (uart_console(port))
 		return;
 
-	spin_lock_init(&port->lock);
-	lockdep_set_class(&port->lock, &port_lock_key);
+	__uart_port_spin_lock_init(port);
 }
 
 #if defined(CONFIG_SERIAL_CORE_CONSOLE) || defined(CONFIG_CONSOLE_POLL)
@@ -2371,6 +2376,13 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
 		/* Power up port for set_mctrl() */
 		uart_change_pm(state, UART_PM_STATE_ON);
 
+		/*
+		 * If this driver supports console, and it hasn't been
+		 * successfully registered yet, initialise spin lock for it.
+		 */
+		if (port->cons && !(port->cons->flags & CON_ENABLED))
+			__uart_port_spin_lock_init(port);
+
 		/*
 		 * Ensure that the modem control lines are de-activated.
 		 * keep the DTR setting that is set in uart_set_options()
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* RE: [PATCH v3] serial: core: Initialise spin lock before use in uart_configure_port()
  2020-07-06 21:49 [PATCH v3] serial: core: Initialise spin lock before use in uart_configure_port() Andy Shevchenko
@ 2020-07-07  7:51 ` Prabhakar Mahadev Lad
  2020-07-10 13:12 ` Anatoly Pugachev
  1 sibling, 0 replies; 3+ messages in thread
From: Prabhakar Mahadev Lad @ 2020-07-07  7:51 UTC (permalink / raw)
  To: Andy Shevchenko, Greg Kroah-Hartman, linux-serial
  Cc: Marc Zyngier, Guenter Roeck, Anatoly Pugachev, Tony Lindgren,
	Geert Uytterhoeven

Hi Andy,

Thank you for the patch.

> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Sent: 06 July 2020 22:49
> To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; linux-serial@vger.kernel.org
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Marc Zyngier <maz@kernel.org>; Prabhakar Mahadev Lad
> <prabhakar.mahadev-lad.rj@bp.renesas.com>; Guenter Roeck <linux@roeck-us.net>; Anatoly Pugachev <matorola@gmail.com>; Tony
> Lindgren <tony@atomide.com>; Geert Uytterhoeven <geert@linux-m68k.org>
> Subject: [PATCH v3] serial: core: Initialise spin lock before use in uart_configure_port()
>
> The comment near to uart_port_spin_lock_init() says:
>
>   Ensure that the serial console lock is initialised early.
>   If this port is a console, then the spinlock is already initialised.
>
> and there is nothing about enabled or disabled consoles. The commit
> a3cb39d258ef ("serial: core: Allow detach and attach serial device
> for console") made a change, which follows the comment, and also to
> prevent reinitialisation of the lock in use, when user detaches and
> attaches back the same console device. But this change discovers
> another issue, that uart_add_one_port() tries to access a spin lock
> that now may be uninitialised. This happens when a driver expects
> the serial core to register a console on its behalf. In this case
> we must initialise a spin lock before use.
>
> Fixes: a3cb39d258ef ("serial: core: Allow detach and attach serial device for console")
> Reported-by: Marc Zyngier <maz@kernel.org>
> Reported-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Reported-by: Anatoly Pugachev <matorola@gmail.com>
> Acked-by: Marc Zyngier <maz@kernel.org>
> Tested-by: Tony Lindgren <tony@atomide.com>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v3: renamed lock init primitive and dropped inline (Marc), added tags (Marc, Tony)
>  drivers/tty/serial/serial_core.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
--Prabhakar

> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 3cc183acf7ba..309fa42b0861 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -1915,6 +1915,12 @@ static inline bool uart_console_enabled(struct uart_port *port)
>  return uart_console(port) && (port->cons->flags & CON_ENABLED);
>  }
>
> +static void __uart_port_spin_lock_init(struct uart_port *port)
> +{
> +spin_lock_init(&port->lock);
> +lockdep_set_class(&port->lock, &port_lock_key);
> +}
> +
>  /*
>   * Ensure that the serial console lock is initialised early.
>   * If this port is a console, then the spinlock is already initialised.
> @@ -1924,8 +1930,7 @@ static inline void uart_port_spin_lock_init(struct uart_port *port)
>  if (uart_console(port))
>  return;
>
> -spin_lock_init(&port->lock);
> -lockdep_set_class(&port->lock, &port_lock_key);
> +__uart_port_spin_lock_init(port);
>  }
>
>  #if defined(CONFIG_SERIAL_CORE_CONSOLE) || defined(CONFIG_CONSOLE_POLL)
> @@ -2371,6 +2376,13 @@ uart_configure_port(struct uart_driver *drv, struct uart_state *state,
>  /* Power up port for set_mctrl() */
>  uart_change_pm(state, UART_PM_STATE_ON);
>
> +/*
> + * If this driver supports console, and it hasn't been
> + * successfully registered yet, initialise spin lock for it.
> + */
> +if (port->cons && !(port->cons->flags & CON_ENABLED))
> +__uart_port_spin_lock_init(port);
> +
>  /*
>   * Ensure that the modem control lines are de-activated.
>   * keep the DTR setting that is set in uart_set_options()
> --
> 2.27.0



Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] serial: core: Initialise spin lock before use in uart_configure_port()
  2020-07-06 21:49 [PATCH v3] serial: core: Initialise spin lock before use in uart_configure_port() Andy Shevchenko
  2020-07-07  7:51 ` Prabhakar Mahadev Lad
@ 2020-07-10 13:12 ` Anatoly Pugachev
  1 sibling, 0 replies; 3+ messages in thread
From: Anatoly Pugachev @ 2020-07-10 13:12 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Greg Kroah-Hartman, linux-serial, Marc Zyngier, Lad Prabhakar,
	Guenter Roeck, Tony Lindgren, Geert Uytterhoeven

On Tue, Jul 7, 2020 at 12:49 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> The comment near to uart_port_spin_lock_init() says:
>
>   Ensure that the serial console lock is initialised early.
>   If this port is a console, then the spinlock is already initialised.
>
> and there is nothing about enabled or disabled consoles. The commit
> a3cb39d258ef ("serial: core: Allow detach and attach serial device
> for console") made a change, which follows the comment, and also to
> prevent reinitialisation of the lock in use, when user detaches and
> attaches back the same console device. But this change discovers
> another issue, that uart_add_one_port() tries to access a spin lock
> that now may be uninitialised. This happens when a driver expects
> the serial core to register a console on its behalf. In this case
> we must initialise a spin lock before use.
>
> Fixes: a3cb39d258ef ("serial: core: Allow detach and attach serial device for console")
> Reported-by: Marc Zyngier <maz@kernel.org>
> Reported-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Reported-by: Anatoly Pugachev <matorola@gmail.com>
> Acked-by: Marc Zyngier <maz@kernel.org>
> Tested-by: Tony Lindgren <tony@atomide.com>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v3: renamed lock init primitive and dropped inline (Marc), added tags (Marc, Tony)
>  drivers/tty/serial/serial_core.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c

Andy,

probably late, but tested on my sparc64 ldom - works (boots).
Thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-07-10 13:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-06 21:49 [PATCH v3] serial: core: Initialise spin lock before use in uart_configure_port() Andy Shevchenko
2020-07-07  7:51 ` Prabhakar Mahadev Lad
2020-07-10 13:12 ` Anatoly Pugachev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).