Re: + n_tty-loss-of-sync-following-a-buffer-overflow.patch addedto -mm tree

[Paul Fulghum]

On Wed, 2008-03-12 at 09:32 -0700, Rupesh Sugathan wrote:
> The max line size from sender is 100 bytes. The read call waits for a
> maximum of 200 bytes.

OK, I understand what is happening.

Multiple lines are received while the N_TTY buffer is
full which increments canon_data multiple time without
setting multiple bits in read_flags. Essentially the
same bit in read_flags is set over and over.

When read_chan starts returning data, canon_data is
decremented once for each bit in read_flags which
leaves canon_data > 0 when there is no data to be read.

Even for the case of a single NL when N_TTY buffer is
full, the read_flag bit is set for tty->read_head when
that slot already contains data (same as tty->read_tail).

So the current code is wrong for the buffer full case.

What I suggest is the following patch for processing NL
in canonical mode.

If buffer full and previous character is not NL then
overwrite that char with NL truncating that line so
it is not prepended to subsequent data or left waiting
without a final NL.

If buffer full and previous character is NL (read_flag set)
then do nothing so canon_data remains in sync with read_flags.

Rupesh, can you try this out please?
Also, none of this answers why your N_TTY buffer is overflowing
it simply keeps things sane when it happens.

--- a/drivers/char/n_tty.c	2008-01-24 16:58:37.000000000 -0600
+++ b/drivers/char/n_tty.c	2008-03-12 12:16:06.000000000 -0500
@@ -839,10 +839,24 @@ send_signal:
 
 		handle_newline:
 			spin_lock_irqsave(&tty->read_lock, flags);
-			set_bit(tty->read_head, tty->read_flags);
-			put_tty_queue_nolock(c, tty);
-			tty->canon_head = tty->read_head;
-			tty->canon_data++;
+			if (tty->read_cnt == N_TTY_BUF_SIZE) {
+				int prev_head;
+				if (tty->read_head)
+					prev_head = tty->read_head - 1;
+				else
+					prev_head = N_TTY_BUF_SIZE - 1;
+				if (!test_bit(prev_head, tty->read_flags)) {
+					/* overwrite previous non NL char */
+					tty->read_cnt--;
+					tty->read_head = prev_head;
+				}
+			}
+			if (tty->read_cnt < N_TTY_BUF_SIZE) {
+				set_bit(tty->read_head, tty->read_flags);
+				put_tty_queue_nolock(c, tty);
+				tty->canon_head = tty->read_head;
+				tty->canon_data++;
+			}
 			spin_unlock_irqrestore(&tty->read_lock, flags);
 			kill_fasync(&tty->fasync, SIGIO, POLL_IN);
 			if (waitqueue_active(&tty->read_wait))


--
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/