From 8193c4290620d9b2a6ac116719f11aa99053a90d Mon Sep 17 00:00:00 2001 From: Thomas Meyer Date: Wed, 5 Oct 2011 23:13:13 +0200 Subject: tty: Support compat_ioctl get/set termios_locked When running a Fedora 15 (x86) on an x86_64 kernel, in the boot process plymouthd complains about those two missing ioctls: [ 2.581783] ioctl32(plymouthd:186): Unknown cmd fd(10) cmd(00005457){t:'T';sz:0} arg(ffb6a5d0) on /dev/tty1 [ 2.581803] ioctl32(plymouthd:186): Unknown cmd fd(10) cmd(00005456){t:'T';sz:0} arg(ffb6a680) on /dev/tty1 both ioctl functions work on the 'struct termios' resp. 'struct termios2', which has the same size (36 bytes resp. 44 bytes) on x86 and x86_64, so it's just a matter of converting the pointer from userland. Signed-off-by: Thomas Meyer Cc: Arnd Bergmann Cc: Alan Cox Signed-off-by: Andrew Morton Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_io.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/tty/tty_io.c') diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 150e4f747c7d..4ca4bcd28ff7 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -2717,6 +2717,8 @@ static long tty_compat_ioctl(struct file *file, unsigned int cmd, ld = tty_ldisc_ref_wait(tty); if (ld->ops->compat_ioctl) retval = ld->ops->compat_ioctl(tty, file, cmd, arg); + else + retval = n_tty_compat_ioctl_helper(tty, file, cmd, arg); tty_ldisc_deref(ld); return retval; -- cgit v1.2.2 From c290f8358acaeffd8e0c551ddcc24d1206143376 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Wed, 12 Oct 2011 11:32:42 +0200 Subject: TTY: drop driver reference in tty_open fail path When tty_driver_lookup_tty fails in tty_open, we forget to drop a reference to the tty driver. This was added by commit 4a2b5fddd5 (Move tty lookup/reopen to caller). Fix that by adding tty_driver_kref_put to the fail path. I will refactor the code later. This is for the ease of backporting to stable. Introduced-in: v2.6.28-rc2 Signed-off-by: Jiri Slaby Cc: stable Cc: Alan Cox Acked-by: Sukadev Bhattiprolu Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_io.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/tty/tty_io.c') diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 4ca4bcd28ff7..6913da8f202c 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -1873,6 +1873,7 @@ got_driver: if (IS_ERR(tty)) { tty_unlock(); mutex_unlock(&tty_mutex); + tty_driver_kref_put(driver); return PTR_ERR(tty); } } -- cgit v1.2.2 From fa90e1c935472281de314e6d7c9a37db9cbc2e4e Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Wed, 12 Oct 2011 11:32:43 +0200 Subject: TTY: make tty_add_file non-failing If tty_add_file fails at the point it is now, we have to revert all the changes we did to the tty. It means either decrease all refcounts if this was a tty reopen or delete the tty if it was newly allocated. There was a try to fix this in v3.0-rc2 using tty_release in 0259894c7 (TTY: fix fail path in tty_open). But instead it introduced a NULL dereference. It's because tty_release dereferences filp->private_data, but that one is set even in our tty_add_file. And when tty_add_file fails, it's still NULL/garbage. Hence tty_release cannot be called there. To circumvent the original leak (and the current NULL deref) we split tty_add_file into two functions, making the latter non-failing. In that case we may do the former early in open, where handling failures is easy. The latter stays as it is now. So there is no change in functionality. The original bug (leak) was introduced by f573bd176 (tty: Remove __GFP_NOFAIL from tty_add_file()). Thanks Dan for reporting this. Later, we may split tty_release into more functions and call only some of them in this fail path instead. (If at all possible.) Introduced-in: v2.6.37-rc2 Signed-off-by: Jiri Slaby Reported-by: Dan Carpenter Cc: stable Cc: Alan Cox Cc: Pekka Enberg Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_io.c | 47 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 35 insertions(+), 12 deletions(-) (limited to 'drivers/tty/tty_io.c') diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 6913da8f202c..767ecbb4761a 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -194,8 +194,7 @@ static inline struct tty_struct *file_tty(struct file *file) return ((struct tty_file_private *)file->private_data)->tty; } -/* Associate a new file with the tty structure */ -int tty_add_file(struct tty_struct *tty, struct file *file) +int tty_alloc_file(struct file *file) { struct tty_file_private *priv; @@ -203,15 +202,36 @@ int tty_add_file(struct tty_struct *tty, struct file *file) if (!priv) return -ENOMEM; + file->private_data = priv; + + return 0; +} + +/* Associate a new file with the tty structure */ +void tty_add_file(struct tty_struct *tty, struct file *file) +{ + struct tty_file_private *priv = file->private_data; + priv->tty = tty; priv->file = file; - file->private_data = priv; spin_lock(&tty_files_lock); list_add(&priv->list, &tty->tty_files); spin_unlock(&tty_files_lock); +} - return 0; +/** + * tty_free_file - free file->private_data + * + * This shall be used only for fail path handling when tty_add_file was not + * called yet. + */ +void tty_free_file(struct file *file) +{ + struct tty_file_private *priv = file->private_data; + + file->private_data = NULL; + kfree(priv); } /* Delete file from its tty */ @@ -222,8 +242,7 @@ void tty_del_file(struct file *file) spin_lock(&tty_files_lock); list_del(&priv->list); spin_unlock(&tty_files_lock); - file->private_data = NULL; - kfree(priv); + tty_free_file(file); } @@ -1812,6 +1831,10 @@ static int tty_open(struct inode *inode, struct file *filp) nonseekable_open(inode, filp); retry_open: + retval = tty_alloc_file(filp); + if (retval) + return -ENOMEM; + noctty = filp->f_flags & O_NOCTTY; index = -1; retval = 0; @@ -1824,6 +1847,7 @@ retry_open: if (!tty) { tty_unlock(); mutex_unlock(&tty_mutex); + tty_free_file(filp); return -ENXIO; } driver = tty_driver_kref_get(tty->driver); @@ -1856,6 +1880,7 @@ retry_open: } tty_unlock(); mutex_unlock(&tty_mutex); + tty_free_file(filp); return -ENODEV; } @@ -1863,6 +1888,7 @@ retry_open: if (!driver) { tty_unlock(); mutex_unlock(&tty_mutex); + tty_free_file(filp); return -ENODEV; } got_driver: @@ -1874,6 +1900,7 @@ got_driver: tty_unlock(); mutex_unlock(&tty_mutex); tty_driver_kref_put(driver); + tty_free_file(filp); return PTR_ERR(tty); } } @@ -1889,15 +1916,11 @@ got_driver: tty_driver_kref_put(driver); if (IS_ERR(tty)) { tty_unlock(); + tty_free_file(filp); return PTR_ERR(tty); } - retval = tty_add_file(tty, filp); - if (retval) { - tty_unlock(); - tty_release(inode, filp); - return retval; - } + tty_add_file(tty, filp); check_tty_count(tty, "tty_open"); if (tty->driver->type == TTY_DRIVER_TYPE_PTY && -- cgit v1.2.2 From 631180aca723cb92e128fdac5fd144e913ca84e5 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Wed, 12 Oct 2011 11:32:45 +0200 Subject: TTY: call tty_driver_lookup_tty unconditionally Commit 4a2b5fddd5 (Move tty lookup/reopen to caller) made the call to tty_driver_lookup_tty conditional in tty_open. It doesn't look like it was an intention. Or if it was, it was not documented in the changelog and the code now looks weird. For example there would be no need to remember the tty driver and tty index. Further the condition depends on a tty which we drop a reference of already. If I'm looking correctly, this should not matter thanks to the locking currently done there. Thus, tty_driver->ttys[idx] cannot change under our hands. But anyway, it makes sense to change that to the old behaviour. Introduced-in: v2.6.28-rc2 Signed-off-by: Jiri Slaby Cc: stable Cc: Sukadev Bhattiprolu Cc: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_io.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) (limited to 'drivers/tty/tty_io.c') diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 767ecbb4761a..0425170d9ed6 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -1821,7 +1821,7 @@ int tty_release(struct inode *inode, struct file *filp) static int tty_open(struct inode *inode, struct file *filp) { - struct tty_struct *tty = NULL; + struct tty_struct *tty; int noctty, retval; struct tty_driver *driver; int index; @@ -1892,17 +1892,14 @@ retry_open: return -ENODEV; } got_driver: - if (!tty) { - /* check whether we're reopening an existing tty */ - tty = tty_driver_lookup_tty(driver, inode, index); - - if (IS_ERR(tty)) { - tty_unlock(); - mutex_unlock(&tty_mutex); - tty_driver_kref_put(driver); - tty_free_file(filp); - return PTR_ERR(tty); - } + /* check whether we're reopening an existing tty */ + tty = tty_driver_lookup_tty(driver, inode, index); + if (IS_ERR(tty)) { + tty_unlock(); + mutex_unlock(&tty_mutex); + tty_driver_kref_put(driver); + tty_free_file(filp); + return PTR_ERR(tty); } if (tty) { -- cgit v1.2.2 From a0340703981baa6cc1e9c7c768095a0a4e718daf Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 19 Oct 2011 08:33:21 -0700 Subject: Revert "TTY: call tty_driver_lookup_tty unconditionally" This reverts commit 631180aca723cb92e128fdac5fd144e913ca84e5. It caused problems when /dev/tty is a pty: https://lkml.org/lkml/2011/10/12/401 Cc: Jiri Slaby Cc: stable Cc: Sukadev Bhattiprolu Cc: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/tty/tty_io.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'drivers/tty/tty_io.c') diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index 0425170d9ed6..767ecbb4761a 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -1821,7 +1821,7 @@ int tty_release(struct inode *inode, struct file *filp) static int tty_open(struct inode *inode, struct file *filp) { - struct tty_struct *tty; + struct tty_struct *tty = NULL; int noctty, retval; struct tty_driver *driver; int index; @@ -1892,14 +1892,17 @@ retry_open: return -ENODEV; } got_driver: - /* check whether we're reopening an existing tty */ - tty = tty_driver_lookup_tty(driver, inode, index); - if (IS_ERR(tty)) { - tty_unlock(); - mutex_unlock(&tty_mutex); - tty_driver_kref_put(driver); - tty_free_file(filp); - return PTR_ERR(tty); + if (!tty) { + /* check whether we're reopening an existing tty */ + tty = tty_driver_lookup_tty(driver, inode, index); + + if (IS_ERR(tty)) { + tty_unlock(); + mutex_unlock(&tty_mutex); + tty_driver_kref_put(driver); + tty_free_file(filp); + return PTR_ERR(tty); + } } if (tty) { -- cgit v1.2.2