Re: [PATCH v2] serial: core: Initialise spin lock before use in uart_configure_port()

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Mon, Jul 06, 2020 at 05:36:56PM +0100, Marc Zyngier wrote:
> On 2020-07-06 15:35, Andy Shevchenko 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.

...

> > +static inline void uart_any_port_spin_lock_init(struct uart_port *port)
> 
> nit: __uart_port_spin_lock_init() looks like a better name to me
> (as a primitive of uart_port_spin_lock_init). You can also drop
> the inline which doesn't mean much these days (and this isn't
> a hot path).

Actually I was thinking about that name, but decided to go without underscores
(b/c some developers like this approach, some like that one). I'm fine with
your proposal nevertheless.

> > +		/*
> > +		 * 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_any_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()
> 
> Otherwise looks OK to me (having tested an earlier version).
> With the above addressed:
> 
> Acked-by: Marc Zyngier <maz@kernel.org>

Thanks!

-- 
With Best Regards,
Andy Shevchenko