Safer flush_to_ldisc()

Safer flush_to_ldisc()

[Johan.Adolfsson]

There is currently (and has always been it seems) a problem
with flush_to_ldisc().
The below diff is against 2.4.20 and contains some stuff made in 2.5,
as well, but the important thing is to set TTY_DONT_FLIP
while processing the flip buffer.
Otherwise, on slow systems or systems with high load and
fast flipping (high baudrates) a new flip might start before
the next flip has been processed, resulting in that characters
could be lost or perhaps arrive out of order.
A driver can write data to a flip buffer that really isn't processed yet.

At least that is what I think happens...
Does it make sense?

/Johan



--- linux/drivers/char/tty_io.c    2 Dec 2002 08:13:09 -0000       1.16
+++ linux/drivers/char/tty_io.c    30 Jul 2003 10:06:04 -0000
@@ -1910,24 +1910,23 @@ static void flush_to_ldisc(void *private
        int             count;
        unsigned long flags;

-       if (test_bit(TTY_DONT_FLIP, &tty->flags)) {
+       /* Have TTY_DONT_FLIP set while processing the flip buffer */
+       if (test_and_set_bit(TTY_DONT_FLIP, &tty->flags)) {
+               /* Already set, process later. */
                queue_task(&tty->flip.tqueue, &tq_timer);
                return;
        }
+       save_flags(flags); cli();
        if (tty->flip.buf_num) {
                cp = tty->flip.char_buf + TTY_FLIPBUF_SIZE;
                fp = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
-               tty->flip.buf_num = 0;
-
-               save_flags(flags); cli();
+               tty->flip.buf_num = 0;
                tty->flip.char_buf_ptr = tty->flip.char_buf;
                tty->flip.flag_buf_ptr = tty->flip.flag_buf;
        } else {
                cp = tty->flip.char_buf;
                fp = tty->flip.flag_buf;
-               tty->flip.buf_num = 1;
-
-               save_flags(flags); cli();
+               tty->flip.buf_num = 1;
                tty->flip.char_buf_ptr = tty->flip.char_buf +
TTY_FLIPBUF_SIZE;
                tty->flip.flag_buf_ptr = tty->flip.flag_buf +
TTY_FLIPBUF_SIZE;
        }
@@ -1936,6 +1935,7 @@ static void flush_to_ldisc(void *private
        restore_flags(flags);

        tty->ldisc.receive_buf(tty, cp, fp, count);
+       clear_bit(TTY_DONT_FLIP, &tty->flags);
 }

Re: Safer flush_to_ldisc()

[Johan.Adolfsson]

Could the silence on this matter be interpreted as
1. No one knows how the tty flipping works (including me I guess)?
2. No one cares about slow machines with high interrupt load?
3. Bad timing, try again...

/Johan

----- Original Message ----- 
From: <johana@axis.com>
To: <linux-kernel@vger.kernel.org>
Sent: Wednesday, July 30, 2003 12:16 PM
Subject: Safer flush_to_ldisc()


> There is currently (and has always been it seems) a problem
> with flush_to_ldisc().
> The below diff is against 2.4.20 and contains some stuff made in 2.5,
> as well, but the important thing is to set TTY_DONT_FLIP
> while processing the flip buffer.
> Otherwise, on slow systems or systems with high load and
> fast flipping (high baudrates) a new flip might start before
> the next flip has been processed, resulting in that characters
> could be lost or perhaps arrive out of order.
> A driver can write data to a flip buffer that really isn't processed yet.
>
> At least that is what I think happens...
> Does it make sense?
>
> /Johan
>
>
>
> --- linux/drivers/char/tty_io.c    2 Dec 2002 08:13:09 -0000       1.16
> +++ linux/drivers/char/tty_io.c    30 Jul 2003 10:06:04 -0000
> @@ -1910,24 +1910,23 @@ static void flush_to_ldisc(void *private
>         int             count;
>         unsigned long flags;
>
> -       if (test_bit(TTY_DONT_FLIP, &tty->flags)) {
> +       /* Have TTY_DONT_FLIP set while processing the flip buffer */
> +       if (test_and_set_bit(TTY_DONT_FLIP, &tty->flags)) {
> +               /* Already set, process later. */
>                 queue_task(&tty->flip.tqueue, &tq_timer);
>                 return;
>         }
> +       save_flags(flags); cli();
>         if (tty->flip.buf_num) {
>                 cp = tty->flip.char_buf + TTY_FLIPBUF_SIZE;
>                 fp = tty->flip.flag_buf + TTY_FLIPBUF_SIZE;
> -               tty->flip.buf_num = 0;
> -
> -               save_flags(flags); cli();
> +               tty->flip.buf_num = 0;
>                 tty->flip.char_buf_ptr = tty->flip.char_buf;
>                 tty->flip.flag_buf_ptr = tty->flip.flag_buf;
>         } else {
>                 cp = tty->flip.char_buf;
>                 fp = tty->flip.flag_buf;
> -               tty->flip.buf_num = 1;
> -
> -               save_flags(flags); cli();
> +               tty->flip.buf_num = 1;
>                 tty->flip.char_buf_ptr = tty->flip.char_buf +
> TTY_FLIPBUF_SIZE;
>                 tty->flip.flag_buf_ptr = tty->flip.flag_buf +
> TTY_FLIPBUF_SIZE;
>         }
> @@ -1936,6 +1935,7 @@ static void flush_to_ldisc(void *private
>         restore_flags(flags);
>
>         tty->ldisc.receive_buf(tty, cp, fp, count);
> +       clear_bit(TTY_DONT_FLIP, &tty->flags);
>  }
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>