[PATCH] kdesu broken
From: Alan Cox
Date: Mon Jul 27 2009 - 09:22:40 EST
> I worried "pair->packet = 0" when I'm thinking this. I guess it would be
> changed more early than before. Is it ok?
I think so, and we can get stuck otherwise.
Tested patch:
commit 70325fd1d4341896c17b6f1f1965370b5258d0b1
Author: Alan Cox <alan@xxxxxxxxxxxxxxx>
Date: Mon Jul 27 14:18:52 2009 +0100
pty: ensure writes hit the reader before close
Implement TTY_EOF/EOFPENDING flags so that we can propogate the close of the
pty through the buffering correctly. The new flag state is locked but the
tty buffer lock as it relates to buffers, and also because the buffer
lock is already held in the hot path.
Signed-off-by: Alan Cox <alan@xxxxxxxxxxxxxxx>
diff --git a/drivers/char/n_tty.c b/drivers/char/n_tty.c
index ff47907..acae995 100644
--- a/drivers/char/n_tty.c
+++ b/drivers/char/n_tty.c
@@ -1777,7 +1777,8 @@ do_it_again:
tty->minimum_to_wake = (minimum - (b - buf));
if (!input_available_p(tty, 0)) {
- if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) {
+ if (test_bit(TTY_EOF, &tty->flags)) {
+ /* PTY pair closed and all data consumed */
retval = -EIO;
break;
}
diff --git a/drivers/char/pty.c b/drivers/char/pty.c
index 6e6942c..de10cc0 100644
--- a/drivers/char/pty.c
+++ b/drivers/char/pty.c
@@ -38,6 +38,9 @@ static struct tty_driver *pts_driver;
static void pty_close(struct tty_struct *tty, struct file *filp)
{
+ struct tty_struct *pair;
+ unsigned long flags;
+
BUG_ON(!tty);
if (tty->driver->subtype == PTY_TYPE_MASTER)
WARN_ON(tty->count > 1);
@@ -47,13 +50,22 @@ static void pty_close(struct tty_struct *tty, struct file *filp)
}
wake_up_interruptible(&tty->read_wait);
wake_up_interruptible(&tty->write_wait);
+
tty->packet = 0;
- if (!tty->link)
+ pair = tty->link;
+ if (!pair)
return;
- tty->link->packet = 0;
- set_bit(TTY_OTHER_CLOSED, &tty->link->flags);
- wake_up_interruptible(&tty->link->read_wait);
- wake_up_interruptible(&tty->link->write_wait);
+
+ spin_lock_irqsave(&pair->buf.lock, flags);
+ pair->packet = 0;
+ /* Indicate that the other end is now closed, set the
+ ENDPENDING marker so that the true end can be processed by
+ the line discipline */
+ set_bit(TTY_EOFPENDING, &pair->flags);
+ set_bit(TTY_OTHER_CLOSED, &pair->flags);
+ spin_unlock_irqrestore(&pair->buf.lock, flags);
+ wake_up_interruptible(&pair->read_wait);
+ wake_up_interruptible(&pair->write_wait);
if (tty->driver->subtype == PTY_TYPE_MASTER) {
set_bit(TTY_OTHER_CLOSED, &tty->flags);
#ifdef CONFIG_UNIX98_PTYS
@@ -180,7 +192,6 @@ static void pty_flush_buffer(struct tty_struct *tty)
if (!to)
return;
- /* tty_buffer_flush(to); FIXME */
if (to->packet) {
spin_lock_irqsave(&tty->ctrl_lock, flags);
tty->ctrl_status |= TIOCPKT_FLUSHWRITE;
@@ -191,23 +202,30 @@ static void pty_flush_buffer(struct tty_struct *tty)
static int pty_open(struct tty_struct *tty, struct file *filp)
{
- int retval = -ENODEV;
+ int retval = -EIO;
+ unsigned long flags;
+ struct tty_struct *pair;
- if (!tty || !tty->link)
- goto out;
+ if (tty == NULL || (pair = tty->link) == NULL)
+ return -ENODEV;
- retval = -EIO;
if (test_bit(TTY_OTHER_CLOSED, &tty->flags))
+ return -EIO;
+ spin_lock_irqsave(&pair->buf.lock, flags);
+ if (test_bit(TTY_PTY_LOCK, &pair->flags))
goto out;
- if (test_bit(TTY_PTY_LOCK, &tty->link->flags))
- goto out;
- if (tty->link->count != 1)
+ if (pair->count != 1)
goto out;
- clear_bit(TTY_OTHER_CLOSED, &tty->link->flags);
+ clear_bit(TTY_OTHER_CLOSED, &pair->flags);
+ /* The buf.lock stops this racing a flush_to_ldisc from
+ the other end */
+ clear_bit(TTY_EOFPENDING, &pair->flags);
+ clear_bit(TTY_EOF, &pair->flags);
set_bit(TTY_THROTTLED, &tty->flags);
retval = 0;
out:
+ spin_unlock_irqrestore(&pair->buf.lock, flags);
return retval;
}
diff --git a/drivers/char/tty_buffer.c b/drivers/char/tty_buffer.c
index 810ee25..19a7ced 100644
--- a/drivers/char/tty_buffer.c
+++ b/drivers/char/tty_buffer.c
@@ -119,6 +119,12 @@ static void __tty_buffer_flush(struct tty_struct *tty)
tty_buffer_free(tty, thead);
}
tty->buf.tail = NULL;
+ /* We can EOF a pty/tty pair with a flush as well as by consuming
+ all the data left over */
+ if (test_bit(TTY_EOFPENDING, &tty->flags)) {
+ set_bit(TTY_EOF, &tty->flags);
+ wake_up(&tty->read_wait);
+ }
}
/**
@@ -405,6 +411,7 @@ static void flush_to_ldisc(struct work_struct *work)
struct tty_buffer *tbuf, *head;
char *char_buf;
unsigned char *flag_buf;
+ int done = 1;
disc = tty_ldisc_ref(tty);
if (disc == NULL) /* !TTY_LDISC */
@@ -433,10 +440,13 @@ static void flush_to_ldisc(struct work_struct *work)
break;
if (!tty->receive_room) {
schedule_delayed_work(&tty->buf.work, 1);
+ done = 0;
break;
}
- if (count > tty->receive_room)
+ if (count > tty->receive_room) {
count = tty->receive_room;
+ done = 0;
+ }
char_buf = head->char_buf_ptr + head->read;
flag_buf = head->flag_buf_ptr + head->read;
head->read += count;
@@ -454,6 +464,10 @@ static void flush_to_ldisc(struct work_struct *work)
__tty_buffer_flush(tty);
clear_bit(TTY_FLUSHPENDING, &tty->flags);
wake_up(&tty->read_wait);
+ } else if (done && test_bit(TTY_EOFPENDING, &tty->flags)) {
+ /* The last bits hit the ldisc so set EOF */
+ wake_up(&tty->read_wait);
+ set_bit(TTY_EOF, &tty->flags);
}
clear_bit(TTY_FLUSHING, &tty->flags);
spin_unlock_irqrestore(&tty->buf.lock, flags);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 85aa525..427d107 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -321,6 +321,8 @@ struct tty_struct {
#define TTY_LDISC 9 /* Line discipline attached */
#define TTY_LDISC_CHANGING 10 /* Line discipline changing */
#define TTY_LDISC_OPEN 11 /* Line discipline is open */
+#define TTY_EOF 12 /* TTY/PTY pair at EOF */
+#define TTY_EOFPENDING 13 /* TTY/PTY pair EOF when data emptied */
#define TTY_HW_COOK_OUT 14 /* Hardware can do output cooking */
#define TTY_HW_COOK_IN 15 /* Hardware can do input cooking */
#define TTY_PTY_LOCK 16 /* pty private */
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/