xen/console: Don't treat NUL character as the end of the buffer

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@arm.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

diff --git a/xen/arch/arm/early_printk.c b/xen/arch/arm/early_printk.c
index 97466a12b1d2899deaeb4d8449028a564e3efd3f..333073d97e32d586646ab00d31cba67dded21077 100644 (file)
--- 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, size_t 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 07095e1ec74674b060c7fd4b29a41cbb7710faec..6234834a201f0c106d1ca6a9de9c505c98396f72 100644 (file)
--- 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, size_t 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, size_t 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 d728e737d1d114766a0cd8663132f76648913110..23fe0296dfc32b530ec229058299a1b4549099eb 100644 (file)
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -326,9 +326,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 *, size_t nr) = early_puts;
 
-int console_steal(int handle, void (*fn)(const char *))
+int console_steal(int handle, void (*fn)(const char *, size_t nr))
 {
     if ( (handle == -1) || (handle != sercon_handle) )
         return 0;
@@ -346,15 +346,15 @@ void console_giveback(int id)
         serial_steal_fn = NULL;
 }
 
-static void sercon_puts(const char *s)
+static void sercon_puts(const char *s, size_t 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)
@@ -387,10 +387,9 @@ static void dump_console_ring_key(unsigned char key)
         sofar += len;
         c += len;
     }
-    buf[sofar] = '\0';
 
-    sercon_puts(buf);
-    video_puts(buf);
+    sercon_puts(buf, sofar);
+    video_puts(buf, sofar);
 
     free_xenheap_pages(buf, order);
 }
@@ -528,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 )
@@ -541,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
 
@@ -576,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 );
+
             *kout = '\0';
             spin_lock(&cd->pbuf_lock);
+            kcount = kin - kbuf;
             if ( c == '\n' )
             {
-                kcount = kin - kbuf;
                 cd->pbuf[cd->pbuf_idx] = '\0';
                 guest_printk(cd, XENLOG_G_DEBUG "%s%s\n", cd->pbuf, kbuf);
                 cd->pbuf_idx = 0;
@@ -667,16 +664,16 @@ static bool_t console_locks_busted;
 
 static void __putstr(const char *str)
 {
+    size_t len = strlen(str);
+
     ASSERT(spin_is_locked(&console_lock));
 
-    sercon_puts(str);
-    video_puts(str);
+    sercon_puts(str, len);
+    video_puts(str, len);
 
 #ifdef CONFIG_X86
     if ( opt_console_xen )
     {
-        size_t len = strlen(str);
-
         if ( xen_guest )
             xen_hypercall_console_write(str, len);
         else
@@ -1250,6 +1247,7 @@ void debugtrace_printk(const char *fmt, ...)
     char          cntbuf[24];
     va_list       args;
     unsigned long flags;
+    unsigned int nr;
 
     if ( debugtrace_bytes == 0 )
         return;
@@ -1261,14 +1259,15 @@ void debugtrace_printk(const char *fmt, ...)
     ASSERT(debugtrace_buf[debugtrace_bytes - 1] == 0);
 
     va_start(args, fmt);
-    vsnprintf(buf, sizeof(buf), fmt, args);
+    nr = vscnprintf(buf, sizeof(buf), fmt, args);
     va_end(args);
 
     if ( debugtrace_send_to_console )
     {
-        snprintf(cntbuf, sizeof(cntbuf), "%u ", ++count);
-        serial_puts(sercon_handle, cntbuf);
-        serial_puts(sercon_handle, buf);
+        unsigned int n = scnprintf(cntbuf, sizeof(cntbuf), "%u ", ++count);
+
+        serial_puts(sercon_handle, cntbuf, n);
+        serial_puts(sercon_handle, buf, nr);
     }
     else
     {
@@ -1381,7 +1380,7 @@ void panic(const char *fmt, ...)
  * **************************************************************
  */
 
-static void suspend_steal_fn(const char *str) { }
+static void suspend_steal_fn(const char *str, size_t nr) { }
 static int suspend_steal_id;
 
 int console_suspend(void)

diff --git a/xen/drivers/char/consoled.c b/xen/drivers/char/consoled.c
index 552abf576625986f89a55f4af0931e1e0813e310..222e01844271a927e693d9c242e2124374bb1b3c 100644 (file)
--- a/xen/drivers/char/consoled.c
+++ b/xen/drivers/char/consoled.c
@@ -77,16 +77,13 @@ size_t consoled_guest_rx(void)
 
         if ( idx >= BUF_SZ )
         {
-            pv_console_puts(buf);
+            pv_console_puts(buf, BUF_SZ);
             idx = 0;
         }
     }
 
     if ( idx )
-    {
-        buf[idx] = '\0';
-        pv_console_puts(buf);
-    }
+        pv_console_puts(buf, idx);
 
     /* No need for a mem barrier because every character was already consumed */
     barrier();

diff --git a/xen/drivers/char/serial.c b/xen/drivers/char/serial.c
index 221a14c0923ecc810abe6f65385534dceee34752..88cd8767908a24d473079cf724cb11fa9bf03d6d 100644 (file)
--- a/xen/drivers/char/serial.c
+++ b/xen/drivers/char/serial.c
@@ -223,11 +223,10 @@ void serial_putc(int handle, char c)
     spin_unlock_irqrestore(&port->tx_lock, flags);
 }
 
-void serial_puts(int handle, const char *s)
+void serial_puts(int handle, const char *s, size_t nr)
 {
     struct serial_port *port;
     unsigned long flags;
-    char c;
 
     if ( handle == -1 )
         return;
@@ -238,8 +237,10 @@ void serial_puts(int handle, const char *s)
 
     spin_lock_irqsave(&port->tx_lock, flags);
 
-    while ( (c = *s++) != '\0' )
+    for ( ; nr > 0; nr--, s++ )
     {
+        char c = *s;
+
         if ( (c == '\n') && (handle & SERHND_COOKED) )
             __serial_putc(port, '\r' | ((handle & SERHND_HI) ? 0x80 : 0x00));
 

diff --git a/xen/drivers/char/xen_pv_console.c b/xen/drivers/char/xen_pv_console.c
index cc1c1d743f1ed6f60f6a08f14f73efdf87b7a90b..612784b0742836967015e514ed967372a7b924e5 100644 (file)
--- a/xen/drivers/char/xen_pv_console.c
+++ b/xen/drivers/char/xen_pv_console.c
@@ -129,13 +129,13 @@ size_t pv_console_rx(struct cpu_user_regs *regs)
     return recv;
 }
 
-static size_t pv_ring_puts(const char *buf)
+static size_t pv_ring_puts(const char *buf, size_t nr)
 {
     XENCONS_RING_IDX cons, prod;
     size_t sent = 0, avail;
     bool put_r = false;
 
-    while ( buf[sent] != '\0' || put_r )
+    while ( sent < nr || put_r )
     {
         cons = ACCESS_ONCE(cons_ring->out_cons);
         prod = cons_ring->out_prod;
@@ -156,7 +156,7 @@ static size_t pv_ring_puts(const char *buf)
             continue;
         }
 
-        while ( avail && (buf[sent] != '\0' || put_r) )
+        while ( avail && (sent < nr || put_r) )
         {
             if ( put_r )
             {
@@ -185,7 +185,7 @@ static size_t pv_ring_puts(const char *buf)
     return sent;
 }
 
-void pv_console_puts(const char *buf)
+void pv_console_puts(const char *buf, size_t nr)
 {
     unsigned long flags;
 
@@ -193,7 +193,7 @@ void pv_console_puts(const char *buf)
         return;
 
     spin_lock_irqsave(&tx_lock, flags);
-    pv_ring_puts(buf);
+    pv_ring_puts(buf, nr);
     spin_unlock_irqrestore(&tx_lock, flags);
 }
 

diff --git a/xen/drivers/video/lfb.c b/xen/drivers/video/lfb.c
index 5022195ae5ade10c9cc0e303449055ea8f17c806..75b749b3303be6e25bc8c2e775d9f03d15720c65 100644 (file)
--- a/xen/drivers/video/lfb.c
+++ b/xen/drivers/video/lfb.c
@@ -53,14 +53,15 @@ static void lfb_show_line(
 }
 
 /* Fast mode which redraws all modified parts of a 2D text buffer. */
-void lfb_redraw_puts(const char *s)
+void lfb_redraw_puts(const char *s, size_t nr)
 {
     unsigned int i, min_redraw_y = lfb.ypos;
-    char c;
 
     /* Paste characters into text buffer. */
-    while ( (c = *s++) != '\0' )
+    for ( ; nr > 0; nr--, s++ )
     {
+        char c = *s;
+
         if ( (c == '\n') || (lfb.xpos >= lfb.lfbp.text_columns) )
         {
             if ( ++lfb.ypos >= lfb.lfbp.text_rows )
@@ -97,13 +98,14 @@ void lfb_redraw_puts(const char *s)
 }
 
 /* Slower line-based scroll mode which interacts better with dom0. */
-void lfb_scroll_puts(const char *s)
+void lfb_scroll_puts(const char *s, size_t nr)
 {
     unsigned int i;
-    char c;
 
-    while ( (c = *s++) != '\0' )
+    for ( ; nr > 0; nr--, s++ )
     {
+        char c = *s;
+
         if ( (c == '\n') || (lfb.xpos >= lfb.lfbp.text_columns) )
         {
             unsigned int bytes = (lfb.lfbp.width *

diff --git a/xen/drivers/video/lfb.h b/xen/drivers/video/lfb.h
index ac40a6637983386b7c12b8ef3a05952df2cef370..e743ccdd6b1199dd7a3b51fdbed789eb6161e75e 100644 (file)
--- a/xen/drivers/video/lfb.h
+++ b/xen/drivers/video/lfb.h
@@ -35,8 +35,8 @@ struct lfb_prop {
     unsigned int text_rows;
 };
 
-void lfb_redraw_puts(const char *s);
-void lfb_scroll_puts(const char *s);
+void lfb_redraw_puts(const char *s, size_t nr);
+void lfb_scroll_puts(const char *s, size_t nr);
 void lfb_carriage_return(void);
 void lfb_free(void);
 

diff --git a/xen/drivers/video/vga.c b/xen/drivers/video/vga.c
index 454457ade8169be7eee70a90f98ddf470b28631d..666f2e25095ddf3d76fc8fafd1984ca66a1d40b4 100644 (file)
--- a/xen/drivers/video/vga.c
+++ b/xen/drivers/video/vga.c
@@ -18,9 +18,9 @@ static int vgacon_keep;
 static unsigned int xpos, ypos;
 static unsigned char *video;
 
-static void vga_text_puts(const char *s);
-static void vga_noop_puts(const char *s) {}
-void (*video_puts)(const char *) = vga_noop_puts;
+static void vga_text_puts(const char *s, size_t nr);
+static void vga_noop_puts(const char *s, size_t nr) {}
+void (*video_puts)(const char *, size_t nr) = vga_noop_puts;
 
 /*
  * 'vga=<mode-specifier>[,keep]' where <mode-specifier> is one of:
@@ -174,12 +174,12 @@ void __init video_endboot(void)
     }
 }
 
-static void vga_text_puts(const char *s)
+static void vga_text_puts(const char *s, size_t nr)
 {
-    char c;
-
-    while ( (c = *s++) != '\0' )
+    for ( ; nr > 0; nr--, s++ )
     {
+        char c = *s;
+
         if ( (c == '\n') || (xpos >= columns) )
         {
             if ( ++ypos >= lines )

diff --git a/xen/include/xen/console.h b/xen/include/xen/console.h
index b4f9463936aabe40975b898a0a5ba71da6a31919..ba914f9e5bcadacd73e1a057bb32847c582d3e61 100644 (file)
--- a/xen/include/xen/console.h
+++ b/xen/include/xen/console.h
@@ -38,7 +38,7 @@ struct domain *console_input_domain(void);
  * Steal output from the console. Returns +ve identifier, else -ve error.
  * Takes the handle of the serial line to steal, and steal callback function.
  */
-int console_steal(int handle, void (*fn)(const char *));
+int console_steal(int handle, void (*fn)(const char *, size_t nr));
 
 /* Give back stolen console. Takes the identifier returned by console_steal. */
 void console_giveback(int id);

diff --git a/xen/include/xen/early_printk.h b/xen/include/xen/early_printk.h
index 2c3e1b3519dcb69718c4746bb6157f8d2ca9b6dd..0f76c3a74f03ed1c5fdd0d6f76ef35e60615e7ae 100644 (file)
--- a/xen/include/xen/early_printk.h
+++ b/xen/include/xen/early_printk.h
@@ -5,7 +5,7 @@
 #define __XEN_EARLY_PRINTK_H__
 
 #ifdef CONFIG_EARLY_PRINTK
-void early_puts(const char *s);
+void early_puts(const char *s, size_t nr);
 #else
 #define early_puts NULL
 #endif

diff --git a/xen/include/xen/pv_console.h b/xen/include/xen/pv_console.h
index cb92539666b029f9c060352165c10e8de2f7dfa4..4745f46f2d1bd681d6e8eb69194bc71866f7e029 100644 (file)
--- a/xen/include/xen/pv_console.h
+++ b/xen/include/xen/pv_console.h
@@ -8,7 +8,7 @@
 void pv_console_init(void);
 void pv_console_set_rx_handler(serial_rx_fn fn);
 void pv_console_init_postirq(void);
-void pv_console_puts(const char *buf);
+void pv_console_puts(const char *buf, size_t nr);
 size_t pv_console_rx(struct cpu_user_regs *regs);
 evtchn_port_t pv_console_evtchn(void);
 
@@ -17,7 +17,7 @@ evtchn_port_t pv_console_evtchn(void);
 static inline void pv_console_init(void) {}
 static inline void pv_console_set_rx_handler(serial_rx_fn fn) { }
 static inline void pv_console_init_postirq(void) { }
-static inline void pv_console_puts(const char *buf) { }
+static inline void pv_console_puts(const char *buf, size_t nr) { }
 static inline size_t pv_console_rx(struct cpu_user_regs *regs) { return 0; }
 evtchn_port_t pv_console_evtchn(void)
 {

diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
index f2994d40932988c50fd8799560cc945ce5ff3316..6548f0b0a9cfe72b170dfd1d6dfcb57818fd719b 100644 (file)
--- a/xen/include/xen/serial.h
+++ b/xen/include/xen/serial.h
@@ -114,8 +114,8 @@ int serial_parse_handle(char *conf);
 /* Transmit a single character via the specified COM port. */
 void serial_putc(int handle, char c);
 
-/* Transmit a NULL-terminated string via the specified COM port. */
-void serial_puts(int handle, const char *s);
+/* Transmit a string via the specified COM port. */
+void serial_puts(int handle, const char *s, size_t nr);
 
 /*
  * An alternative to registering a character-receive hook. This function

diff --git a/xen/include/xen/video.h b/xen/include/xen/video.h
index 2e897f9df56c598074c57c803b43e4773544263d..96f8a50132980d69dac3a598975ac7c5234226ee 100644 (file)
--- a/xen/include/xen/video.h
+++ b/xen/include/xen/video.h
@@ -13,11 +13,11 @@
 
 #ifdef CONFIG_VIDEO
 void video_init(void);
-extern void (*video_puts)(const char *);
+extern void (*video_puts)(const char *, size_t nr);
 void video_endboot(void);
 #else
 #define video_init()    ((void)0)
-#define video_puts(s)   ((void)0)
+static inline void video_puts(const char *str, size_t nr) {}
 #define video_endboot() ((void)0)
 #endif