Reported by clang++, but not by g++.
../qga/vss-win32/provider.cpp:48:6: error: no previous prototype for function 'LockModule' [-Werror,-Wmissing-prototypes]
48 | void LockModule(BOOL lock)
| ^
../qga/vss-win32/provider.cpp:48:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
48 | void LockModule(BOOL lock)
| ^
| static
../qga/vss-win32/provider.cpp:531:13: error: no previous prototype for function 'DllMain' [-Werror,-Wmissing-prototypes]
531 | BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID lpReserved)
| ^
../qga/vss-win32/provider.cpp:531:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
531 | BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID lpReserved)
| ^
| static
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Link: https://lore.kernel.org/r/20241031040426.772604-7-pierrick.bouvier@linaro.org
Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Change requester_freeze so that the VSS backup type queried from the registry
Signed-off-by: Kfir Manor <kfir@daynix.com>
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Query VSS backup type number (DWORD) from QEMU Guest Agent VSS Provider registry key registry value VssOption
Translate the VSS backup type number (DWORD) into its VSS backup type (VSS_BACKUP_TYPE)
Returns the queried VSS backup type if the program encounters unexpected behaviors or values return default VSS backup type VSS_BT_FULL instead
Signed-off-by: Kfir Manor <kfir@daynix.com>
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
The custom action uses cmd.exe to run VSS Service installation
and removal which causes an interactive command shell to spawn.
This shell can be used to execute any commands as a SYSTEM user.
Even if call qemu-ga.exe directly the interactive command shell
will be spawned as qemu-ga.exe is a console application and used
by users from the console as well as a service.
As VSS Service runs from DLL which contains the installer and
uninstaller code, it can be run directly by rundll32.exe without
any interactive command shell.
Add specific entry points for rundll which is just a wrapper
for COMRegister/COMUnregister functions with proper arguments.
resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2167423
fixes: CVE-2023-0664 (part 2 of 2)
Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Reviewed-by: Yan Vugenfirer <yvugenfi@redhat.com>
Reported-by: Brian Wiltse <brian.wiltse@live.com>
volume_name_wchar is allocated by 'void* operator new [](long long unsigned int)
Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220512154909.331481-1-kkostiuk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Message-Id: <20220512154906.331399-1-kkostiuk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This fixes:
qga/vss-win32/install.cpp:49:24: error: cannot initialize a variable of
type 'char *' with an rvalue of type 'const char *'
char *msg = NULL, *nul = strchr(text, '(');
^ ~~~~~~~~~~~~~~~~~
Signed-off-by: Helge Konetzka <hk@zapateado.de>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Message-Id: <39400817-3dc9-516d-9096-bc1f68862531@zapateado.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
One less qemu-specific macro. It also helps to make some headers/units
only depend on glib, and thus moved in standalone projects eventually.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
The VssCoordinator & VssAdmin interfaces have been moved to vsadmin.h in
the Windows SDK.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20220222194008.610377-3-marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The glib_dynamic detection does not work because the dependency is
overridden in the main meson.build.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[Rewritten commit message, added requirement in qga/meson.build - Paolo]
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
There are no good reason anymore to keep a pre-built file in the repository.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
C++ presence is checked by the qga/ directory, so it can be assumed
when building VSS module.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Stop using special paths with outdated headers from an old SDK.
Instead, use standard include paths.
You can still build against the old SDK by running configure with
--extra-cxxflags="-isystem /path/to/inc/win2003/"
(this also allows to build against MinGW headers, which are currently
broken as in 9.0)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
widl from mingw64-tools and wine can compile a TLB file.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Microsoft midl compiler doesn't take "-options" form, nor does it take
OUTPUT filename. The -I option seems needless as well (at least with
VS15).
It's not clear what was actually used when it was introduced in commit
b39297aedf.
strings says "Created by MIDL version 7.00.0555 at Fri Dec 21 13:36:39
2012".
I doubt the makefile rule actually ever worked.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The current GLib version implements the DllMain function. DllMain is also
present in the provider.cpp code. So in the case of static linking, the
DllMain redefinition error occurs. For now, just switch to dynamic linking
and revert this patch when the issue will be solved.
See Glib issue for more details https://gitlab.gnome.org/GNOME/glib/-/issues/692
Signed-off-by: Kostiantyn Kostiuk <konstantin@daynix.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
On one hand "guest-fsfreeze-freeze" command, "COM+ System Application service" is
stopped, on the other hand "guest-fsfreeze-thaw" stops QGA VSS Provider service from
"COM+ Application Admin Catalog".
Invoking a series of freeze and thaw commands may result in QGA failing to stop
VSS Provider service as "COM+ System Application service" is stopped, which can
cause some delay in qga response.
In this commit StopService function was changed and VSS Provider service is now
stopped using Winsvc library API.
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1549425
Signed-off-by: Basil Salman <bsalman@redhat.com>
Signed-off-by: Basil Salman <basil@daynix.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
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>
Some of the CFLAGS that are discovered during configure, for example
compiler warnings, are being included on the linker command line because
QEMU_CFLAGS is added to it. Other flags, such as the -m32, appear twice
because they are included in both QEMU_CFLAGS and LDFLAGS. All this
leads to confusion with respect to what goes in which Makefile variables
(and we have plenty).
So, introduce QEMU_LDFLAGS for flags discovered by configure, following
the lead of QEMU_CFLAGS, and stop adding to it:
1) options that are already in CFLAGS, for example "-g"
2) duplicate options
At the same time, options that _are_ needed by both compiler and linker
must now be added to both QEMU_CFLAGS and QEMU_LDFLAGS, which is clearer.
This is mostly -fsanitize options. For now, --extra-cflags has this behavior
(but --extra-cxxflags does not).
Meson will not include CFLAGS on the linker command line, do the same in our
build system as well.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Leading underscores are ill-advised because such identifiers are
reserved. Trailing underscores are merely ugly. Strip both.
Our header guards commonly end in _H. Normalize the exceptions.
Done with scripts/clean-header-guards.pl.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20190315145123.28030-7-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[Changes to slirp/ dropped, as we're about to spin it off]
Commit 7be41675f7 set -std=gnu99 for C code via QEMU_CFLAGS. Currently
we generate a "custom" QEMU_CXXFLAGS for VSS DLL C++ build by
filtering out some options from QEMU_CFLAGS and adding some others.
Since we don't filter out -std=gnu99 currently this breaks builds when
VSS support is enabled.
We could keep the existing approach, filter out -std=gnu99 from
QEMU_CFLAGS, and add -std=gnu++98, like configure currently does for
QEMU_CXXFLAGS, but as it turns out our resulting QEMU_CXXFLAGS would
be exactly what configure already generates, just with these filtered
out:
-fstack-protector-all -fstack-protector-strong
and these added:
-Wno-unknown-pragmas -Wno-delete-non-virtual-dtor
So fix the issue by re-using configure-generated QEMU_CXXFLAGS and
just handling these specific changes.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Commit 3ebee3b191 defined assert() as g_assert(), but when we build
the VSS DLL component of QGA (to handle fsfreeze) we do not include
glib, which results in breakage when building with VSS support enabled.
Fix this by including glib (along with the -lintl and -lws2_32
dependencies it brings).
Since the VSS DLL is built statically, this introduces an additional
dependency on static glib and supporting libs for the mingw environment
(possibly why we didn't include glib originally), but VSS support
already has very specific prerequisites so it shouldn't affect too many
build environments.
Since the VSS DLL code does use qemu/osdep.h, this should also help
avoid future breakages and possibly allow for some clean ups in current
VSS code.
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
This patch add support for freeze specified fs.
The valid mountpoints list member are [1]:
The path of a mounted folder, for example, Y:\MountX\
A drive letter, for example, D:\
A volume GUID path of the form \\?\Volume{GUID}\,
where GUID identifies the volume
A UNC path that specifies a remote file share,
for example, \\Clusterx\Share1\
[1] https://docs.microsoft.com/en-us/windows/desktop/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
In one case we misconstrue a BOOL return as an HRESULT, and in the
other case we don't check the BOOL return from LookupAccountSidW()
before extracting the HRESULT from GetLastError(). Both can lead to
getNameByStringSID() misreporting an error.
Reported-by: Chen Hanxiao <chenhanxiao@gmail.com>
Suggested-by: Tomáš Golembiovský <tgolembi@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
When VM is in a heavy IO, if the command "guest-fsfreeze-freeze"
is executed, VSS may timeout when trying to hold writes.
Inside guest, Event ID 12298(VSS_ERROR_HOLD_WRITES_TIMEOUT)
is logged in the Event Viewer.
At that time, if we call AbortBackup, qga may hang forever.
This patch will solve this issue.
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: Tomoki Sekiyama <tomoki.sekiyama@gmail.com>
Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1357789
Replace hardcoded user and group names ("Administrators", "SYSTEM") with the ones acquired from system. Windows uses localized strings for these names and it may cause the installation to fail.
Windows has Well-known SIDs for "Administrators" group and "SYSTEM" user so they were used to identify required users and groups.
Well-known SIDs: https://support.microsoft.com/en-us/help/243330/well-known-security-identifiers-in-windows-operating-systems
Signed-off-by: Daniel Rempel <daniel@daynix.com>
Signed-off-by: Sameeh Jubran <sjubran@redhat.com>
Reviewed-by: Sameeh Jubran <sameeh@daynix.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
When the command "guest-fsfreeze-freeze" is executed it causes
the VSS service to log the error below in the Event Viewer. This
error is caused by an issue in the function "CommitSnapshots" in
provider.cpp:
* When VSS_TIMEOUT_MSEC expires the funtion returns E_ABORT. This causes
the error #12293.
|event id| error |
* 12293 : Volume Shadow Copy Service error: Error calling a routine on a
Shadow Copy Provider {00000000-0000-0000-0000-000000000000}.
Routine details CommitSnapshots [hr = 0x80004004, Operation
aborted.
Signed-off-by: Sameeh Jubran <sameeh@daynix.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
After triggering a freeze command without any following thaw command,
qemu-ga will not respond to stop operation. This behaviour is wanted on Linux
as there is no time limit for a freeze command and we want to prevent
quitting in the middle of freeze, on the other hand on Windows the time
limit for freeze is 10 seconds, so we should wait for the timeout, thaw
the file system and quit.
Signed-off-by: Sameeh Jubran <sameeh@daynix.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Currently the service runs in background on boot even though it is not
needed and once it is running it never stops. The service needs to be
running only during freeze operation and it should be stopped after
executing thaw.
Signed-off-by: Sameeh Jubran <sameeh@daynix.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
The quiet-command make rule currently takes two arguments:
the command and arguments to run, and a string to print if
the V flag is not set (ie we are not being verbose).
By convention, the string printed is of the form
" NAME some args". Unfortunately to get nicely lined up
output all the strings have to agree about what column the
arguments should start in, which means that if we add a
new quiet-command usage which wants a slightly longer CMD
name then we either put up with misalignment or change
every quiet-command string.
Split the quiet-mode string into two, the "NAME" and
the "same args" part, and use printf(1) to format the
string automatically. This means we only need to change
one place if we want to support a longer maximum name.
In particular, we can now print 7-character names lined
up properly (they are needed for the OSX "SETTOOL" invocation).
Change all the uses of quiet-command to the new syntax.
(Any which are missed or inadvertently reintroduced
via later merges will result in slightly misformatted
quiet output rather than disaster.)
A few places in the pc-bios/ makefiles are updated to use
"BUILD", "SIGN" and "STRIP" rather than "Building",
"Signing" and "Stripping" for consistency and to keep them
below 7 characters. Module .mo links now print "LD" rather
than the nonstandard "LD -r".
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1475598441-27908-1-git-send-email-peter.maydell@linaro.org
Header guard symbols should match their file name to make guard
collisions less likely. Offenders found with
scripts/clean-header-guards.pl -vn.
Cleaned up with scripts/clean-header-guards.pl, followed by some
renaming of new guard symbols picked by the script to better ones.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Tracked down with an ugly, brittle and probably buggy Perl script.
Also move includes converted to <...> up so they get included before
ours where that's obviously okay.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Tested-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Manually drop redundant includes that scripts/clean-includes misses,
e.g. because they're hidden in generator programs, or they use the
wrong kind of delimiter.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
requester.h relied on qemu/compiler.h definitions to
handle GCC_FMT_ATTR() stub, but this include was removed as part
of scripted clean-ups via 30456d5:
all: Clean up includes
under the assumption that all C files would have included it via
qemu/osdep.h at that point. requester.cpp was likely missed
due to C++ files requiring manual/special handling as well as
VSS build options needing to be enabled to trigger build failures.
Fix this by including qemu/osdep.h. That in turn pulls in a
macro from qapi/error.h that conflicts with a struct field name
in requester.h, so fix that as well by renaming the field.
While we're at it, fix up provider.cpp/install.cpp to include
osdep.h as well.
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.
This commit was created with scripts/clean-includes.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
This is particularly useful when we abort in error_propagate(),
because there the stack backtrace doesn't lead to where the error was
created. Looks like this:
Unexpected error in parse_block_error_action() at .../qemu/blockdev.c:322:
qemu-system-x86_64: -drive if=none,werror=foo: 'foo' invalid write error action
Aborted (core dumped)
Note: to get this example output, I monkey-patched drive_new() to pass
&error_abort to blockdev_init().
To keep the error handling boiler plate from growing even more, all
error_setFOO() become macros expanding into error_setFOO_internal()
with additional __FILE__, __LINE__, __func__ arguments. Not exactly
pretty, but it works.
The macro trickery breaks down when you take the address of an
error_setFOO(). Fortunately, we do that in just one place: qemu-ga's
Windows VSS provider and requester DLL wants to call
error_setg_win32() through a function pointer "to avoid linking glib
to the DLL". Use error_setg_win32_internal() there. The use of the
function pointer is already wrapped in a macro, so the churn isn't
bad.
Code size increases by some 35KiB for me (0.7%). Tolerable. Could be
less if we passed relative rather than absolute source file names to
the compiler, or forwent reporting __func__.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
requester.cpp uses this pattern to receive an error and pass it on to
the caller (err_is_set() macro peeled off for clarity):
... code that may set errset->errp ...
if (errset->errp && *errset->errp) {
... handle error ...
}
This breaks when errset->errp is null. As far as I can tell, it
currently isn't, so this is merely fragile, not actually broken.
The robust way to do this is to receive the error in a local variable,
then propagate it up, like this:
Error *err = NULL;
... code that may set err ...
if (err)
... handle error ...
error_propagate(errset->errp, err);
}
See also commit 5e54769, 0f230bf, a903f40.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
qga_vss_fsfreeze() casts error_set_win32() from
void (*)(Error **, int, ErrorClass, const char *, ...)
to
void (*)(void **, int, int, const char *, ...)
The result is later called. Since the two types are not compatible,
the call is undefined behavior. It works in practice anyway.
However, there's no real need for trickery here. Clean it up as
follows:
* Declare struct Error, and fix the first parameter.
* Switch to error_setg_win32(). This gets rid of the troublesome
ErrorClass parameter. Requires converting error_setg_win32() from
macro to function, but that's trivially easy, because this is the
only user of error_set_win32().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
