| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 27 Jul 2009 19:28:53 +0530
From: "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>
To: Alan Cox <alan@...rguk.ukuu.org.uk>
Cc: OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>,
Linus Torvalds <torvalds@...ux-foundation.org>,
"Rafael J. Wysocki" <rjw@...k.pl>, Ray Lee <ray-lk@...rabbit.org>,
LKML <linux-kernel@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] kdesu broken
On Mon, Jul 27, 2009 at 02:23:03PM +0100, Alan Cox wrote:
> > 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@...ux.intel.com>
> 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@...ux.intel.com>
>
> 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 */
>
>
With this patch i still have "the compile in emacs" problem. But it is less frequent.
1 in 5 tries fail now
-aneesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists