[PATCH] tty: Fix ptmx open without closed slave.

[PATCH] tty: Fix ptmx open without closed slave.

[Ilya Zykov]

When we are opening ptmx, we have closed pts, by description.
Now only if we open and after close all pts' descriptions, pty_close() sets
this bit correctly

Signed-off-by: Ilya Zykov <ilya@ilyx.ru>
---
 drivers/tty/pty.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 1ce1362..7b69307 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -659,6 +659,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
 	retval = ptm_driver->ops->open(tty, filp);
 	if (retval)
 		goto err_release;
+	set_bit(TTY_OTHER_CLOSED, &tty->flags); /* THE SLAVE STILL CLOSED */
 
 	tty_unlock(tty);
 	return 0;

Re: [PATCH] tty: Fix ptmx open without closed slave.

[Greg Kroah-Hartman]

On Wed, Dec 19, 2012 at 11:00:05PM +0400, Ilya Zykov wrote:
> When we are opening ptmx, we have closed pts, by description.
> Now only if we open and after close all pts' descriptions, pty_close() sets
> this bit correctly
> 
> Signed-off-by: Ilya Zykov <ilya@ilyx.ru>

Ok, I have a ton of patches from you, lots of different threads, patches
with the same subject, and I don't know what one Alan agreed to.

So, can you please resend what Alan and you agree should be applied to
the tree?

totally confused,

greg k-h

Re: [PATCH] tty: Fix ptmx open without closed slave.

[Ilya Zykov]

> 
> Ok, I have a ton of patches from you, lots of different threads, patches
> with the same subject, and I don't know what one Alan agreed to.
> 
> So, can you please resend what Alan and you agree should be applied to
> the tree?
> 


I think only two important:
[PATCH v4] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).

[PATCH] tty: Correct tty buffer flush.
With Alan's ACK from 04.12.2012 for kernel <= 3.7

But for kernel 3.8 I resent this 05.12.2012
[PATCH -next 0/2] tty: Correct tty buffer flush.
[PATCH -next 1/2] tty: Correct tty buffer flush.
[PATCH -next 2/2] tty: Correct tty buffer flush.
The last I resent with new description after Jiri's remark.

If need I can resent.

Re: [PATCH] tty: Fix ptmx open without closed slave.

[Greg Kroah-Hartman]

On Wed, Jan 16, 2013 at 11:48:38AM +0400, Ilya Zykov wrote:
> 
> > 
> > Ok, I have a ton of patches from you, lots of different threads, patches
> > with the same subject, and I don't know what one Alan agreed to.
> > 
> > So, can you please resend what Alan and you agree should be applied to
> > the tree?
> > 
> 
> 
> I think only two important:
> [PATCH v4] tty: Add driver unthrottle in ioctl(...,TCFLSH,..).
> 
> [PATCH] tty: Correct tty buffer flush.
> With Alan's ACK from 04.12.2012 for kernel <= 3.7
> 
> But for kernel 3.8 I resent this 05.12.2012
> [PATCH -next 0/2] tty: Correct tty buffer flush.
> [PATCH -next 1/2] tty: Correct tty buffer flush.
> [PATCH -next 2/2] tty: Correct tty buffer flush.
> The last I resent with new description after Jiri's remark.

How can you have more than one patch with the same subject?  Please
resend all patches that you wish to see applied, with proper Subject:
lines and acks, if you have gotten them.

thanks,

greg k-h

Re: [PATCH] tty: Fix ptmx open without closed slave.

[Peter Hurley]

Hi Ilya,

On Wed, 2012-12-19 at 23:00 +0400, Ilya Zykov wrote:
> When we are opening ptmx, we have closed pts, by description.
> Now only if we open and after close all pts' descriptions, pty_close() sets
> this bit correctly
> 
> Signed-off-by: Ilya Zykov <ilya@ilyx.ru>
> ---
>  drivers/tty/pty.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
> index 1ce1362..7b69307 100644
> --- a/drivers/tty/pty.c
> +++ b/drivers/tty/pty.c
> @@ -659,6 +659,7 @@ static int ptmx_open(struct inode *inode, struct file *filp)
>  	retval = ptm_driver->ops->open(tty, filp);
>  	if (retval)
>  		goto err_release;
> +	set_bit(TTY_OTHER_CLOSED, &tty->flags); /* THE SLAVE STILL CLOSED */

I'm not sure this is a good idea.

Ideally, if you were only trying to make the logic "more correct", this
change would be here, instead:
	mutex_unlock(&tty_mutex);

	set_bit(TTY_PTY_LOCK, &tty->flags); /* LOCK THE SLAVE */
+	set_bit(TTY_OTHER_CLOSED, &tty->flags); /* THE SLAVE STILL CLOSED */
	tty->driver_data = inode;

	tty_add_file(tty, filp);

Of course, that would be a bad idea because then the master pty_open()
would fail because of the test in pty_open().

Setting TTY_OTHER_CLOSED after the open() -- as you've done -- appears
to leave a race open when this bit is not set but while a slave open()
may still be attempted.

But as far as I can tell, this change doesn't actually affect any code
branches -- that is, doesn't actually do anything -- so no such race
exists. Is that correct?

Regards,
Peter Hurley