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 in0259894c7
(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 byf573bd176
(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 <jslaby@suse.cz> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Cc: stable <stable@vger.kernel.org> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk> Cc: Pekka Enberg <penberg@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
This commit is contained in:
parent
c290f8358a
commit
fa90e1c935
|
@ -657,12 +657,18 @@ static int ptmx_open(struct inode *inode, struct file *filp)
|
||||||
|
|
||||||
nonseekable_open(inode, filp);
|
nonseekable_open(inode, filp);
|
||||||
|
|
||||||
|
retval = tty_alloc_file(filp);
|
||||||
|
if (retval)
|
||||||
|
return retval;
|
||||||
|
|
||||||
/* find a device that is not in use. */
|
/* find a device that is not in use. */
|
||||||
tty_lock();
|
tty_lock();
|
||||||
index = devpts_new_index(inode);
|
index = devpts_new_index(inode);
|
||||||
tty_unlock();
|
tty_unlock();
|
||||||
if (index < 0)
|
if (index < 0) {
|
||||||
return index;
|
retval = index;
|
||||||
|
goto err_file;
|
||||||
|
}
|
||||||
|
|
||||||
mutex_lock(&tty_mutex);
|
mutex_lock(&tty_mutex);
|
||||||
tty_lock();
|
tty_lock();
|
||||||
|
@ -676,9 +682,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
|
||||||
|
|
||||||
set_bit(TTY_PTY_LOCK, &tty->flags); /* LOCK THE SLAVE */
|
set_bit(TTY_PTY_LOCK, &tty->flags); /* LOCK THE SLAVE */
|
||||||
|
|
||||||
retval = tty_add_file(tty, filp);
|
tty_add_file(tty, filp);
|
||||||
if (retval)
|
|
||||||
goto out;
|
|
||||||
|
|
||||||
retval = devpts_pty_new(inode, tty->link);
|
retval = devpts_pty_new(inode, tty->link);
|
||||||
if (retval)
|
if (retval)
|
||||||
|
@ -697,6 +701,8 @@ out2:
|
||||||
out:
|
out:
|
||||||
devpts_kill_index(inode, index);
|
devpts_kill_index(inode, index);
|
||||||
tty_unlock();
|
tty_unlock();
|
||||||
|
err_file:
|
||||||
|
tty_free_file(filp);
|
||||||
return retval;
|
return retval;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -194,8 +194,7 @@ static inline struct tty_struct *file_tty(struct file *file)
|
||||||
return ((struct tty_file_private *)file->private_data)->tty;
|
return ((struct tty_file_private *)file->private_data)->tty;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Associate a new file with the tty structure */
|
int tty_alloc_file(struct file *file)
|
||||||
int tty_add_file(struct tty_struct *tty, struct file *file)
|
|
||||||
{
|
{
|
||||||
struct tty_file_private *priv;
|
struct tty_file_private *priv;
|
||||||
|
|
||||||
|
@ -203,15 +202,36 @@ int tty_add_file(struct tty_struct *tty, struct file *file)
|
||||||
if (!priv)
|
if (!priv)
|
||||||
return -ENOMEM;
|
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->tty = tty;
|
||||||
priv->file = file;
|
priv->file = file;
|
||||||
file->private_data = priv;
|
|
||||||
|
|
||||||
spin_lock(&tty_files_lock);
|
spin_lock(&tty_files_lock);
|
||||||
list_add(&priv->list, &tty->tty_files);
|
list_add(&priv->list, &tty->tty_files);
|
||||||
spin_unlock(&tty_files_lock);
|
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 */
|
/* Delete file from its tty */
|
||||||
|
@ -222,8 +242,7 @@ void tty_del_file(struct file *file)
|
||||||
spin_lock(&tty_files_lock);
|
spin_lock(&tty_files_lock);
|
||||||
list_del(&priv->list);
|
list_del(&priv->list);
|
||||||
spin_unlock(&tty_files_lock);
|
spin_unlock(&tty_files_lock);
|
||||||
file->private_data = NULL;
|
tty_free_file(file);
|
||||||
kfree(priv);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@ -1812,6 +1831,10 @@ static int tty_open(struct inode *inode, struct file *filp)
|
||||||
nonseekable_open(inode, filp);
|
nonseekable_open(inode, filp);
|
||||||
|
|
||||||
retry_open:
|
retry_open:
|
||||||
|
retval = tty_alloc_file(filp);
|
||||||
|
if (retval)
|
||||||
|
return -ENOMEM;
|
||||||
|
|
||||||
noctty = filp->f_flags & O_NOCTTY;
|
noctty = filp->f_flags & O_NOCTTY;
|
||||||
index = -1;
|
index = -1;
|
||||||
retval = 0;
|
retval = 0;
|
||||||
|
@ -1824,6 +1847,7 @@ retry_open:
|
||||||
if (!tty) {
|
if (!tty) {
|
||||||
tty_unlock();
|
tty_unlock();
|
||||||
mutex_unlock(&tty_mutex);
|
mutex_unlock(&tty_mutex);
|
||||||
|
tty_free_file(filp);
|
||||||
return -ENXIO;
|
return -ENXIO;
|
||||||
}
|
}
|
||||||
driver = tty_driver_kref_get(tty->driver);
|
driver = tty_driver_kref_get(tty->driver);
|
||||||
|
@ -1856,6 +1880,7 @@ retry_open:
|
||||||
}
|
}
|
||||||
tty_unlock();
|
tty_unlock();
|
||||||
mutex_unlock(&tty_mutex);
|
mutex_unlock(&tty_mutex);
|
||||||
|
tty_free_file(filp);
|
||||||
return -ENODEV;
|
return -ENODEV;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1863,6 +1888,7 @@ retry_open:
|
||||||
if (!driver) {
|
if (!driver) {
|
||||||
tty_unlock();
|
tty_unlock();
|
||||||
mutex_unlock(&tty_mutex);
|
mutex_unlock(&tty_mutex);
|
||||||
|
tty_free_file(filp);
|
||||||
return -ENODEV;
|
return -ENODEV;
|
||||||
}
|
}
|
||||||
got_driver:
|
got_driver:
|
||||||
|
@ -1874,6 +1900,7 @@ got_driver:
|
||||||
tty_unlock();
|
tty_unlock();
|
||||||
mutex_unlock(&tty_mutex);
|
mutex_unlock(&tty_mutex);
|
||||||
tty_driver_kref_put(driver);
|
tty_driver_kref_put(driver);
|
||||||
|
tty_free_file(filp);
|
||||||
return PTR_ERR(tty);
|
return PTR_ERR(tty);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -1889,15 +1916,11 @@ got_driver:
|
||||||
tty_driver_kref_put(driver);
|
tty_driver_kref_put(driver);
|
||||||
if (IS_ERR(tty)) {
|
if (IS_ERR(tty)) {
|
||||||
tty_unlock();
|
tty_unlock();
|
||||||
|
tty_free_file(filp);
|
||||||
return PTR_ERR(tty);
|
return PTR_ERR(tty);
|
||||||
}
|
}
|
||||||
|
|
||||||
retval = tty_add_file(tty, filp);
|
tty_add_file(tty, filp);
|
||||||
if (retval) {
|
|
||||||
tty_unlock();
|
|
||||||
tty_release(inode, filp);
|
|
||||||
return retval;
|
|
||||||
}
|
|
||||||
|
|
||||||
check_tty_count(tty, "tty_open");
|
check_tty_count(tty, "tty_open");
|
||||||
if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
|
if (tty->driver->type == TTY_DRIVER_TYPE_PTY &&
|
||||||
|
|
|
@ -471,7 +471,9 @@ extern void proc_clear_tty(struct task_struct *p);
|
||||||
extern struct tty_struct *get_current_tty(void);
|
extern struct tty_struct *get_current_tty(void);
|
||||||
extern void tty_default_fops(struct file_operations *fops);
|
extern void tty_default_fops(struct file_operations *fops);
|
||||||
extern struct tty_struct *alloc_tty_struct(void);
|
extern struct tty_struct *alloc_tty_struct(void);
|
||||||
extern int tty_add_file(struct tty_struct *tty, struct file *file);
|
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 void free_tty_struct(struct tty_struct *tty);
|
extern void free_tty_struct(struct tty_struct *tty);
|
||||||
extern void initialize_tty_struct(struct tty_struct *tty,
|
extern void initialize_tty_struct(struct tty_struct *tty,
|
||||||
struct tty_driver *driver, int idx);
|
struct tty_driver *driver, int idx);
|
||||||
|
|
Loading…
Reference in a new issue