From patchwork Tue Feb 26 23:03:51 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Julien Grall X-Patchwork-Id: 159244 Delivered-To: patch@linaro.org Received: by 2002:a02:5cc1:0:0:0:0:0 with SMTP id w62csp3760490jad; Tue, 26 Feb 2019 15:05:55 -0800 (PST) X-Google-Smtp-Source: AHgI3Ib5yC6GP1y/ziA5WU05AqBBR0eRdFzI/1p3CLONPBLxu6dSOFmoOk5URFY3cj10yd7jrqpQ X-Received: by 2002:a81:1b02:: with SMTP id b2mr9754811ywb.268.1551222355143; Tue, 26 Feb 2019 15:05:55 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1551222355; cv=none; d=google.com; s=arc-20160816; b=avLpa4x2GMRVEASsLNa1Njqeoy9BLrWPI9XXg3CXeOrAntHfRZ6aExBbMU4cF35ddb aWTOGstzrVnkek9sCvAldJjGe6aUB/MmSiaBmWk6eWOyQogZ4XGWNlwgjq86DuZ+10Q6 RmHutJEPoTB18gEmWWtuGXIq6R6CUAg6hjV5XKscRCu5c/erw5HLX1VKWb3pk6e49HgF o1h3r9haU2ThkMO6ko3zIILrWsg2g5dp18Q3c2mqpCV3nilfM2iHwrvfPTKEWz29UTtr NS3bujI5v/Xhiod7KGc/YtdkxkA++Z+GzIVtJYXhR7YSRA2WoPhh9Egcv7pg8ieGz/2Y geQA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:mime-version:cc :list-subscribe:list-help:list-post:list-unsubscribe:list-id :precedence:subject:message-id:date:to:from; bh=oRBe8YpKHyko/QSUfw+UN2JrWsEvd+u0LsHXyf9aWLk=; b=PSvd4nhzVhnO+GDlkfyQqh4j2NgqqrrimZADabws3+OV4yK//Ci1Ai5xyvS7Qj71vD sNlfQ9s0yhA1sPCudULmjL31wk95+WiASk9UkIiXQLABTMFNkTd1hjDzigtv36VN/FMP KFYMQnDodMjhrfhGqL9oMP0MdV3nm0ufsNTUNAqGv17ICdOCQyuBbfhzV045HOWcmEnb doJ450q7dS9Rmnt0T+YjIhcSHwdhjc0pWJ/an0AlzTeybDqOz3VcK2rhOScjkgJPPkEv 4hQYiE36mVuGZaHYF63gOlLE3oGjhOoKS8EbyIYsKpNUWOetybOk9n21mjk25lnvwmms ReWQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of xen-devel-bounces@lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Return-Path: Received: from lists.xenproject.org (lists.xenproject.org. [192.237.175.120]) by mx.google.com with ESMTPS id p10si8094525ywh.180.2019.02.26.15.05.54 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 26 Feb 2019 15:05:55 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of xen-devel-bounces@lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of xen-devel-bounces@lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1gyllY-0001yc-R3; Tue, 26 Feb 2019 23:04:00 +0000 Received: from us1-rack-dfw2.inumbo.com ([104.130.134.6]) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1gyllX-0001yX-Da for xen-devel@lists.xenproject.org; Tue, 26 Feb 2019 23:03:59 +0000 X-Inumbo-ID: cc5f6203-3a1a-11e9-bc90-bc764e045a96 Received: from foss.arm.com (unknown [217.140.101.70]) by us1-rack-dfw2.inumbo.com (Halon) with ESMTP id cc5f6203-3a1a-11e9-bc90-bc764e045a96; Tue, 26 Feb 2019 23:03:58 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1F899A78; Tue, 26 Feb 2019 15:03:58 -0800 (PST) Received: from e108454-lin.cambridge.arm.com (e108454-lin.cambridge.arm.com [10.1.196.50]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2FA4E3F738; Tue, 26 Feb 2019 15:03:56 -0800 (PST) From: Julien Grall To: xen-devel@lists.xenproject.org Date: Tue, 26 Feb 2019 23:03:51 +0000 Message-Id: <20190226230351.12882-1-julien.grall@arm.com> X-Mailer: git-send-email 2.11.0 Subject: [Xen-devel] [PATCH for-4.12 RFC] xen/console: Handle NUL character in buffer sent via CONSOLEIO_write X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Stefano Stabellini , Wei Liu , Konrad Rzeszutek Wilk , George Dunlap , Andrew Cooper , Ian Jackson , Tim Deegan , Julien Grall , Jan Beulich MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" After upgrading Debian to Buster, I started noticing console mangling when using zsh. This is happenning because output sent by zsh to the console may contain NUL character in the middle of the buffer. Linux is sending the buffer as it is to Xen console via CONSOLEIO_write. However, the implementation in Xen considers NUL character is used to terminate the buffer and therefore will ignore anything after it. The actual documentation of CONSOLEIO_write is pretty limited. 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 --- This is an early RFC to start getting feedback on the issue and raise awareness on the problem. This patch is candidate for Xen 4.12. Without it zsh output gets mangled when using the upcoming Debian Buster. A workaround is to add in your .zshrc: setopt single_line_zle For the longer bits, it looks like zsh is now adding NUL characters in the middle of the output sent onto the console. Below an easy way to repro it 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 whether the video and PV console is correct. I would appreciate help for testing here. TODO: Actually document CONSOLEIO_write in the public header. --- xen/arch/arm/early_printk.c | 14 +++++++++----- xen/drivers/char/console.c | 37 +++++++++++++++++++------------------ xen/drivers/char/consoled.c | 4 ++-- 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 ++-- 13 files changed, 66 insertions(+), 55 deletions(-) diff --git a/xen/arch/arm/early_printk.c b/xen/arch/arm/early_printk.c index 97466a12b1..dd2e9fb46f 100644 --- a/xen/arch/arm/early_printk.c +++ b/xen/arch/arm/early_printk.c @@ -17,13 +17,17 @@ 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') { - if (*s == '\n') + unsigned int i; + + for ( i = 0; i < nr; i++ ) + { + char c = *s; + + if (c == '\n') early_putch('\r'); - early_putch(*s); - s++; + early_putch(c); } /* diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c index 4315588f05..cce1211a0c 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 ) @@ -547,8 +547,8 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer, int count) /* 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 ) @@ -666,16 +666,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 @@ -1233,6 +1233,7 @@ void debugtrace_printk(const char *fmt, ...) va_list args; char *p; unsigned long flags; + unsigned int nr; if ( debugtrace_bytes == 0 ) return; @@ -1246,12 +1247,12 @@ void debugtrace_printk(const char *fmt, ...) snprintf(buf, sizeof(buf), "%u ", ++count); va_start(args, fmt); - (void)vsnprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), fmt, args); + nr = vscnprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), fmt, args); va_end(args); if ( debugtrace_send_to_console ) { - serial_puts(sercon_handle, buf); + serial_puts(sercon_handle, buf, nr); } else { @@ -1357,7 +1358,7 @@ void panic(const char *fmt, ...) * ************************************************************** */ -static void suspend_steal_fn(const char *str) { } +static void suspend_steal_fn(const char *str, unsigned int 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 552abf5766..3e849a2557 100644 --- a/xen/drivers/char/consoled.c +++ b/xen/drivers/char/consoled.c @@ -77,7 +77,7 @@ size_t consoled_guest_rx(void) if ( idx >= BUF_SZ ) { - pv_console_puts(buf); + pv_console_puts(buf, BUF_SZ); idx = 0; } } @@ -85,7 +85,7 @@ size_t consoled_guest_rx(void) 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 */ diff --git a/xen/drivers/char/serial.c b/xen/drivers/char/serial.c index 221a14c092..7498299807 100644 --- a/xen/drivers/char/serial.c +++ b/xen/drivers/char/serial.c @@ -223,11 +223,11 @@ 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, unsigned int nr) { struct serial_port *port; unsigned long flags; - char c; + unsigned int i; if ( handle == -1 ) return; @@ -238,8 +238,10 @@ void serial_puts(int handle, const char *s) spin_lock_irqsave(&port->tx_lock, flags); - while ( (c = *s++) != '\0' ) + for ( i = 0; i < nr; i++ ) { + char c = s[i]; + 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 cc1c1d743f..5bb303d4c8 100644 --- 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, unsigned int 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, unsigned int 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 d0c8c492b0..93b6a33a41 100644 --- a/xen/drivers/video/lfb.c +++ b/xen/drivers/video/lfb.c @@ -59,14 +59,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, unsigned int nr) { unsigned int i, min_redraw_y = lfb.ypos; - char c; /* Paste characters into text buffer. */ - while ( (c = *s++) != '\0' ) + for ( i = 0; i < nr; i++ ) { + char c = s[i]; + if ( (c == '\n') || (lfb.xpos >= lfb.lfbp.text_columns) ) { if ( ++lfb.ypos >= lfb.lfbp.text_rows ) @@ -103,13 +104,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, unsigned int nr) { unsigned int i; - char c; - while ( (c = *s++) != '\0' ) + for ( i = 0; i < nr; i++ ) { + char c = s[i]; + 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 ac40a66379..15599e22ef 100644 --- 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, unsigned int nr); +void lfb_scroll_puts(const char *s, unsigned int 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 6a64fd9013..01f12aae42 100644 --- 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, unsigned int nr); +static void vga_noop_puts(const char *s, unsigned int nr) {} +void (*video_puts)(const char *, unsigned int nr) = vga_noop_puts; /* * 'vga=[,keep]' where is one of: @@ -177,12 +177,14 @@ void __init video_endboot(void) } } -static void vga_text_puts(const char *s) +static void vga_text_puts(const char *s, unsigned int nr) { - char c; + unsigned int i; - while ( (c = *s++) != '\0' ) + for ( i = 0; i < nr; i++ ) { + char c = s[i]; + if ( (c == '\n') || (xpos >= columns) ) { if ( ++ypos >= lines ) diff --git a/xen/include/xen/console.h b/xen/include/xen/console.h index b4f9463936..dafa53ba6b 100644 --- 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 *, unsigned int 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 2c3e1b3519..22f8009a5f 100644 --- 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, unsigned int nr); #else #define early_puts NULL #endif diff --git a/xen/include/xen/pv_console.h b/xen/include/xen/pv_console.h index cb92539666..41144890e4 100644 --- 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, unsigned int 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, unsigned int 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 f2994d4093..e11d6d3ebc 100644 --- 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, unsigned int 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 2e897f9df5..ddd21f374d 100644 --- 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 *, unsigned int nr); void video_endboot(void); #else #define video_init() ((void)0) -#define video_puts(s) ((void)0) +#define video_puts(s, nr) ((void)0) #define video_endboot() ((void)0) #endif