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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Juergen,

On 3/5/19 4:44 PM, Juergen Gross wrote:
> On 05/03/2019 16:28, Julien Grall wrote:
> > Hi Juergen,
> >
> > On 3/5/19 2:12 PM, Juergen Gross wrote:
> > > On 05/03/2019 15:08, Julien Grall wrote:
> > > > Hi Juergen,
> > > >
> > > > On 3/5/19 12:57 PM, Juergen Gross wrote:
> > > > > On 27/02/2019 11:45, Julien Grall wrote:
> > > > > > (+ Juergen Gross as RM)
> > > > > >
> > > > > > I forgot to CC Juergen for this.
> > > > > >
> > > > > > On 2/26/19 11:03 PM, 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>
> > > > > The risk for a regression is too high this late in the 4.12 release
> > > > > process IMO.
> > > > This code path is fairly well tested (console are used pretty much all
> > > > the times). So a regression would be quickly noticed.
> > > Only if you test all the guests out in the wild.
> > The buffer is bounded to 'nr'. So the worst things that can happen is
> > you print more characters than you wanted. CONSOLEIO_write is using a
> > semantics very similar to write() and we didn't document what happen
> > when encountering a NUL character. So, I highly doubt anyone relies on
> > the current behavior.
> >
> > Thinking a bit more, from what Ian wrote [1], the issue maybe wider than
> > zsh. So maybe we want to write a band-aid patch at least helping the
> > most common case (i.e losing all characters after the first NUL character).
> >
> > The band-aid patch should be contained to just the hypercall. Would that
> > be more suitable for you?
> My main concern is the reasoning of Daniel:
>
> "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."
>
> With keeping in mind that the behavior is the same since more than 5
> years I'd prefer to just add the text below to the release note.
I am afraid you have taken Daniel's comment out of the context. His 
comment was in discussion about what should be the guest behavior with 
NUL character. I have no plan for Xen 4.12 (or any later release) to 
modify this behavior.
In the case of Dom0, we already print all the non-printable characters 
until the first NUL character (or end of the buffer). What this patch is 
doing is to avoid using NUL character as a special case.
Anyway, I am not going to insist. I will defer this to Xen 4.13.

Cheers,

--
Julien Grall

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