From 06c835f85709874c849208a2659ff3546870aa92 Mon Sep 17 00:00:00 2001
From: Daniel Bertalan <dani@danielbertalan.dev>
Date: Sun, 23 May 2021 23:16:00 +0200
Subject: [PATCH] Kernel: Signal EOF/EOL characters properly in `TTY`

I introduced a regression in #7184 where `TTY` would report 1 byte read
in canonical mode even if we had no more characters left. This was
caused by counting the '\0' that denotes EOF into the number of
characters that were read.

The fix was simple: exclude the EOF character from the number of bytes.

This still wouldn't be correct by itself, as the EOF and EOL control
characters could change between when the data was written to the TTY and
when it is read. We fix this by signaling out-of-band whether something
is a special character. End-of-file markers have a value of zero and
have their special bits set. Any other bytes with a special flag are
treated as line endings. This is possible, as POSIX doesn't allow
special characters to be 0.

Fixes #7419
---
 Kernel/TTY/TTY.cpp | 43 +++++++++++++++++++++++++++++--------------
 Kernel/TTY/TTY.h   |  7 ++++++-
 2 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/Kernel/TTY/TTY.cpp b/Kernel/TTY/TTY.cpp
index ce7c21a678..f32e50b5ed 100644
--- a/Kernel/TTY/TTY.cpp
+++ b/Kernel/TTY/TTY.cpp
@@ -53,23 +53,27 @@ KResultOr<size_t> TTY::read(FileDescription&, u64, UserOrKernelBuffer& buffer, s
 
     bool need_evaluate_block_conditions = false;
     auto result = buffer.write_buffered<512>(size, [&](u8* data, size_t data_size) {
-        for (size_t i = 0; i < data_size; ++i) {
-            u8 ch = m_input_buffer.dequeue();
-            if (in_canonical_mode()) {
+        size_t bytes_written = 0;
+        for (; bytes_written < data_size; ++bytes_written) {
+            auto bit_index = m_input_buffer.head_index();
+            bool is_special_character = m_special_character_bitmask[bit_index / 8] & (1 << (bit_index % 8));
+            if (in_canonical_mode() && is_special_character) {
+                u8 ch = m_input_buffer.dequeue();
                 if (ch == '\0') {
+                    // EOF
                     m_available_lines--;
                     need_evaluate_block_conditions = true;
                     break;
-                } else if (ch == '\n' || is_eol(ch)) {
-                    data[i] = ch;
-                    i++;
+                } else {
+                    // '\n' or EOL
+                    data[bytes_written++] = ch;
                     m_available_lines--;
                     break;
                 }
             }
-            data[i] = ch;
+            data[bytes_written] = m_input_buffer.dequeue();
         }
-        return data_size;
+        return bytes_written;
     });
     if ((!result.is_error() && result.value() > 0) || need_evaluate_block_conditions)
         evaluate_block_conditions();
@@ -203,11 +207,20 @@ void TTY::emit(u8 ch, bool do_evaluate_block_conditions)
     else if (ch == '\n' && (m_termios.c_iflag & INLCR))
         ch = '\r';
 
+    auto current_char_head_index = (m_input_buffer.head_index() + m_input_buffer.size()) % TTY_BUFFER_SIZE;
+    m_special_character_bitmask[current_char_head_index / 8] &= ~(1u << (current_char_head_index % 8));
+
+    auto set_special_bit = [&] {
+        m_special_character_bitmask[current_char_head_index / 8] |= (1u << (current_char_head_index % 8));
+    };
+
     if (in_canonical_mode()) {
         if (is_eof(ch)) {
+            // Since EOF might change between when the data came in and when it is read,
+            // we use '\0' along with the bitmask to signal EOF. Any non-zero byte with
+            // the special bit set signals an end-of-line.
+            set_special_bit();
             m_available_lines++;
-            //We use '\0' to delimit the end
-            //of a line.
             m_input_buffer.enqueue('\0');
             return;
         }
@@ -224,17 +237,19 @@ void TTY::emit(u8 ch, bool do_evaluate_block_conditions)
             return;
         }
 
-        if (is_eol(ch)) {
-            m_available_lines++;
-        }
-
         if (ch == '\n') {
             if (m_termios.c_lflag & ECHO || m_termios.c_lflag & ECHONL)
                 echo('\n');
+            set_special_bit();
             m_input_buffer.enqueue('\n');
             m_available_lines++;
             return;
         }
+
+        if (is_eol(ch)) {
+            set_special_bit();
+            m_available_lines++;
+        }
     }
 
     m_input_buffer.enqueue(ch);
diff --git a/Kernel/TTY/TTY.h b/Kernel/TTY/TTY.h
index 3f5bd0efd2..a34961bad3 100644
--- a/Kernel/TTY/TTY.h
+++ b/Kernel/TTY/TTY.h
@@ -13,6 +13,8 @@
 #include <Kernel/ProcessGroup.h>
 #include <Kernel/UnixTypes.h>
 
+#define TTY_BUFFER_SIZE 1024
+
 namespace Kernel {
 
 class TTY : public CharacterDevice {
@@ -79,7 +81,10 @@ private:
     // ^CharacterDevice
     virtual bool is_tty() const final override { return true; }
 
-    CircularDeque<u8, 1024> m_input_buffer;
+    CircularDeque<u8, TTY_BUFFER_SIZE> m_input_buffer;
+    // FIXME: use something like AK::Bitmap but which takes a size template parameter
+    u8 m_special_character_bitmask[TTY_BUFFER_SIZE / 8];
+
     WeakPtr<Process> m_original_process_parent;
     WeakPtr<ProcessGroup> m_pg;
     termios m_termios;