Re: [Xen-devel] [PATCH for-4.12 RFC] xen/console: Handle NUL character in buffer sent via CONSOLEIO_write

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Feb 26, 2019 at 11:03:51PM +0000, Julien Grall wrote:
> 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 <julien.grall@xxxxxxx>
> 
> ---
> 
[...]
> @@ -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);
>  

I think you missed the non-hwdom branch in the same function. It still
strips non-printable characters.

>  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';

Can this be deleted? Now that you've explicitly sized the buffer.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel