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/pty.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index 98b6e3bdb000..7613f95f2d6b 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -657,12 +657,18 @@ static int ptmx_open(struct inode *inode, struct file *filp) nonseekable_open(inode, filp); + retval = tty_alloc_file(filp); + if (retval) + return retval; + /* find a device that is not in use. */ tty_lock(); index = devpts_new_index(inode); tty_unlock(); - if (index < 0) - return index; + if (index < 0) { + retval = index; + goto err_file; + } mutex_lock(&tty_mutex); 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 */ - retval = tty_add_file(tty, filp); - if (retval) - goto out; + tty_add_file(tty, filp); retval = devpts_pty_new(inode, tty->link); if (retval) @@ -697,6 +701,8 @@ out2: out: devpts_kill_index(inode, index); tty_unlock(); +err_file: + tty_free_file(filp); return retval; } -- cgit v1.2.2 From 1177c0efc04d032644895b8d757f55b433912596 Mon Sep 17 00:00:00 2001 From: Jiri Slaby Date: Wed, 12 Oct 2011 11:32:44 +0200 Subject: TTY: pty, release tty in all ptmx_open fail paths Mistakenly, commit 64ba3dc3143d (tty: never hold BTM while getting tty_mutex) switched one fail path in ptmx_open to not free the newly allocated tty. Fix that by jumping to the appropriate place. And rename the labels so that it's clear what is going on there. Introduced-in: v2.6.36-rc2 Signed-off-by: Jiri Slaby Cc: stable Cc: Arnd Bergmann Cc: Alan Cox Signed-off-by: Greg Kroah-Hartman --- drivers/tty/pty.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/tty/pty.c') diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c index 7613f95f2d6b..2feea63d497a 100644 --- a/drivers/tty/pty.c +++ b/drivers/tty/pty.c @@ -686,15 +686,15 @@ static int ptmx_open(struct inode *inode, struct file *filp) retval = devpts_pty_new(inode, tty->link); if (retval) - goto out1; + goto err_release; retval = ptm_driver->ops->open(tty, filp); if (retval) - goto out2; -out1: + goto err_release; + tty_unlock(); - return retval; -out2: + return 0; +err_release: tty_unlock(); tty_release(inode, filp); return retval; -- cgit v1.2.2