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

[Juergen Gross <jgross@xxxxxxxx>]

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.

> 
>>
>>> This patch has the advantage to allow upgrade to a newer Debian without
>>> loosing part of your prompt on zsh. I am not sure whether the problem is
>>> the same with other Distros.
>>>
>>>>
>>>> My plan is to have only one further RC before branching off 4.12,
>>>> so please let us shift this patch to 4.13.
>>>
>>> I understand. It is possible to workaround the problem at least with
>>> zsh. So a release note in Xen (and maybe Debian) should do the job.
>>
>> Could you please post the needed information for the 4.12 RN?
> 
> How about:
> 
> "
> While the hypercall CONSOLEIO_write looks analogous to the POSIX call
> write, it will only print character up to the first NUL character if any
> in the buffer. This may result to loss of characters for any application
> using directly the POSIX call write.
> 
> The issue has been confirmed some zsh version (such as in Debian
> Buster). Where the prompt is mangled, this could be avoided by adding
> 'setopt single_line_zle' in your .zshrc.
> "

Sounds okay for me.

With keeping in mind that the behavior is the same since more than 5
years I'm in favor of just adding above text to the release note.


Juergen

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