[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Julien Grall <julien.grall@xxxxxxx>, Wei Liu <wei.liu2@xxxxxxxxxx>
  • From: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
  • Date: Fri, 1 Mar 2019 17:59:00 -0500
  • Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, Tim Deegan <tim@xxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Fri, 01 Mar 2019 23:04:47 +0000
  • Ironport-phdr: 9a23:zR/SjxHQXbsQfupFirGQF51GYnF86YWxBRYc798ds5kLTJ76pMS7bnLW6fgltlLVR4KTs6sC17KG9fi4EUU7or+5+EgYd5JNUxJXwe43pCcHRPC/NEvgMfTxZDY7FskRHHVs/nW8LFQHUJ2mPw6arXK99yMdFQviPgRpOOv1BpTSj8Oq3Oyu5pHfeQpFiCa+bL9oMBm6sRjau9ULj4dlNqs/0AbCrGFSe+RRy2NoJFaTkAj568yt4pNt8Dletuw4+cJYXqr0Y6o3TbpDDDQ7KG81/9HktQPCTQSU+HQRVHgdnwdSDAjE6BH6WYrxsjf/u+Fg1iSWIdH6QLYpUjm58axlVAHnhzsGNz4h8WHYlMpwjL5AoBm8oxBz2pPYbJ2JOPZ7eK7Sc8kaRW5cVchPUSJPDJ63Y48WA+YfIepUqo/wrEYMoxSjHwmhHP7hxCFGhnH23qM03eouHg7E0wM8ENwDq2jUodbvOasOTey4wqvFwDPeZP1Wwzf9743Ifwgvr/6WW7JwcNTeyU0yHA3LkFqbtI3rPymP2esXvWiQ8u1tWv+gi2E6tQ5xrSKvyd03h4nVhoMa1lDE9SJjzIYzPt23UlR3YdGjEJtOriyXMZZ9TMA6Q2xwpSo3xbILtYS7cSQX0pgr2RHSZ+Kdf4SV5B/oSfyfLi1ihH1/fbKynxOy8U+9xeLiTsS0y1NKrjZdktnLq3ANywTf6siZRft5+UeswSqP2BrJ6uFFPEA0jrDXK4Ihw7EslpoTtl7PHinql0XtkKCabEAk+ums6+j/Y7XmoIGTN5Nshw3jPakjldazDOQlPgQUQWSW9vqw2Kf+8UHhRbVFlPw2kq3XsJDAIsQbo7a0DBJa0ok+9Rm/AC2m384DkHkbLFNKZBKHj4/zN1HIO/D3F+2zg1urkDd13/zGJKHuAo3RLnjfl7fsZbR961NYyAoy099f4YhYCr4bIP3pXk/xsMfVAQUjMwyx2eroFNJ91oYGU2KVHqCZKL/SsUOP5u83LeiDeo4VtCz5K/gk+v7ik2Q0lkMcfam1x5sXaX+5Eu56LEWeZHrmms0BHnsSvgoiUOzqj0WPUDFNaHa0Rq4z+y80CJy4AofHXY2thL2B3DynHp1NfGxHBU6DEXHwd4WeXPcMajydLdN9kjAeUrihUYAh3wm0tADm07pnMvbU+ioAuJL4z9h1+/fcmgos+jxwC8Sd0meNT2Bvk2MLWTA2xqZ/rlJ5yluZ1qh4mfNYH8RJ5/xVSgc6KYLcz+tiBt/oXALOY82JR0ioQ9m8HT4xSdUxw8cQbEZnFdivlQzM3yu2A78PlryKC4Y4/b7b33j0P8x90WrJ1LE9j1k6RctCLWyoibB49wjJCI7GjV+Vmai3daQa2C7C7n+DwHGQs0FFSgJ/TaTFXWwFZkvXotX1/F/NT7irCb4/KAtO1daCKrdWat3ulVhGRe3sONLEb2KzgWi/GRWIxqiLbIrsYGgSwjjdBFIYnAAS4XaGLwk+Byi7r23CCzxuEErlY1nw/ulmtHO7Ukg0whmIb0J6ybW15xoVhf2ARPMTxb8Eozohqy5qE1qnw93WDN+ArRJ7fKpAedM9/EtH1WXBugx+OZygKbpiiUQDfAhtsULu1hF3CoZbnMgttnMl1hZ9KaaG319bazyY2pXwMKXNKmbu5BCvd7LW2lbG3daK+6cP7e81qlr9swGvDEYi9G9n09YGm0ebs7fXCAsfV9reSE8z81AupazeYyQ7oYzO3HloGaCut3nJ3Nd/QKMaxxfoW9ZCePebGQ60F8wHXeClKfAwmkjvZRUBarN87qkxau+vcfqL3OaHMa5PhjuvgywT7I9x302W/gJgW+XI2NAD2Pje0QyZAWSvxGy9u9z6zNgXLQoZGXCynG29XtZc
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 2/27/19 1:45 PM, Julien Grall wrote:
Hi Wei,

On 2/27/19 12:55 PM, Wei Liu wrote:
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.

Good point. The non-printable characters was added by Daniel in commit 48d50de8e0 " 
console: buffer and show origin of guest PV writes" without much explanation.

Yes, I added stripping of non-printable characters because escape sequences 
printed out by some guests (in particular, clear screen sequences printed out 
by some distro's early boot scripts) interfered with the output of other 
guests.  It also prevents guests from pretending to be one another or the 
hypervisor, if the console is being used for some kind of auditing or logging.

One thing I didn't consider that I probably should have is that isprint() in 
the hypervisor is not a UTF-8 aware check, so it will end up corrupting 
characters if your guests treat the console as having that encoding.

The only reason I can see is, as we buffer the guest writes, the console would be screwed 
if we split an escape sequence. Furthermore, for guest output, we will always append 
"(dX)" to the output. So I am not entirely sure what to do in the non-hwdom 
case.

Any opinions?

This really depends on the purpose of the console in the system.  Since there's 
usually possible hypervisor messages in the output, it makes sense to me to 
treat it as a line-based log containing readable text.  However, the ability 
for the hardware domain to use it interactively is also important for 
debugging, and limiting or buffering that domain's output would interfere with 
that.  The current handling of the output is a compromise between these two 
uses.

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.