From bb1ce44b15f159b67fafc5f4b285bbf20a1961e9 Mon Sep 17 00:00:00 2001 From: Basil Salman <basil@daynix.com> Date: Wed, 11 Mar 2020 19:04:15 +0200 Subject: [PATCH 1/5] qga: Installer: Wait for installation to finish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Installation might fail if we don't wait for the provider unregisteration process to finish. Signed-off-by: Sameeh Jubran <sjubran@redhat.com> Signed-off-by: Basil Salman <basil@daynix.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> --- qga/installer/qemu-ga.wxs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qga/installer/qemu-ga.wxs b/qga/installer/qemu-ga.wxs index 64bf90bd85..f6781752e6 100644 --- a/qga/installer/qemu-ga.wxs +++ b/qga/installer/qemu-ga.wxs @@ -81,7 +81,7 @@ Arguments="-d --retry-path" > </ServiceInstall> - <ServiceControl Id="StartService" Start="install" Stop="both" Remove="uninstall" Name="QEMU-GA" Wait="no" /> + <ServiceControl Id="StartService" Start="install" Stop="both" Remove="uninstall" Name="QEMU-GA" Wait="yes" /> </Component> <?ifdef var.InstallVss?> <Component Id="qga_vss_dll" Guid="{CB19C453-FABB-4BB1-ABAB-6B74F687BFBB}"> From b2413df83348acf371c03bced9a3845bba883ed5 Mon Sep 17 00:00:00 2001 From: Sameeh Jubran <sjubran@redhat.com> Date: Wed, 11 Mar 2020 19:04:16 +0200 Subject: [PATCH 2/5] qga-win: Handle VSS_E_PROVIDER_ALREADY_REGISTERED error This patch handles the case where VSS Provider is already registered, where in such case qga uninstalls the provider and registers it again. Signed-off-by: Sameeh Jubran <sjubran@redhat.com> Signed-off-by: Basil Salman <basil@daynix.com> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> --- qga/vss-win32/install.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp index 6713e58670..a456841360 100644 --- a/qga/vss-win32/install.cpp +++ b/qga/vss-win32/install.cpp @@ -443,6 +443,17 @@ STDAPI DllRegisterServer(void) VSS_PROV_SOFTWARE, const_cast<WCHAR*>(QGA_PROVIDER_VERSION), g_gProviderVersion); + if (hr == (long int) VSS_E_PROVIDER_ALREADY_REGISTERED) { + DllUnregisterServer(); + hr = pVssAdmin->RegisterProvider(g_gProviderId, CLSID_QGAVSSProvider, + const_cast<WCHAR * > + (QGA_PROVIDER_LNAME), + VSS_PROV_SOFTWARE, + const_cast<WCHAR * > + (QGA_PROVIDER_VERSION), + g_gProviderVersion); + } + if (FAILED(hr)) { errmsg_dialog(hr, "RegisterProvider failed"); } From 807e2b6fce022707418bc8f61c069d91c613b3d2 Mon Sep 17 00:00:00 2001 From: Basil Salman <basil@daynix.com> Date: Wed, 11 Mar 2020 19:04:17 +0200 Subject: [PATCH 3/5] qga-win: prevent crash when executing guest-file-read with large count guest-file-read command is currently implemented to read from a file handle count number of bytes. when executed with a very large count number qemu-ga crashes. after some digging turns out that qemu-ga crashes after trying to allocate a buffer large enough to save the data read in it, the buffer was allocated using g_malloc0 which is not fail safe, and results a crash in case of failure. g_malloc0 was replaced with g_try_malloc0() which returns NULL on failure, A check was added for that case in order to prevent qemu-ga from crashing and to send a response to the qemu-ga client accordingly. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1594054 Signed-off-by: Basil Salman <basil@daynix.com> Reported-by: Fakhri Zulkifli <mohdfakhrizulkifli@gmail.com> Cc: qemu-stable@nongnu.org Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> --- qga/commands-win32.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 9c744d6405..b49920e201 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -343,7 +343,13 @@ GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count, } fh = gfh->fh; - buf = g_malloc0(count+1); + buf = g_try_malloc0(count + 1); + if (!buf) { + error_setg(errp, + "failed to allocate sufficient memory " + "to complete the requested service"); + return NULL; + } is_ok = ReadFile(fh, buf, count, &read_count, NULL); if (!is_ok) { error_setg_win32(errp, GetLastError(), "failed to read file"); From a23f38a72921fa915536a981a4f8a9134512f120 Mon Sep 17 00:00:00 2001 From: Eric Blake <eblake@redhat.com> Date: Fri, 20 Mar 2020 10:05:07 -0500 Subject: [PATCH 4/5] qga: Fix undefined C behavior The QAPI struct GuestFileWhence has a comment about how we are exploiting equivalent values between two different integer types shared in a union. But C says behavior is undefined on assignments to overlapping storage when the two types are not the same width, and indeed, 'int64_t value' and 'enum QGASeek name' are very likely to be different in width. Utilize a temporary variable to fix things. Reported-by: Peter Maydell <peter.maydell@linaro.org> Fixes: 0b4b49387 Fixes: Coverity CID 1421990 Signed-off-by: Eric Blake <eblake@redhat.com> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> --- qga/commands.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/qga/commands.c b/qga/commands.c index f8852beb9c..4471a9f08d 100644 --- a/qga/commands.c +++ b/qga/commands.c @@ -482,10 +482,15 @@ done: * the guest's SEEK_ constants. */ int ga_parse_whence(GuestFileWhence *whence, Error **errp) { - /* Exploit the fact that we picked values to match QGA_SEEK_*. */ + /* + * Exploit the fact that we picked values to match QGA_SEEK_*; + * however, we have to use a temporary variable since the union + * members may have different size. + */ if (whence->type == QTYPE_QSTRING) { + int value = whence->u.name; whence->type = QTYPE_QNUM; - whence->u.value = whence->u.name; + whence->u.value = value; } switch (whence->u.value) { case QGA_SEEK_SET: From 7b46aadbbfb7b06cd45a3b113b1f7c003c68f603 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi <stefanha@redhat.com> Date: Mon, 23 Mar 2020 11:04:08 +0000 Subject: [PATCH 5/5] qemu-ga: document vsock-listen in the man page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Although qemu-ga has supported vsock since 2016 it was not documented on the man page. Also add the socket address representation to the qga --help output. Fixes: 586ef5dee77180fc32e33bc08051600030630239 ("qga: add vsock-listen method") Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> --- docs/interop/qemu-ga.rst | 5 +++-- qga/main.c | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/interop/qemu-ga.rst b/docs/interop/qemu-ga.rst index 1313a4ae1c..3063357bb5 100644 --- a/docs/interop/qemu-ga.rst +++ b/docs/interop/qemu-ga.rst @@ -36,13 +36,14 @@ Options .. option:: -m, --method=METHOD Transport method: one of ``unix-listen``, ``virtio-serial``, or - ``isa-serial`` (``virtio-serial`` is the default). + ``isa-serial``, or ``vsock-listen`` (``virtio-serial`` is the default). .. option:: -p, --path=PATH Device/socket path (the default for virtio-serial is ``/dev/virtio-ports/org.qemu.guest_agent.0``, - the default for isa-serial is ``/dev/ttyS0``) + the default for isa-serial is ``/dev/ttyS0``). Socket addresses for + vsock-listen are written as ``<cid>:<port>``. .. option:: -l, --logfile=PATH diff --git a/qga/main.c b/qga/main.c index 8ee2736f8e..f0e454f28d 100644 --- a/qga/main.c +++ b/qga/main.c @@ -234,7 +234,9 @@ QEMU_COPYRIGHT "\n" " -p, --path device/socket path (the default for virtio-serial is:\n" " %s,\n" " the default for isa-serial is:\n" -" %s)\n" +" %s).\n" +" Socket addresses for vsock-listen are written as\n" +" <cid>:<port>.\n" " -l, --logfile set logfile path, logs to stderr by default\n" " -f, --pidfile specify pidfile (default is %s)\n" #ifdef CONFIG_FSFREEZE