From 7171604ae7b3bbc738b6a4b7cd0ee73eb0d551d9 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Mon, 4 Jun 2012 13:35:28 +0200 Subject: PTY: remove one empty ops->remove Currently, there are two as a left-over from previous patches. Although we really need to provide an empty handler, we do not need two. So remove one of them. Signed-off-by: Jiri Slaby Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index 5505ffc91da4..4bcaf896fac4 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -557,18 +557,14 @@ err_free_tty: return -ENOMEM; } -static void ptm_unix98_remove(struct tty_driver *driver, struct tty_struct *tty) -{ -} - -static void pts_unix98_remove(struct tty_driver *driver, struct tty_struct *tty) +static void pty_unix98_remove(struct tty_driver *driver, struct tty_struct *tty) { } static const struct tty_operations ptm_unix98_ops = { .lookup = ptm_unix98_lookup, .install = pty_unix98_install, - .remove = ptm_unix98_remove, + .remove = pty_unix98_remove, .open = pty_open, .close = pty_close, .write = pty_write, @@ -585,7 +581,7 @@ static const struct tty_operations ptm_unix98_ops = { static const struct tty_operations pty_unix98_ops = { .lookup = pts_unix98_lookup, .install = pty_unix98_install, - .remove = pts_unix98_remove, + .remove = pty_unix98_remove, .open = pty_open, .close = pty_close, .write = pty_write, -- cgit v1.2.3 From 5d249bc6a61e7a434c69e0d0becc77a803c8c5e8 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Mon, 4 Jun 2012 13:35:29 +0200 Subject: PTY: merge pty_install implementations There are currently two instances of code which handles PTY install. One for the legacy BSD PTY's, one for unix98's PTY's. Both of them are very similar and differ only in termios allocation and handling. Since we will need to allocate a tty_port at that place, this would require editing two places with the same pattern. Instead, let us move the implementation to one common place and call it from both places. Signed-off-by: Jiri Slaby Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 110 ++++++++++++++++++++++-------------------------------- 1 file changed, 45 insertions(+), 65 deletions(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index 4bcaf896fac4..881888f0a445 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -282,39 +282,53 @@ done: return 0; } -/* Traditional BSD devices */ -#ifdef CONFIG_LEGACY_PTYS - -static int pty_install(struct tty_driver *driver, struct tty_struct *tty) +static int pty_common_install(struct tty_driver *driver, struct tty_struct *tty, + bool legacy) { struct tty_struct *o_tty; int idx = tty->index; - int retval; + int retval = -ENOMEM; o_tty = alloc_tty_struct(); if (!o_tty) - return -ENOMEM; + goto err; if (!try_module_get(driver->other->owner)) { /* This cannot in fact currently happen */ - retval = -ENOMEM; goto err_free_tty; } initialize_tty_struct(o_tty, driver->other, idx); - /* We always use new tty termios data so we can do this - the easy way .. */ - retval = tty_init_termios(tty); - if (retval) - goto err_deinit_tty; - - retval = tty_init_termios(o_tty); - if (retval) - goto err_free_termios; + if (legacy) { + /* We always use new tty termios data so we can do this + the easy way .. */ + retval = tty_init_termios(tty); + if (retval) + goto err_deinit_tty; + + retval = tty_init_termios(o_tty); + if (retval) + goto err_free_termios; + + driver->other->ttys[idx] = o_tty; + driver->ttys[idx] = tty; + } else { + tty->termios = kzalloc(sizeof(struct ktermios[2]), GFP_KERNEL); + if (tty->termios == NULL) + goto err_deinit_tty; + *tty->termios = driver->init_termios; + tty->termios_locked = tty->termios + 1; + + o_tty->termios = kzalloc(sizeof(struct ktermios[2]), + GFP_KERNEL); + if (o_tty->termios == NULL) + goto err_free_termios; + *o_tty->termios = driver->other->init_termios; + o_tty->termios_locked = o_tty->termios + 1; + } /* * Everything allocated ... set up the o_tty structure. */ - driver->other->ttys[idx] = o_tty; tty_driver_kref_get(driver->other); if (driver->subtype == PTY_TYPE_MASTER) o_tty->count++; @@ -324,18 +338,29 @@ static int pty_install(struct tty_driver *driver, struct tty_struct *tty) tty_driver_kref_get(driver); tty->count++; - driver->ttys[idx] = tty; return 0; err_free_termios: - tty_free_termios(tty); + if (legacy) + tty_free_termios(tty); + else + kfree(tty->termios); err_deinit_tty: deinitialize_tty_struct(o_tty); module_put(o_tty->driver->owner); err_free_tty: free_tty_struct(o_tty); +err: return retval; } +/* Traditional BSD devices */ +#ifdef CONFIG_LEGACY_PTYS + +static int pty_install(struct tty_driver *driver, struct tty_struct *tty) +{ + return pty_common_install(driver, tty, true); +} + static int pty_bsd_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg) { @@ -509,52 +534,7 @@ static void pty_unix98_shutdown(struct tty_struct *tty) static int pty_unix98_install(struct tty_driver *driver, struct tty_struct *tty) { - struct tty_struct *o_tty; - int idx = tty->index; - - o_tty = alloc_tty_struct(); - if (!o_tty) - return -ENOMEM; - if (!try_module_get(driver->other->owner)) { - /* This cannot in fact currently happen */ - goto err_free_tty; - } - initialize_tty_struct(o_tty, driver->other, idx); - - tty->termios = kzalloc(sizeof(struct ktermios[2]), GFP_KERNEL); - if (tty->termios == NULL) - goto err_free_mem; - *tty->termios = driver->init_termios; - tty->termios_locked = tty->termios + 1; - - o_tty->termios = kzalloc(sizeof(struct ktermios[2]), GFP_KERNEL); - if (o_tty->termios == NULL) - goto err_free_mem; - *o_tty->termios = driver->other->init_termios; - o_tty->termios_locked = o_tty->termios + 1; - - tty_driver_kref_get(driver->other); - if (driver->subtype == PTY_TYPE_MASTER) - o_tty->count++; - /* Establish the links in both directions */ - tty->link = o_tty; - o_tty->link = tty; - /* - * All structures have been allocated, so now we install them. - * Failures after this point use release_tty to clean up, so - * there's no need to null out the local pointers. - */ - tty_driver_kref_get(driver); - tty->count++; - return 0; -err_free_mem: - deinitialize_tty_struct(o_tty); - kfree(o_tty->termios); - kfree(tty->termios); - module_put(o_tty->driver->owner); -err_free_tty: - free_tty_struct(o_tty); - return -ENOMEM; + return pty_common_install(driver, tty, false); } static void pty_unix98_remove(struct tty_driver *driver, struct tty_struct *tty) -- cgit v1.2.3 From d03702a27df017d1807fd4809f03adaa8e37005f Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Mon, 4 Jun 2012 13:35:30 +0200 Subject: PTY: add tty_port This has *no* function in the PTY driver yet. However as the tty buffers will move to the tty_port structure, we will need tty_port for all TTYs in the system, PTY inclusive. For PTYs this is ensured by allocating 2 tty_port's in pty_install, i.e. where the tty->link is allocated. Both tty_port's are properly assigned to each end of the tty. Freeing is done at the same place where tty is freed, i.e. in tty->ops->cleanup. This means BTW that tty_port does not outlive TTY in PTY. This might be a subject to change in the future if we see some problems. Signed-off-by: Jiri Slaby Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index 881888f0a445..b50fc1c01415 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -286,12 +286,15 @@ static int pty_common_install(struct tty_driver *driver, struct tty_struct *tty, bool legacy) { struct tty_struct *o_tty; + struct tty_port *ports[2]; int idx = tty->index; int retval = -ENOMEM; o_tty = alloc_tty_struct(); - if (!o_tty) - goto err; + ports[0] = kmalloc(sizeof **ports, GFP_KERNEL); + ports[1] = kmalloc(sizeof **ports, GFP_KERNEL); + if (!o_tty || !ports[0] || !ports[1]) + goto err_free_tty; if (!try_module_get(driver->other->owner)) { /* This cannot in fact currently happen */ goto err_free_tty; @@ -335,6 +338,10 @@ static int pty_common_install(struct tty_driver *driver, struct tty_struct *tty, /* Establish the links in both directions */ tty->link = o_tty; o_tty->link = tty; + tty_port_init(ports[0]); + tty_port_init(ports[1]); + o_tty->port = ports[0]; + tty->port = ports[1]; tty_driver_kref_get(driver); tty->count++; @@ -348,11 +355,17 @@ err_deinit_tty: deinitialize_tty_struct(o_tty); module_put(o_tty->driver->owner); err_free_tty: + kfree(ports[0]); + kfree(ports[1]); free_tty_struct(o_tty); -err: return retval; } +static void pty_cleanup(struct tty_struct *tty) +{ + kfree(tty->port); +} + /* Traditional BSD devices */ #ifdef CONFIG_LEGACY_PTYS @@ -391,6 +404,7 @@ static const struct tty_operations master_pty_ops_bsd = { .unthrottle = pty_unthrottle, .set_termios = pty_set_termios, .ioctl = pty_bsd_ioctl, + .cleanup = pty_cleanup, .resize = pty_resize }; @@ -404,6 +418,7 @@ static const struct tty_operations slave_pty_ops_bsd = { .chars_in_buffer = pty_chars_in_buffer, .unthrottle = pty_unthrottle, .set_termios = pty_set_termios, + .cleanup = pty_cleanup, .resize = pty_resize }; @@ -555,6 +570,7 @@ static const struct tty_operations ptm_unix98_ops = { .set_termios = pty_set_termios, .ioctl = pty_unix98_ioctl, .shutdown = pty_unix98_shutdown, + .cleanup = pty_cleanup, .resize = pty_resize }; @@ -570,7 +586,8 @@ static const struct tty_operations pty_unix98_ops = { .chars_in_buffer = pty_chars_in_buffer, .unthrottle = pty_unthrottle, .set_termios = pty_set_termios, - .shutdown = pty_unix98_shutdown + .shutdown = pty_unix98_shutdown, + .cleanup = pty_cleanup, }; /** -- cgit v1.2.3 From f5e3bcc504c3c35cc6e06a9ee42efed7c274066b Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Fri, 29 Jun 2012 14:48:36 +0100 Subject: tty: localise the lock The termios and other changes mean the other protections needed on the driver tty arrays should be adequate. Turn it all back on. This contains pieces folded in from the fixes made to the original patches | From: Geert Uytterhoeven (fix m68k) | From: Paul Gortmaker (fix cris) | From: Jiri Kosina (lockdep) | From: Eric Dumazet (lockdep) Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index b50fc1c01415..d8558834ce57 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -47,6 +47,7 @@ static void pty_close(struct tty_struct *tty, struct file *filp) wake_up_interruptible(&tty->read_wait); wake_up_interruptible(&tty->write_wait); tty->packet = 0; + /* Review - krefs on tty_link ?? */ if (!tty->link) return; tty->link->packet = 0; @@ -62,9 +63,9 @@ static void pty_close(struct tty_struct *tty, struct file *filp) mutex_unlock(&devpts_mutex); } #endif - tty_unlock(); + tty_unlock(tty); tty_vhangup(tty->link); - tty_lock(); + tty_lock(tty); } } @@ -615,26 +616,27 @@ static int ptmx_open(struct inode *inode, struct file *filp) return retval; /* find a device that is not in use. */ - tty_lock(); + mutex_lock(&devpts_mutex); index = devpts_new_index(inode); - tty_unlock(); if (index < 0) { retval = index; goto err_file; } + mutex_unlock(&devpts_mutex); + mutex_lock(&tty_mutex); - mutex_lock(&devpts_mutex); tty = tty_init_dev(ptm_driver, index); - mutex_unlock(&devpts_mutex); - tty_lock(); - mutex_unlock(&tty_mutex); if (IS_ERR(tty)) { retval = PTR_ERR(tty); goto out; } + /* The tty returned here is locked so we can safely + drop the mutex */ + mutex_unlock(&tty_mutex); + set_bit(TTY_PTY_LOCK, &tty->flags); /* LOCK THE SLAVE */ tty_add_file(tty, filp); @@ -647,16 +649,17 @@ static int ptmx_open(struct inode *inode, struct file *filp) if (retval) goto err_release; - tty_unlock(); + tty_unlock(tty); return 0; err_release: - tty_unlock(); + tty_unlock(tty); tty_release(inode, filp); return retval; out: + mutex_unlock(&tty_mutex); devpts_kill_index(inode, index); - tty_unlock(); err_file: + mutex_unlock(&devpts_mutex); tty_free_file(filp); return retval; } -- cgit v1.2.3 From 79d753209245de3d6f02480535a8f5cf21ad02f7 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 11 Jul 2012 09:40:40 +0300 Subject: tty: double unlock on error in ptmx_open() The problem here is that we called mutex_unlock(&devpts_mutex) on the error path when we weren't holding the lock. Signed-off-by: Dan Carpenter Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index d8558834ce57..a0ca0830cbcf 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -618,13 +618,12 @@ static int ptmx_open(struct inode *inode, struct file *filp) /* find a device that is not in use. */ mutex_lock(&devpts_mutex); index = devpts_new_index(inode); + mutex_unlock(&devpts_mutex); if (index < 0) { retval = index; goto err_file; } - mutex_unlock(&devpts_mutex); - mutex_lock(&tty_mutex); tty = tty_init_dev(ptm_driver, index); @@ -659,7 +658,6 @@ out: mutex_unlock(&tty_mutex); devpts_kill_index(inode, index); err_file: - mutex_unlock(&devpts_mutex); tty_free_file(filp); return retval; } -- cgit v1.2.3 From 6d31a88cb2e01d46c0cb74aa5da529e1f92ae3db Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Sat, 14 Jul 2012 15:31:27 +0100 Subject: tty: revert incorrectly applied lock patch I sent GregKH this after the pre-requisites. He dropped the pre-requesites for good reason and unfortunately then applied this patch. Without this reverted you get random kernel memory corruption which will make bisecting anything between it and the properly applied patches a complete sod. Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index a0ca0830cbcf..b50fc1c01415 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -47,7 +47,6 @@ static void pty_close(struct tty_struct *tty, struct file *filp) wake_up_interruptible(&tty->read_wait); wake_up_interruptible(&tty->write_wait); tty->packet = 0; - /* Review - krefs on tty_link ?? */ if (!tty->link) return; tty->link->packet = 0; @@ -63,9 +62,9 @@ static void pty_close(struct tty_struct *tty, struct file *filp) mutex_unlock(&devpts_mutex); } #endif - tty_unlock(tty); + tty_unlock(); tty_vhangup(tty->link); - tty_lock(tty); + tty_lock(); } } @@ -616,26 +615,26 @@ static int ptmx_open(struct inode *inode, struct file *filp) return retval; /* find a device that is not in use. */ - mutex_lock(&devpts_mutex); + tty_lock(); index = devpts_new_index(inode); - mutex_unlock(&devpts_mutex); + tty_unlock(); if (index < 0) { retval = index; goto err_file; } mutex_lock(&tty_mutex); + mutex_lock(&devpts_mutex); tty = tty_init_dev(ptm_driver, index); + mutex_unlock(&devpts_mutex); + tty_lock(); + mutex_unlock(&tty_mutex); if (IS_ERR(tty)) { retval = PTR_ERR(tty); goto out; } - /* The tty returned here is locked so we can safely - drop the mutex */ - mutex_unlock(&tty_mutex); - set_bit(TTY_PTY_LOCK, &tty->flags); /* LOCK THE SLAVE */ tty_add_file(tty, filp); @@ -648,15 +647,15 @@ static int ptmx_open(struct inode *inode, struct file *filp) if (retval) goto err_release; - tty_unlock(tty); + tty_unlock(); return 0; err_release: - tty_unlock(tty); + tty_unlock(); tty_release(inode, filp); return retval; out: - mutex_unlock(&tty_mutex); devpts_kill_index(inode, index); + tty_unlock(); err_file: tty_free_file(filp); return retval; -- cgit v1.2.3 From adc8d746caa67fff4b53ba3e5163a6cbacc3b523 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Sat, 14 Jul 2012 15:31:47 +0100 Subject: tty: move the termios object into the tty This will let us sort out a whole pile of tty related races. The alternative would be to keep points and refcount the termios objects. However 1. They are tiny anyway 2. Many devices don't use the stored copies 3. We can remove a pty special case Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index b50fc1c01415..5ad7ccc49f74 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -231,8 +231,8 @@ out: static void pty_set_termios(struct tty_struct *tty, struct ktermios *old_termios) { - tty->termios->c_cflag &= ~(CSIZE | PARENB); - tty->termios->c_cflag |= (CS8 | CREAD); + tty->termios.c_cflag &= ~(CSIZE | PARENB); + tty->termios.c_cflag |= (CS8 | CREAD); } /** @@ -315,18 +315,10 @@ static int pty_common_install(struct tty_driver *driver, struct tty_struct *tty, driver->other->ttys[idx] = o_tty; driver->ttys[idx] = tty; } else { - tty->termios = kzalloc(sizeof(struct ktermios[2]), GFP_KERNEL); - if (tty->termios == NULL) - goto err_deinit_tty; - *tty->termios = driver->init_termios; - tty->termios_locked = tty->termios + 1; - - o_tty->termios = kzalloc(sizeof(struct ktermios[2]), - GFP_KERNEL); - if (o_tty->termios == NULL) - goto err_free_termios; - *o_tty->termios = driver->other->init_termios; - o_tty->termios_locked = o_tty->termios + 1; + memset(&tty->termios_locked, 0, sizeof(tty->termios_locked)); + tty->termios = driver->init_termios; + memset(&o_tty->termios_locked, 0, sizeof(tty->termios_locked)); + o_tty->termios = driver->other->init_termios; } /* @@ -349,8 +341,6 @@ static int pty_common_install(struct tty_driver *driver, struct tty_struct *tty, err_free_termios: if (legacy) tty_free_termios(tty); - else - kfree(tty->termios); err_deinit_tty: deinitialize_tty_struct(o_tty); module_put(o_tty->driver->owner); @@ -541,7 +531,6 @@ static void pty_unix98_shutdown(struct tty_struct *tty) { tty_driver_remove_tty(tty->driver, tty); /* We have our own method as we don't use the tty index */ - kfree(tty->termios); } /* We have no need to install and remove our tty objects as devpts does all -- cgit v1.2.3 From 36b3c070d2346c890d690d71f6eab02f8c511137 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Tue, 17 Jul 2012 17:06:57 +0100 Subject: tty: Move the handling of the tty release logic Now that we don't have tty->termios tied to drivers->tty we can untangle the logic here. In addition we can push the removal logic out of the destructor path. At that point we can think about sorting out tty_port and console and all the other ugly hangovers. Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index 5ad7ccc49f74..60c08ce83782 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -527,12 +527,6 @@ static struct tty_struct *pts_unix98_lookup(struct tty_driver *driver, return tty; } -static void pty_unix98_shutdown(struct tty_struct *tty) -{ - tty_driver_remove_tty(tty->driver, tty); - /* We have our own method as we don't use the tty index */ -} - /* We have no need to install and remove our tty objects as devpts does all the work for us */ @@ -558,9 +552,8 @@ static const struct tty_operations ptm_unix98_ops = { .unthrottle = pty_unthrottle, .set_termios = pty_set_termios, .ioctl = pty_unix98_ioctl, - .shutdown = pty_unix98_shutdown, - .cleanup = pty_cleanup, - .resize = pty_resize + .resize = pty_resize, + .cleanup = pty_cleanup }; static const struct tty_operations pty_unix98_ops = { @@ -575,7 +568,6 @@ static const struct tty_operations pty_unix98_ops = { .chars_in_buffer = pty_chars_in_buffer, .unthrottle = pty_unthrottle, .set_termios = pty_set_termios, - .shutdown = pty_unix98_shutdown, .cleanup = pty_cleanup, }; -- cgit v1.2.3 From d155255a344c417acad74156654295a2964e6b81 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Fri, 27 Jul 2012 18:02:54 +0100 Subject: tty: Fix race in tty release Ian Abbott found that the tty layer would explode with the right set of parallel open and close operations. This is because we race in the handling of tty->drivers->termios[]. Correct this by Making tty_ldisc_release behave like nromal code (takes the lock, does stuff, drops the lock) Drop the tty lock earlier in tty_ldisc_release Taking the tty mutex around the driver->termios update in all cases Adding a WARN_ON to catch future screwups. I also forgot to clean up the pty resources properly. With a pty pair we need to pull both halves out of the tables. Signed-off-by: Alan Cox Tested-by: Ian Abbott Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index 60c08ce83782..d6579a9064c4 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -282,6 +282,17 @@ done: return 0; } +/** + * pty_common_install - set up the pty pair + * @driver: the pty driver + * @tty: the tty being instantiated + * @bool: legacy, true if this is BSD style + * + * Perform the initial set up for the tty/pty pair. Called from the + * tty layer when the port is first opened. + * + * Locking: the caller must hold the tty_mutex + */ static int pty_common_install(struct tty_driver *driver, struct tty_struct *tty, bool legacy) { @@ -364,6 +375,14 @@ static int pty_install(struct tty_driver *driver, struct tty_struct *tty) return pty_common_install(driver, tty, true); } +static void pty_remove(struct tty_driver *driver, struct tty_struct *tty) +{ + struct tty_struct *pair = tty->link; + driver->ttys[tty->index] = NULL; + if (pair) + pair->driver->ttys[pair->index] = NULL; +} + static int pty_bsd_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg) { @@ -395,7 +414,8 @@ static const struct tty_operations master_pty_ops_bsd = { .set_termios = pty_set_termios, .ioctl = pty_bsd_ioctl, .cleanup = pty_cleanup, - .resize = pty_resize + .resize = pty_resize, + .remove = pty_remove }; static const struct tty_operations slave_pty_ops_bsd = { @@ -409,7 +429,8 @@ static const struct tty_operations slave_pty_ops_bsd = { .unthrottle = pty_unthrottle, .set_termios = pty_set_termios, .cleanup = pty_cleanup, - .resize = pty_resize + .resize = pty_resize, + .remove = pty_remove }; static void __init legacy_pty_init(void) -- cgit v1.2.3 From 89c8d91e31f267703e365593f6bfebb9f6d2ad01 Mon Sep 17 00:00:00 2001 From: Alan Cox Date: Wed, 8 Aug 2012 16:30:13 +0100 Subject: tty: localise the lock The termios and other changes mean the other protections needed on the driver tty arrays should be adequate. Turn it all back on. This contains pieces folded in from the fixes made to the original patches | From: Geert Uytterhoeven (fix m68k) | From: Paul Gortmaker (fix cris) | From: Jiri Kosina (lockdep) | From: Eric Dumazet (lockdep) Signed-off-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index d6579a9064c4..4399f1dbd131 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -47,6 +47,7 @@ static void pty_close(struct tty_struct *tty, struct file *filp) wake_up_interruptible(&tty->read_wait); wake_up_interruptible(&tty->write_wait); tty->packet = 0; + /* Review - krefs on tty_link ?? */ if (!tty->link) return; tty->link->packet = 0; @@ -62,9 +63,9 @@ static void pty_close(struct tty_struct *tty, struct file *filp) mutex_unlock(&devpts_mutex); } #endif - tty_unlock(); + tty_unlock(tty); tty_vhangup(tty->link); - tty_lock(); + tty_lock(tty); } } @@ -617,26 +618,27 @@ static int ptmx_open(struct inode *inode, struct file *filp) return retval; /* find a device that is not in use. */ - tty_lock(); + mutex_lock(&devpts_mutex); index = devpts_new_index(inode); - tty_unlock(); if (index < 0) { retval = index; goto err_file; } + mutex_unlock(&devpts_mutex); + mutex_lock(&tty_mutex); - mutex_lock(&devpts_mutex); tty = tty_init_dev(ptm_driver, index); - mutex_unlock(&devpts_mutex); - tty_lock(); - mutex_unlock(&tty_mutex); if (IS_ERR(tty)) { retval = PTR_ERR(tty); goto out; } + /* The tty returned here is locked so we can safely + drop the mutex */ + mutex_unlock(&tty_mutex); + set_bit(TTY_PTY_LOCK, &tty->flags); /* LOCK THE SLAVE */ tty_add_file(tty, filp); @@ -649,16 +651,17 @@ static int ptmx_open(struct inode *inode, struct file *filp) if (retval) goto err_release; - tty_unlock(); + tty_unlock(tty); return 0; err_release: - tty_unlock(); + tty_unlock(tty); tty_release(inode, filp); return retval; out: + mutex_unlock(&tty_mutex); devpts_kill_index(inode, index); - tty_unlock(); err_file: + mutex_unlock(&devpts_mutex); tty_free_file(filp); return retval; } -- cgit v1.2.3 From 6f9ea7ad7be10dca95e7ca57221c5f81be48d852 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Tue, 7 Aug 2012 21:47:26 +0200 Subject: TTY: pty, stop passing NULL to free_tty_struct In case alloc_tty_struct fails in pty_common_install, we pass NULL to free_tty_struct. This is invalid as the function is not ready to cope with that. And even if it was, it is not nice to do that anyway. Signed-off-by: Jiri Slaby Reported-by: Dan Carpenter Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index 4399f1dbd131..d9ea9e2c9ec5 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -303,9 +303,11 @@ static int pty_common_install(struct tty_driver *driver, struct tty_struct *tty, int retval = -ENOMEM; o_tty = alloc_tty_struct(); + if (!o_tty) + goto err; ports[0] = kmalloc(sizeof **ports, GFP_KERNEL); ports[1] = kmalloc(sizeof **ports, GFP_KERNEL); - if (!o_tty || !ports[0] || !ports[1]) + if (!ports[0] || !ports[1]) goto err_free_tty; if (!try_module_get(driver->other->owner)) { /* This cannot in fact currently happen */ @@ -360,6 +362,7 @@ err_free_tty: kfree(ports[0]); kfree(ports[1]); free_tty_struct(o_tty); +err: return retval; } -- cgit v1.2.3 From 21aca2fa00259f781d228496631b61dbb4274e90 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Wed, 8 Aug 2012 22:26:41 +0200 Subject: TTY: pty, switch to tty_alloc_driver Switch to the new driver allocation interface, as this is one of the special call-sites. Here, we need TTY_DRIVER_DYNAMIC_ALLOC to not allocate tty_driver->ports, cdevs and potentially other structures because we reserve too many lines in pty. Instead, it provides the tty_port<->tty_struct link in tty->ops->install already. Signed-off-by: Jiri Slaby Acked-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index d9ea9e2c9ec5..f5a27c66ff60 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -444,11 +444,17 @@ static void __init legacy_pty_init(void) if (legacy_count <= 0) return; - pty_driver = alloc_tty_driver(legacy_count); + pty_driver = tty_alloc_driver(legacy_count, + TTY_DRIVER_RESET_TERMIOS | + TTY_DRIVER_REAL_RAW | + TTY_DRIVER_DYNAMIC_ALLOC); if (!pty_driver) panic("Couldn't allocate pty driver"); - pty_slave_driver = alloc_tty_driver(legacy_count); + pty_slave_driver = tty_alloc_driver(legacy_count, + TTY_DRIVER_RESET_TERMIOS | + TTY_DRIVER_REAL_RAW | + TTY_DRIVER_DYNAMIC_ALLOC); if (!pty_slave_driver) panic("Couldn't allocate pty slave driver"); @@ -465,7 +471,6 @@ static void __init legacy_pty_init(void) pty_driver->init_termios.c_lflag = 0; pty_driver->init_termios.c_ispeed = 38400; pty_driver->init_termios.c_ospeed = 38400; - pty_driver->flags = TTY_DRIVER_RESET_TERMIOS | TTY_DRIVER_REAL_RAW; pty_driver->other = pty_slave_driver; tty_set_operations(pty_driver, &master_pty_ops_bsd); @@ -479,8 +484,6 @@ static void __init legacy_pty_init(void) pty_slave_driver->init_termios.c_cflag = B38400 | CS8 | CREAD; pty_slave_driver->init_termios.c_ispeed = 38400; pty_slave_driver->init_termios.c_ospeed = 38400; - pty_slave_driver->flags = TTY_DRIVER_RESET_TERMIOS | - TTY_DRIVER_REAL_RAW; pty_slave_driver->other = pty_driver; tty_set_operations(pty_slave_driver, &slave_pty_ops_bsd); @@ -673,10 +676,20 @@ static struct file_operations ptmx_fops; static void __init unix98_pty_init(void) { - ptm_driver = alloc_tty_driver(NR_UNIX98_PTY_MAX); + ptm_driver = tty_alloc_driver(NR_UNIX98_PTY_MAX, + TTY_DRIVER_RESET_TERMIOS | + TTY_DRIVER_REAL_RAW | + TTY_DRIVER_DYNAMIC_DEV | + TTY_DRIVER_DEVPTS_MEM | + TTY_DRIVER_DYNAMIC_ALLOC); if (!ptm_driver) panic("Couldn't allocate Unix98 ptm driver"); - pts_driver = alloc_tty_driver(NR_UNIX98_PTY_MAX); + pts_driver = tty_alloc_driver(NR_UNIX98_PTY_MAX, + TTY_DRIVER_RESET_TERMIOS | + TTY_DRIVER_REAL_RAW | + TTY_DRIVER_DYNAMIC_DEV | + TTY_DRIVER_DEVPTS_MEM | + TTY_DRIVER_DYNAMIC_ALLOC); if (!pts_driver) panic("Couldn't allocate Unix98 pts driver"); @@ -693,8 +706,6 @@ static void __init unix98_pty_init(void) ptm_driver->init_termios.c_lflag = 0; ptm_driver->init_termios.c_ispeed = 38400; ptm_driver->init_termios.c_ospeed = 38400; - ptm_driver->flags = TTY_DRIVER_RESET_TERMIOS | TTY_DRIVER_REAL_RAW | - TTY_DRIVER_DYNAMIC_DEV | TTY_DRIVER_DEVPTS_MEM; ptm_driver->other = pts_driver; tty_set_operations(ptm_driver, &ptm_unix98_ops); @@ -708,8 +719,6 @@ static void __init unix98_pty_init(void) pts_driver->init_termios.c_cflag = B38400 | CS8 | CREAD; pts_driver->init_termios.c_ispeed = 38400; pts_driver->init_termios.c_ospeed = 38400; - pts_driver->flags = TTY_DRIVER_RESET_TERMIOS | TTY_DRIVER_REAL_RAW | - TTY_DRIVER_DYNAMIC_DEV | TTY_DRIVER_DEVPTS_MEM; pts_driver->other = ptm_driver; tty_set_operations(pts_driver, &pty_unix98_ops); -- cgit v1.2.3 From c3a6344ae475763b6fb0fb2ec3639004f500d0f1 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Thu, 16 Aug 2012 16:16:56 +0300 Subject: TTY: tty_alloc_driver() returns error pointers We changed these from alloc_tty_driver() to tty_alloc_driver() so the error handling needs to modified to check for IS_ERR() instead of NULL. Signed-off-by: Dan Carpenter Acked-by: Jiri Slaby Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index f5a27c66ff60..2bace847eb39 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -448,14 +448,14 @@ static void __init legacy_pty_init(void) TTY_DRIVER_RESET_TERMIOS | TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_ALLOC); - if (!pty_driver) + if (IS_ERR(pty_driver)) panic("Couldn't allocate pty driver"); pty_slave_driver = tty_alloc_driver(legacy_count, TTY_DRIVER_RESET_TERMIOS | TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_ALLOC); - if (!pty_slave_driver) + if (IS_ERR(pty_slave_driver)) panic("Couldn't allocate pty slave driver"); pty_driver->driver_name = "pty_master"; @@ -682,7 +682,7 @@ static void __init unix98_pty_init(void) TTY_DRIVER_DYNAMIC_DEV | TTY_DRIVER_DEVPTS_MEM | TTY_DRIVER_DYNAMIC_ALLOC); - if (!ptm_driver) + if (IS_ERR(ptm_driver)) panic("Couldn't allocate Unix98 ptm driver"); pts_driver = tty_alloc_driver(NR_UNIX98_PTY_MAX, TTY_DRIVER_RESET_TERMIOS | @@ -690,7 +690,7 @@ static void __init unix98_pty_init(void) TTY_DRIVER_DYNAMIC_DEV | TTY_DRIVER_DEVPTS_MEM | TTY_DRIVER_DYNAMIC_ALLOC); - if (!pts_driver) + if (IS_ERR(pts_driver)) panic("Couldn't allocate Unix98 pts driver"); ptm_driver->driver_name = "pty_master"; -- cgit v1.2.3