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.
'tty->port' will be set by the driver and if the driver has not yet done
it before we open the tty device we can get to this situation. By adding
some extra debug prints, I noticed that tty_port_link_device() is
initialising 'driver->ports[index]' just few microseconds after I
get the warning.
So, add one retry so that tty_init_dev() will return -EAGAIN on its first
try if 'tty->port' is not set yet, and then tty_open() will try to open
it again.
Signed-off-by: Sudip Mukherjee <sudipm.mukherjee@xxxxxxxxx>
---
drivers/tty/pty.c | 2 +-
drivers/tty/serdev/serdev-ttyport.c | 2 +-
drivers/tty/tty_io.c | 20 ++++++++++++++------
include/linux/tty.h | 3 ++-
4 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 00099a8439d2..22e8c40d9f9c 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -842,7 +842,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
mutex_lock(&tty_mutex);
- tty = tty_init_dev(ptm_driver, index);
+ tty = tty_init_dev(ptm_driver, index, 0);
/* The tty returned here is locked so we can safely
drop the mutex */
mutex_unlock(&tty_mutex);
diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index d1cdd2ab8b4c..1162b4202e80 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -109,7 +109,7 @@ static int ttyport_open(struct serdev_controller *ctrl)
struct ktermios ktermios;
int ret;
- tty = tty_init_dev(serport->tty_drv, serport->tty_idx);
+ tty = tty_init_dev(serport->tty_drv, serport->tty_idx, 0);
if (IS_ERR(tty))
return PTR_ERR(tty);
serport->tty = tty;
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index cb6370906a6d..e1b2086317fb 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -1295,6 +1295,7 @@ static int tty_reopen(struct tty_struct *tty)
* tty_init_dev - initialise a tty device
* @driver: tty driver we are opening a device on
* @idx: device index
+ * @retry: retry count if driver has not set tty->port yet
* @ret_tty: returned tty structure
*
* Prepare a tty device. This may not be a "new" clean device but
@@ -1315,7 +1316,8 @@ static int tty_reopen(struct tty_struct *tty)
* relaxed for the (most common) case of reopening a tty.
*/
-struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx)
+struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx,
+ int retry)
{
struct tty_struct *tty;
int retval;
@@ -1344,6 +1346,10 @@ struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx)
if (!tty->port)
tty->port = driver->ports[idx];
+ if (!tty->port && retry) {
+ retval = -EAGAIN;
+ goto err_release_driver;
+ }
WARN_RATELIMIT(!tty->port,
"%s: %s driver does not set tty->port. This will crash the kernel later. Fix the driver!\n",
@@ -1366,6 +1372,8 @@ struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx)
/* Return the tty locked so that it cannot vanish under the caller */
return tty;
+err_release_driver:
+ tty_driver_remove_tty(driver, tty);
err_free_tty:
tty_unlock(tty);
free_tty_struct(tty);
@@ -1910,7 +1918,7 @@ struct tty_struct *tty_kopen(dev_t device)
tty_kref_put(tty);
tty = ERR_PTR(-EBUSY);
} else { /* tty_init_dev returns tty with the tty_lock held */
- tty = tty_init_dev(driver, index);
+ tty = tty_init_dev(driver, index, 0);
if (IS_ERR(tty))
goto out;
tty_port_set_kopened(tty->port, 1);
@@ -1937,7 +1945,7 @@ EXPORT_SYMBOL_GPL(tty_kopen);
* - concurrent tty driver removal w/ lookup
* - concurrent tty removal from driver table
*/
-static struct tty_struct *tty_open_by_driver(dev_t device,
+static struct tty_struct *tty_open_by_driver(dev_t device, int retry,
struct file *filp)
{
struct tty_struct *tty;
@@ -1981,7 +1989,7 @@ static struct tty_struct *tty_open_by_driver(dev_t device,
tty = ERR_PTR(retval);
}
} else { /* Returns with the tty_lock held for now */
- tty = tty_init_dev(driver, index);
+ tty = tty_init_dev(driver, index, retry);
mutex_unlock(&tty_mutex);
}
out:
@@ -2016,7 +2024,7 @@ static struct tty_struct *tty_open_by_driver(dev_t device,
static int tty_open(struct inode *inode, struct file *filp)
{
struct tty_struct *tty;
- int noctty, retval;
+ int noctty, retval, retry = 1;
dev_t device = inode->i_rdev;
unsigned saved_flags = filp->f_flags;
@@ -2029,7 +2037,7 @@ static int tty_open(struct inode *inode, struct file *filp)
tty = tty_open_current_tty(device, filp);
if (!tty)
- tty = tty_open_by_driver(device, filp);
+ tty = tty_open_by_driver(device, retry--, filp);
if (IS_ERR(tty)) {
tty_free_file(filp);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index bfa4e2ee94a9..2f74fc138e6a 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -559,7 +559,8 @@ extern struct tty_struct *alloc_tty_struct(struct tty_driver *driver, int idx);
extern int tty_alloc_file(struct file *file);
extern void tty_add_file(struct tty_struct *tty, struct file *file);
extern void tty_free_file(struct file *file);
-extern struct tty_struct *tty_init_dev(struct tty_driver *driver, int idx);
+extern struct tty_struct *tty_init_dev(struct tty_driver *driver,
+ int idx, int retry);
extern void tty_release_struct(struct tty_struct *tty, int idx);
extern int tty_release(struct inode *inode, struct file *filp);
extern void tty_init_termios(struct tty_struct *tty);
--
2.11.0
