From 0a818095d9f0a38edb7e56130de54d0682c357aa Mon Sep 17 00:00:00 2001 From: Nathan Wang Date: Thu, 1 Feb 2024 00:12:20 +0000 Subject: [PATCH] Add partial support for VERASE and VWERASE in canonical mode in PTYs --- pkg/sentry/fsimpl/devpts/line_discipline.go | 89 ++++++++++++ test/syscalls/linux/pty.cc | 152 +++++++++++++++----- 2 files changed, 209 insertions(+), 32 deletions(-) diff --git a/pkg/sentry/fsimpl/devpts/line_discipline.go b/pkg/sentry/fsimpl/devpts/line_discipline.go index 714554c8c..8d99656b8 100644 --- a/pkg/sentry/fsimpl/devpts/line_discipline.go +++ b/pkg/sentry/fsimpl/devpts/line_discipline.go @@ -16,6 +16,7 @@ package devpts import ( "bytes" + "unicode" "unicode/utf8" "gvisor.dev/gvisor/pkg/abi/linux" @@ -450,6 +451,94 @@ func (*inputQueueTransformer) transform(l *lineDiscipline, q *queue, buf []byte) l.terminal.replicaKTTY.SignalForegroundProcessGroup(kernel.SignalInfoPriv(linux.SIGTSTP)) case l.termios.ControlCharacters[linux.VQUIT]: // ctrl-\ l.terminal.replicaKTTY.SignalForegroundProcessGroup(kernel.SignalInfoPriv(linux.SIGQUIT)) + + // In canonical mode, some characters need to be handled specially; for example, backspace. + // This roughly aligns with n_tty.c:n_tty_receive_char_canon and n_tty.c:eraser + // cBytes[0] == ControlCharacters[linux.VKILL] is also handled by n_tty.c:eraser, but this isn't implemented + case l.termios.ControlCharacters[linux.VWERASE]: + if !l.termios.LEnabled(linux.IEXTEN) { + break + } + fallthrough + case l.termios.ControlCharacters[linux.VERASE]: + if !l.termios.LEnabled(linux.ICANON) { + break + } + + c := cBytes[0] + killType := linux.VERASE + if c == l.termios.ControlCharacters[linux.VWERASE] { + killType = linux.VWERASE + } + seenAlphanumeric := false + for len(q.readBuf) > 0 { + // Erase a character. If IUTF8 is enabled, erase an entire multibyte unicode character. + var toErase byte + cnt := 0 + isContinuationByte := true + for ; cnt < len(q.readBuf) && isContinuationByte; cnt++ { + toErase = q.readBuf[len(q.readBuf)-cnt-1] + isContinuationByte = l.termios.IEnabled(linux.IUTF8) && (toErase&0xc0) == 0x80 + } + if isContinuationByte { + // Do not partially erase a multibyte unicode character. + break + } + + // VWERASE will continue erasing characters until we encounter the first non-alphanumeric character + // that follows some alphanumeric character. We consider "_" to be alphanumeric. + if killType == linux.VWERASE { + if unicode.IsLetter(rune(toErase)) || unicode.IsDigit(rune(toErase)) || toErase == '_' { + seenAlphanumeric = true + } else if seenAlphanumeric { + break + } + } + + q.readBuf = q.readBuf[:len(q.readBuf)-cnt] + if l.termios.LEnabled(linux.ECHO) { + if l.termios.LEnabled(linux.ECHOPRT) { + // Not implemented + } else if killType == linux.VERASE && !l.termios.LEnabled(linux.ECHOE) { + // Not implemented + } else if toErase == '\t' { + // Not implemented + } else { + const unicodeDelete byte = 0x7f + isCtrl := toErase < 0x20 || toErase == unicodeDelete + echoctl := l.termios.LEnabled(linux.ECHOCTL) + + charsToDelete := 1 + if isCtrl { + // echoctl controls how we echo control characters, which also determines how we delete them. + if echoctl { + // echoctl echoes control characters as ^X, so we need to erase two characters. + charsToDelete = 2 + } else { + // if echoctl is disabled, we don't echo control characters so we don't have to erase anything. + charsToDelete = 0 + } + } + for i := 0; i < charsToDelete; i++ { + // Linux's kernel does character deletion with this sequence + // of bytes, presumably because some older terminals don't erase + // characters with \b, so we need to "erase" the old character + // by writing a space over it. + l.outQueue.writeBytes([]byte{'\b', ' ', '\b'}, l) + } + } + } + + // VERASE only erases a single character + if killType == linux.VERASE { + break + } + } + + buf = buf[1:] + ret += 1 + notifyEcho = true + continue } // In canonical mode, we discard non-terminating characters diff --git a/test/syscalls/linux/pty.cc b/test/syscalls/linux/pty.cc index 27da24810..8c08d64a9 100644 --- a/test/syscalls/linux/pty.cc +++ b/test/syscalls/linux/pty.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -29,9 +30,8 @@ #include #include +#include -#include "gmock/gmock.h" -#include "gtest/gtest.h" #include "absl/base/macros.h" #include "absl/strings/str_cat.h" #include "absl/synchronization/notification.h" @@ -48,6 +48,8 @@ #include "test/util/temp_path.h" #include "test/util/test_util.h" #include "test/util/thread_util.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" namespace gvisor { namespace testing { @@ -92,8 +94,8 @@ struct kernel_termios { cc_t c_cc[KERNEL_NCCS]; }; -bool operator==(struct kernel_termios const& a, - struct kernel_termios const& b) { +bool operator==(struct kernel_termios const &a, + struct kernel_termios const &b) { return memcmp(&a, &b, sizeof(a)) == 0; } @@ -116,14 +118,14 @@ constexpr char FromControlCharacter(char c) { return c + 'A' - 1; } constexpr bool IsControlCharacter(char c) { return c <= 31; } struct Field { - const char* name; + const char *name; uint64_t mask; uint64_t value; }; // ParseFields returns a string representation of value, using the names in // fields. -std::string ParseFields(const Field* fields, size_t len, uint64_t value) { +std::string ParseFields(const Field *fields, size_t len, uint64_t value) { bool first = true; std::string s; for (size_t i = 0; i < len; i++) { @@ -287,7 +289,7 @@ std::string FormatCC(char c) { return absl::StrCat("\\x", absl::Hex(c)); } -std::ostream& operator<<(std::ostream& os, struct kernel_termios const& a) { +std::ostream &operator<<(std::ostream &os, struct kernel_termios const &a) { os << "{ c_iflag = " << ParseFields(kIflagFields, ABSL_ARRAYSIZE(kIflagFields), a.c_iflag); os << ", c_oflag = " @@ -351,7 +353,7 @@ struct kernel_termios DefaultTermios() { // Returns a partial read if some bytes were read. // // fd must be non-blocking. -PosixErrorOr PollAndReadFd(int fd, void* buf, size_t count, +PosixErrorOr PollAndReadFd(int fd, void *buf, size_t count, absl::Duration timeout) { absl::Time end = absl::Now() + timeout; @@ -370,7 +372,7 @@ PosixErrorOr PollAndReadFd(int fd, void* buf, size_t count, } ssize_t n = - ReadFd(fd, static_cast(buf) + completed, count - completed); + ReadFd(fd, static_cast(buf) + completed, count - completed); if (n < 0) { if (errno == EAGAIN) { // Linux sometimes returns EAGAIN from this read, despite the fact that @@ -465,14 +467,14 @@ PosixErrorOr WaitUntilReceived(int fd, int count) { } // Verifies that there is nothing left to read from fd. -void ExpectFinished(const FileDescriptor& fd) { +void ExpectFinished(const FileDescriptor &fd) { // Nothing more to read. char c; EXPECT_THAT(ReadFd(fd.get(), &c, 1), SyscallFailsWithErrno(EAGAIN)); } // Verifies that we can read expected bytes from fd into buf. -void ExpectReadable(const FileDescriptor& fd, int expected, char* buf) { +void ExpectReadable(const FileDescriptor &fd, int expected, char *buf) { size_t n = ASSERT_NO_ERRNO_AND_VALUE( PollAndReadFd(fd.get(), buf, expected, kTimeout)); EXPECT_EQ(expected, n); @@ -631,7 +633,7 @@ TEST(BasicPtyTest, SetMode) { } class PtyTest : public ::testing::Test { - protected: +protected: void SetUp() override { master_ = ASSERT_NO_ERRNO_AND_VALUE(Open("/dev/ptmx", O_RDWR | O_NONBLOCK)); replica_ = ASSERT_NO_ERRNO_AND_VALUE(OpenReplica(master_)); @@ -651,6 +653,28 @@ class PtyTest : public ::testing::Test { EXPECT_THAT(ioctl(replica_.get(), TCSETS, &t), SyscallSucceeds()); } + // Writes master_input to the master file descriptor and verifies that + // the replica and the echo output match what is expected. + void TestCanonicalIO(const char *master_input, + const char *expected_replica_output, + const char *expected_echo_output) { + ASSERT_THAT(WriteFd(master_.get(), master_input, strlen(master_input)), + SyscallSucceedsWithValue(strlen(master_input))); + + std::string buf(strlen(expected_replica_output), '\0'); + ASSERT_NO_ERRNO( + WaitUntilReceived(replica_.get(), strlen(expected_replica_output))); + ExpectReadable(replica_, strlen(expected_replica_output), &buf[0]); + EXPECT_STREQ(buf.c_str(), expected_replica_output); + + std::string echo_buf(strlen(expected_echo_output), '\0'); + ExpectReadable(master_, strlen(expected_echo_output), &echo_buf[0]); + EXPECT_STREQ(echo_buf.c_str(), expected_echo_output); + + ExpectFinished(master_); + ExpectFinished(replica_); + } + // Master and replica ends of the PTY. Non-blocking. FileDescriptor master_; FileDescriptor replica_; @@ -783,7 +807,7 @@ TEST_F(PtyTest, MasterTermiosUnchangable) { ASSERT_THAT(WriteFd(replica_.get(), &c, 1), SyscallSucceedsWithValue(1)); ExpectReadable(master_, 1, &c); - EXPECT_EQ(c, '\r'); // ICRNL had no effect! + EXPECT_EQ(c, '\r'); // ICRNL had no effect! ExpectFinished(master_); } @@ -792,7 +816,7 @@ TEST_F(PtyTest, MasterTermiosUnchangable) { TEST_F(PtyTest, TermiosICRNL) { struct kernel_termios t = DefaultTermios(); t.c_iflag |= ICRNL; - t.c_lflag &= ~ICANON; // for byte-by-byte reading. + t.c_lflag &= ~ICANON; // for byte-by-byte reading. ASSERT_THAT(ioctl(replica_.get(), TCSETS, &t), SyscallSucceeds()); char c = '\r'; @@ -808,7 +832,7 @@ TEST_F(PtyTest, TermiosICRNL) { TEST_F(PtyTest, TermiosONLCR) { struct kernel_termios t = DefaultTermios(); t.c_oflag |= ONLCR; - t.c_lflag &= ~ICANON; // for byte-by-byte reading. + t.c_lflag &= ~ICANON; // for byte-by-byte reading. ASSERT_THAT(ioctl(replica_.get(), TCSETS, &t), SyscallSucceeds()); char c = '\n'; @@ -826,7 +850,7 @@ TEST_F(PtyTest, TermiosONLCR) { TEST_F(PtyTest, TCSETSFTermiosICRNL) { struct kernel_termios t = DefaultTermios(); t.c_iflag |= ICRNL; - t.c_lflag &= ~ICANON; // for byte-by-byte reading. + t.c_lflag &= ~ICANON; // for byte-by-byte reading. ASSERT_THAT(ioctl(replica_.get(), TCSETSF, &t), SyscallSucceeds()); char c = '\r'; @@ -842,7 +866,7 @@ TEST_F(PtyTest, TCSETSFTermiosICRNL) { TEST_F(PtyTest, TCSETSFTermiosONLCR) { struct kernel_termios t = DefaultTermios(); t.c_oflag |= ONLCR; - t.c_lflag &= ~ICANON; // for byte-by-byte reading. + t.c_lflag &= ~ICANON; // for byte-by-byte reading. ASSERT_THAT(ioctl(replica_.get(), TCSETSF, &t), SyscallSucceeds()); char c = '\n'; @@ -859,7 +883,7 @@ TEST_F(PtyTest, TCSETSFTermiosONLCR) { TEST_F(PtyTest, TermiosIGNCR) { struct kernel_termios t = DefaultTermios(); t.c_iflag |= IGNCR; - t.c_lflag &= ~ICANON; // for byte-by-byte reading. + t.c_lflag &= ~ICANON; // for byte-by-byte reading. ASSERT_THAT(ioctl(replica_.get(), TCSETS, &t), SyscallSucceeds()); char c = '\r'; @@ -874,7 +898,7 @@ TEST_F(PtyTest, TermiosIGNCR) { TEST_F(PtyTest, TermiosPollReplica) { struct kernel_termios t = DefaultTermios(); t.c_iflag |= IGNCR; - t.c_lflag &= ~ICANON; // for byte-by-byte reading. + t.c_lflag &= ~ICANON; // for byte-by-byte reading. ASSERT_THAT(ioctl(replica_.get(), TCSETS, &t), SyscallSucceeds()); absl::Notification notify; @@ -904,7 +928,7 @@ TEST_F(PtyTest, TermiosPollReplica) { TEST_F(PtyTest, TermiosPollMaster) { struct kernel_termios t = DefaultTermios(); t.c_iflag |= IGNCR; - t.c_lflag &= ~ICANON; // for byte-by-byte reading. + t.c_lflag &= ~ICANON; // for byte-by-byte reading. ASSERT_THAT(ioctl(master_.get(), TCSETS, &t), SyscallSucceeds()); absl::Notification notify; @@ -933,7 +957,7 @@ TEST_F(PtyTest, TermiosPollMaster) { TEST_F(PtyTest, TermiosINLCR) { struct kernel_termios t = DefaultTermios(); t.c_iflag |= INLCR; - t.c_lflag &= ~ICANON; // for byte-by-byte reading. + t.c_lflag &= ~ICANON; // for byte-by-byte reading. ASSERT_THAT(ioctl(replica_.get(), TCSETS, &t), SyscallSucceeds()); char c = '\n'; @@ -948,7 +972,7 @@ TEST_F(PtyTest, TermiosINLCR) { TEST_F(PtyTest, TermiosONOCR) { struct kernel_termios t = DefaultTermios(); t.c_oflag |= ONOCR; - t.c_lflag &= ~ICANON; // for byte-by-byte reading. + t.c_lflag &= ~ICANON; // for byte-by-byte reading. ASSERT_THAT(ioctl(replica_.get(), TCSETS, &t), SyscallSucceeds()); // The terminal is at column 0, so there should be no CR to read. @@ -984,7 +1008,7 @@ TEST_F(PtyTest, TermiosONOCR) { TEST_F(PtyTest, TermiosOCRNL) { struct kernel_termios t = DefaultTermios(); t.c_oflag |= OCRNL; - t.c_lflag &= ~ICANON; // for byte-by-byte reading. + t.c_lflag &= ~ICANON; // for byte-by-byte reading. ASSERT_THAT(ioctl(replica_.get(), TCSETS, &t), SyscallSucceeds()); // The terminal is at column 0, so there should be no CR to read. @@ -1025,6 +1049,70 @@ TEST_F(PtyTest, VEOLTermination) { ExpectFinished(replica_); } +// Tests that sending "backspace" to the master fd will be handled properly +// in canonical mode +TEST_F(PtyTest, CanonInputBackspace) { + constexpr char kInput[] = "gvisor\x7f\n"; + constexpr char kExpectedOutput[] = "gviso\n"; + constexpr char kEchoExpectedOutput[] = "gvisor\b \b\r\n"; + + TestCanonicalIO(kInput, kExpectedOutput, kEchoExpectedOutput); +} + +// one backspace should delete the entire multibyte character when IUTF8 is set +TEST_F(PtyTest, CanonInputBackspaceMultibyteCharacterWithIUTF8) { + struct kernel_termios t = DefaultTermios(); + t.c_iflag |= IUTF8; + ASSERT_THAT(ioctl(replica_.get(), TCSETS, &t), SyscallSucceeds()); + + constexpr char kInput[] = "gviso\xc6\xa9\x7f\n"; + constexpr char kExpectedOutput[] = "gviso\n"; + constexpr char kEchoExpectedOutput[] = "gviso\xc6\xa9\b \b\r\n"; + + TestCanonicalIO(kInput, kExpectedOutput, kEchoExpectedOutput); +} + +// one backspace should only delete one byte in a multibyte character +// if IUTF8 is not set +TEST_F(PtyTest, CanonInputBackspaceMultibyteCharacterWithoutIUTF8) { + constexpr char kInput[] = "gviso\xc6\xa9\x7f\n"; + constexpr char kExpectedOutput[] = "gviso\xc6\n"; + constexpr char kEchoExpectedOutput[] = "gviso\xc6\xa9\b \b\r\n"; + + TestCanonicalIO(kInput, kExpectedOutput, kEchoExpectedOutput); +} + +// backspace should not partially delete a multibyte character when IUTF8 is set +TEST_F(PtyTest, CanonInputBackspaceMultibyteCharacterPartialWithIUTF8) { + struct kernel_termios t = DefaultTermios(); + t.c_iflag |= IUTF8; + ASSERT_THAT(ioctl(replica_.get(), TCSETS, &t), SyscallSucceeds()); + + constexpr char kInput[] = "\x80\x7f\n"; + constexpr char kExpectedOutput[] = "\x80\n"; + constexpr char kEchoExpectedOutput[] = "\x80\r\n"; + + TestCanonicalIO(kInput, kExpectedOutput, kEchoExpectedOutput); +} + +// backspace can partially delete a multibyte character when IUTF8 is not set +TEST_F(PtyTest, CanonInputBackspaceMultibyteCharacterPartialWithoutIUTF8) { + constexpr char kInput[] = "\x80\x7f\n"; + constexpr char kExpectedOutput[] = "\n"; + constexpr char kEchoExpectedOutput[] = "\x80\b \b\r\n"; + + TestCanonicalIO(kInput, kExpectedOutput, kEchoExpectedOutput); +} + +// ^W, \x17, should erase a word +TEST_F(PtyTest, CanonInputWordErase) { + constexpr char kInput[] = "hello hi\x17\n"; + constexpr char kExpectedOutput[] = "hello \n"; + constexpr char kEchoExpectedOutput[] = "hello hi\b \b\b \b\r\n"; + + TestCanonicalIO(kInput, kExpectedOutput, kEchoExpectedOutput); +} + // Tests that we can write more than the 4096 character limit, then a // terminating character, then read out just the first 4095 bytes plus the // terminator. @@ -1314,7 +1402,7 @@ TEST_F(PtyTest, SwitchTwiceMultiline) { std::string kExpected = "GO\nBLUE\n!"; // Write each line. - for (const std::string& input : kInputs) { + for (const std::string &input : kInputs) { ASSERT_THAT(WriteFd(master_.get(), input.c_str(), input.size()), SyscallSucceedsWithValue(input.size())); } @@ -1354,18 +1442,18 @@ TEST_F(PtyTest, QueueSize) { TEST_F(PtyTest, PartialBadBuffer) { // Allocate 2 pages. - void* addr = mmap(nullptr, 2 * kPageSize, PROT_READ | PROT_WRITE, + void *addr = mmap(nullptr, 2 * kPageSize, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); ASSERT_NE(addr, MAP_FAILED); - char* buf = reinterpret_cast(addr); + char *buf = reinterpret_cast(addr); // Guard the 2nd page for our read to run into. ASSERT_THAT( - mprotect(reinterpret_cast(buf + kPageSize), kPageSize, PROT_NONE), + mprotect(reinterpret_cast(buf + kPageSize), kPageSize, PROT_NONE), SyscallSucceeds()); // Leave only one free byte in the buffer. - char* bad_buffer = buf + kPageSize - 1; + char *bad_buffer = buf + kPageSize - 1; // Write to the master. constexpr char kBuf[] = "hello\n"; @@ -1450,7 +1538,7 @@ TEST_F(PtyTest, SetMasterWindowSize) { } class JobControlTest : public ::testing::Test { - protected: +protected: void SetUp() override { master_ = ASSERT_NO_ERRNO_AND_VALUE(Open("/dev/ptmx", O_RDWR | O_NONBLOCK)); replica_ = ASSERT_NO_ERRNO_AND_VALUE(OpenReplica(master_)); @@ -2051,6 +2139,6 @@ TEST_F(JobControlTest, ReuseControllingTTYAfterExit) { ASSERT_NO_ERRNO(res2); } -} // namespace -} // namespace testing -} // namespace gvisor +} // namespace +} // namespace testing +} // namespace gvisor