Skip to contentSkip to conversation

tty: Fix ptmx open without closed slave.

PATCH In review Ilya Zykov 3 participants 5 messages Dec 19, 2012 – Jan 31, 2013 TTY layer and serial drivers

lore
Share & tools
lore

Ilya Zykov

author
#0 lore raw

PATCH 1/1 tty: Fix ptmx open without closed slave.

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 <[email protected]>
---
 drivers/tty/pty.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/tty/pty.c +1 -0
index 1ce1362..7b69307 100644
@@ -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;
maintainer
#1 lore raw
Quoted 5 lines · 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 <[email protected]>

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

author
#2 lore raw
Quoted 8 lines

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.

maintainer
#3 lore raw
Quoted 13 lines · Ilya Zykov
Quoted 8 lines

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,..).

Quoted 8 lines

[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

#4 lore raw
In reply to Ilya Zykov

Hi Ilya,

Quoted 14 lines · 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 <[email protected]>
---
 drivers/tty/pty.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
drivers/tty/pty.c +0 -0
index 1ce1362..7b69307 100644

drivers/tty/pty.c static int ptmx_open(struct inode *inode, struct file *filp) · line 662

Quoted 4 lines
drivers/tty/pty.c +1 -0
@@ -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