[Qemu-devel] Re: [PATCH] Fix changing password using monitor over VNC.

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 1554 bytes --]

Zachary Amsden wrote:
> A simple segfault turned out to be a relatively complex fix.
> 
> The monitor calls back to main_loop_wait() to wait for the completion
> of the password change event; this results in a nested invocation of

This is no longer true with trunk as this nasty blocking password
reading has been converted into an async operation. Is your patch
required nevertheless? Or is this band-aid for stable?

> the associated I/O handlers.  For stdio monitor, this is okay, but VNC
> maintains an input buffer which is not flushed until after the
> invocation of protocol actions.  This is non-reentrant; the result is
> that the nested invocation consumes the same protocol event as the
> parent (which was a '\n', setting a NULL password), and it gets worse
> when both the child and the parent attempt to shift in the same input
> event, resulting in a memmove of size -1ULL, and a segfault.
> 
> The fix is to consume the input buffer before invoking protocol actions
> which may cause nested invocation of the handler; we must also set up
> the child handler to receive new events, which was cleanest done with
> vnc_read_when() from the protcol handler (doing it in the outer loop
> causes bugs with other types of waits, such as auth).  We return fed=1
> from the outer handler to prevent the logic in vnc_client_read from
> reconsuming the pre-consumed buffer, and simply reset the expect
> value to receive the next protocol command.
> 
> Signed-off-by: Zachary Amsden <zamsden@redhat.com>
> ---

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]