Messages in this thread Patch in this message | | | Date | Tue, 10 Dec 2019 11:41:47 +0000 | From | Sudip Mukherjee <> | Subject | Re: [PATCH v2 2/2] tty: use tty_init_dev_retry() to workaround a race condition |
| |
Hi Jiri,
On Fri, Nov 22, 2019 at 10:05:09AM +0100, Jiri Slaby wrote: > On 21. 11. 19, 22:01, Sudip Mukherjee wrote: > > Hi Greg, > > > > On Thu, Nov 21, 2019 at 05:41:38PM +0100, Greg Kroah-Hartman wrote: > >> On Thu, Nov 21, 2019 at 03:22:39PM +0000, Sudip Mukherjee wrote: > >>> There seems to be a race condition in tty drivers and I could see on > >>> many boot cycles a NULL pointer dereference as tty_init_dev() tries to > >>> do 'tty->port->itty = tty' even though tty->port is NULL. <snip> > >>> > >>> uart_add_one_port() registers the console, as soon as it registers, the > >>> userspace tries to use it and that leads to tty_open() but > >>> uart_add_one_port() has not yet done tty_port_link_device() and so > >>> tty->port is not yet configured when control reaches tty_init_dev(). > >> > >> Shouldn't we do tty_port_link_device() before uart_add_one_port() to > >> remove that race? Once you register the console, yes, tty_open() can > >> happen, so the driver had better be ready to go at that point in time. > >> > > > > But tty_port_link_device() is done by uart_add_one_port() itself. > > After registering the console uart_add_one_port() will call > > tty_port_register_device_attr_serdev() and tty_port_link_device() is > > called from this. Thats still tty core. > > Interferences of console vs tty code are ugly. Does it help to simply > put tty_port_link_device to uart_add_one_port before uart_configure_port?
sorry for the late response, got busy with an out-of-tree driver.
It fixes the problem if I put tty_port_link_device() before uart_configure_port(). Please check the attached patch and that completely fixes the problem. Do you want me to send a proper patch for it or do you want me to check more into it?
-- Regards Sudip diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 351843f847c0..006d478a63be 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -2820,6 +2820,7 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport) if (uport->cons && uport->dev) of_console_check(uport->dev->of_node, uport->cons->name, uport->line); + tty_port_link_device(port, drv->tty_driver, uport->line); uart_configure_port(drv, state, uport); port->console = uart_console(uport); diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c index a9e12b3bc31d..a4f85fc75539 100644 --- a/drivers/tty/tty_port.c +++ b/drivers/tty/tty_port.c @@ -89,7 +89,8 @@ void tty_port_link_device(struct tty_port *port, { if (WARN_ON(index >= driver->num)) return; - driver->ports[index] = port; + if (!driver->ports[index]) + driver->ports[index] = port; } EXPORT_SYMBOL_GPL(tty_port_link_device); | |