After upgrading Debian to Buster, I have began to notice console
mangling when using zsh in Dom0. This is happenning because output sent by
zsh to the console may contain NULs in the middle of the buffer.
The actual implementation of CONSOLEIO_write considers that a buffer
always terminate with a NUL and therefore will ignore anything after it.
In general, NULs are perfectly legitimate in terminal streams. For
instance, this could be used for padding slow terminals. See terminfo(5)
section `Delays and Padding`, or search for the pcre '\bpad'.
Other use cases includes using the console for dumping non-human
readable information (e.g debugger, file if no network...). With the
current behavior, the resulting stream will end up to be corrupted.
The documentation for CONSOLEIO_write is pretty limited (to not say
inexistent). From the declaration, the hypercall takes a buffer and size.
So this could lead to think the NUL character is allowed in the middle of
the buffer.
This patch updates the console API to pass the size along the buffer
down so we can remove the reliance on buffer terminating by a NUL
character.
Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
---
This patch was originally sent standalone [1]. But the series grows to
include another bug found in the console code and documentation.
Change since the standalone version:
- Fix early printk on Arm
- Fix gdbstub
- Remove unecessary leading NUL character added by Xen
- Handle DomU console
- Rework the commit message
Below a small C program to repro the bug on Xen:
int main(void)
{
write(1,
"\r\33[0m\0\0\0\0\0\0\0\0\33[27m\33[24m\33[j\33[32mjulien\33[31m@\33[00m\33[36mjuno2-julieng:~\33[37m>",
75);
write(1,
"\33[K\33[32C\33[01;33m--juno2-julieng-13:44--\33[00m\33[37m\33[55D",
54);
write(1, "\33[?2004h", 8);
return 0;
}
Without this patch, the only --juno2-julieng-13:44-- will be printed in
yellow.
This patch was tested on Arm using serial console. I am not entirely
sure whether the video and PV console is correct. I would appreciate help
for testing here.
[1] https://lists.xenproject.org/archives/html/xen-devel/2019-02/msg01932.html
---
xen/arch/arm/early_printk.c | 5 ++--
xen/common/gdbstub.c | 6 ++--
xen/drivers/char/console.c | 58 +++++++++++++++++++--------------------
xen/drivers/char/consoled.c | 7 ++---
xen/drivers/char/serial.c | 8 ++++--
xen/drivers/char/xen_pv_console.c | 10 +++----
xen/drivers/video/lfb.c | 14 ++++++----
xen/drivers/video/lfb.h | 4 +--
xen/drivers/video/vga.c | 14 ++++++----
xen/include/xen/console.h | 2 +-
xen/include/xen/early_printk.h | 2 +-
xen/include/xen/pv_console.h | 4 +--
xen/include/xen/serial.h | 4 +--
xen/include/xen/video.h | 4 +--
14 files changed, 73 insertions(+), 69 deletions(-)
diff --git a/xen/arch/arm/early_printk.c b/xen/arch/arm/early_printk.c
index 97466a12b1..35a47c7229 100644
--- a/xen/arch/arm/early_printk.c
+++ b/xen/arch/arm/early_printk.c
@@ -17,9 +17,10 @@
void early_putch(char c);
void early_flush(void);
-void early_puts(const char *s)
+void early_puts(const char *s, unsigned int nr)
{
- while (*s != '\0') {
+ while ( nr-- > 0 )
+ {
if (*s == '\n')
early_putch('\r');
early_putch(*s);
diff --git a/xen/common/gdbstub.c b/xen/common/gdbstub.c
index 07095e1ec7..08a4dda9f3 100644
--- a/xen/common/gdbstub.c
+++ b/xen/common/gdbstub.c
@@ -68,7 +68,7 @@ static void gdb_smp_resume(void);
static char __initdata opt_gdb[30];
string_param("gdb", opt_gdb);
-static void gdbstub_console_puts(const char *str);
+static void gdbstub_console_puts(const char *str, unsigned int nr);
/* value <-> char (de)serialzers */
static char
@@ -546,14 +546,14 @@ __gdb_ctx = {
static struct gdb_context *gdb_ctx = &__gdb_ctx;
static void
-gdbstub_console_puts(const char *str)
+gdbstub_console_puts(const char *str, unsigned int nr)
{
const char *p;
gdb_start_packet(gdb_ctx);
gdb_write_to_packet_char('O', gdb_ctx);
- for ( p = str; *p != '\0'; p++ )
+ for ( p = str; nr > 0; p++, nr-- )
{
gdb_write_to_packet_char(hex2char((*p>>4) & 0x0f), gdb_ctx );
gdb_write_to_packet_char(hex2char((*p) & 0x0f), gdb_ctx );
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 9bbcb0f57a..b119bf980b 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -325,9 +325,9 @@ long read_console_ring(struct xen_sysctl_readconsole *op)
static char serial_rx_ring[SERIAL_RX_SIZE];
static unsigned int serial_rx_cons, serial_rx_prod;
-static void (*serial_steal_fn)(const char *) = early_puts;
+static void (*serial_steal_fn)(const char *, unsigned int nr) = early_puts;
-int console_steal(int handle, void (*fn)(const char *))
+int console_steal(int handle, void (*fn)(const char *, unsigned int nr))
{
if ( (handle == -1) || (handle != sercon_handle) )
return 0;
@@ -345,15 +345,15 @@ void console_giveback(int id)
serial_steal_fn = NULL;
}
-static void sercon_puts(const char *s)
+static void sercon_puts(const char *s, unsigned int nr)
{
if ( serial_steal_fn != NULL )
- (*serial_steal_fn)(s);
+ (*serial_steal_fn)(s, nr);
else
- serial_puts(sercon_handle, s);
+ serial_puts(sercon_handle, s, nr);
/* Copy all serial output into PV console */
- pv_console_puts(s);
+ pv_console_puts(s, nr);
}
static void dump_console_ring_key(unsigned char key)
@@ -388,8 +388,8 @@ static void dump_console_ring_key(unsigned char key)
}
buf[sofar] = '\0';
- sercon_puts(buf);
- video_puts(buf);
+ sercon_puts(buf, sofar);
+ video_puts(buf, sofar);
free_xenheap_pages(buf, order);
}
@@ -527,7 +527,7 @@ static inline void xen_console_write_debug_port(const char
*buf, size_t len)
static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int
count)
{
char kbuf[128];
- int kcount = 0;
+ unsigned int kcount = 0;
struct domain *cd = current->domain;
while ( count > 0 )
@@ -540,25 +540,22 @@ static long
guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count)
kcount = min_t(int, count, sizeof(kbuf)-1);
if ( copy_from_guest(kbuf, buffer, kcount) )
return -EFAULT;
- kbuf[kcount] = '\0';
if ( is_hardware_domain(cd) )
{
/* Use direct console output as it could be interactive */
spin_lock_irq(&console_lock);
- sercon_puts(kbuf);
- video_puts(kbuf);
+ sercon_puts(kbuf, kcount);
+ video_puts(kbuf, kcount);
#ifdef CONFIG_X86
if ( opt_console_xen )
{
- size_t len = strlen(kbuf);
-
if ( xen_guest )
- xen_hypercall_console_write(kbuf, len);
+ xen_hypercall_console_write(kbuf, kcount);
else
- xen_console_write_debug_port(kbuf, len);
+ xen_console_write_debug_port(kbuf, kcount);
}
#endif
@@ -575,19 +572,20 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count)
char *kin = kbuf, *kout = kbuf, c;
/* Strip non-printable characters */
- for ( ; ; )
+ do
{
c = *kin++;
- if ( c == '\0' || c == '\n' )
+ if ( c == '\n' )
break;
if ( isprint(c) || c == '\t' )
*kout++ = c;
- }
+ } while ( --kcount > 0 );
