Skip to contentSkip to conversation

n_tty : Loss of sync following a buffer overflow

RFC PATCH In review Rupesh Sugathan 2 participants 2 messages Mar 11, 2008 Char and misc drivers

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 n_tty : Loss of sync following a buffer overflow

From: Rupesh Sugathan <[email protected]>

There seems to be a synchronization issue with the n_tty.c driver when working in canonical mode.
The n_tty rightly discards data received following a buffer overflow and hence the tty->read_cnt is not updated. However, the newline characters received following a buffer overflow seems to increment the tty-

Quoted 2 lines

canon_data index. This may result in a loss of sync between the tty- canon_data & tty->read_cnt while processing read in the read_chan().

This loss of sync might be irrecoverable even when the data is later received at a slower rate.

----
I am not very sure if there is any deliberate rationale to process the newlines even when the buffer has overflown. Please comment and review the patch

diff -pu a/drivers/char/n_tty.c b/drivers/char/n_tty.c

drivers/char/n_tty.c +11 -9
@@ -838,15 +838,17 @@ send_signal:
 				put_tty_queue(c, tty);
 
 		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++;
-			spin_unlock_irqrestore(&tty->read_lock, flags);
-			kill_fasync(&tty->fasync, SIGIO, POLL_IN);
-			if (waitqueue_active(&tty->read_wait))
-				wake_up_interruptible(&tty->read_wait);
+			if (tty->read_cnt < N_TTY_BUF_SIZE) {
+				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++;
+				spin_unlock_irqrestore(&tty->read_lock, flags);
+				kill_fasync(&tty->fasync, SIGIO, POLL_IN);
+				if (waitqueue_active(&tty->read_wait))
+					wake_up_interruptible(&tty->read_wait);
+			}
 			return;
 		}
 	}
#1 lore raw
Quoted 10 lines · Rupesh Sugathan

There seems to be a synchronization issue with the n_tty.c driver when working in canonical mode.
The n_tty rightly discards data received following a buffer overflow and hence the tty->read_cnt is not updated. However, the newline characters received following a buffer overflow seems to increment the tty- canon_data index. This may result in a loss of sync between the tty- canon_data & tty->read_cnt while processing read in the read_chan(). This loss of sync might be irrecoverable even when the data is later received at a slower rate.

Quoted 6 lines

----
I am not very sure if there is any deliberate rationale to process the newlines even when the buffer has overflown. Please comment and review the patch

diff -pu a/drivers/char/n_tty.c b/drivers/char/n_tty.c

Some more information:

This is not purely theoretical. Rupesh is actually
working a problem where he has seen the N_TTY buffer overflow and the canonical state variables get out of sync with read_cnt, etc.

This patch seems obviously correct, if N_TTY drops a char due to a full buffer the rest of the character processing should not take place. Particularly in this case where the extra processing leaves the tty structure in an inconsistent state.

--
Paul Fulghum
Microgate Systems, Ltd