Automake has builtin support to prevent botched conditional nesting,
but only if you use:
if FOO
else !FOO
endif !FOO
An example error message when using the wrong name:
daemon/Makefile.am:378: error: else reminder (LIBVIRT_INIT_SCRIPT_SYSTEMD_TRUE) incompatible with current conditional: LIBVIRT_INIT_SCRIPT_SYSTEMD_FALSE
daemon/Makefile.am:381: error: endif reminder (LIBVIRT_INIT_SCRIPT_SYSTEMD_TRUE) incompatible with current conditional: LIBVIRT_INIT_SCRIPT_SYSTEMD_FALSE
As our makefiles tend to have quite a bit of nested conditionals,
it's better to take advantage of the benefits of the build system
double-checking that our conditionals are well-nested, but that
requires a syntax check to enforce our usage style.
Alas, unlike C preprocessor and spec files, we can't use indentation
to make it easier to see how deeply nesting goes.
* cfg.mk (sc_makefile_conditionals): New rule.
* daemon/Makefile.am: Enforce the style.
* gnulib/tests/Makefile.am: Likewise.
* python/Makefile.am: Likewise.
* src/Makefile.am: Likewise.
* tests/Makefile.am: Likewise.
* tools/Makefile.am: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
The return values for the virConnectListAllSecrets call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virConnectListAllNWFilters call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virConnectListAllNodeDevices call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virConnectListAllInterfaces call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virConnectListAllNetworks call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virStoragePoolListAllVolumes call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virConnectListAllStoragePools call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virConnectListAllDomains call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virDomain{SnapshotListAllChildren,ListAllSnapshots}
calls were not bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The return values for the virDomainGetJobStats call were not
bounds checked. This is a robustness issue for clients if
something where to cause corruption of the RPC stream data.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The parameters for the virDomainMigrate*Params RPC calls were
not bounds checks, meaning a malicious client can cause libvirtd
to consume arbitrary memory
This issue was introduced in the 1.1.0 release of libvirt
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virtlockd/libvirtd daemons had listed '?' as the short option
for --help. getopt_long uses '?' for any unknown option. We want
to be able to distinguish unknown options (which use EXIT_FAILURE)
from correct usage of help (which should use EXIT_SUCCESS). Thus
we should use 'h' as a short option for --help. Also add this to
the man page docs
The virtlockd/libvirtd daemons did not list any short option
for the --version arg. Add -V as a valid short option, since
-v is already used for --verbose.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This configuration knob lets user to set the length of queue of
connection requests waiting to be accept()-ed by the daemon. IOW, it
just controls the @backlog passed to listen:
int listen(int sockfd, int backlog);
Makefiles are another easy file to enforce line limits.
Mostly straightforward; interesting tricks worth noting:
src/Makefile.am: $(confdir) was already defined, use it in more places
tests/Makefile.am: path_add and VG required some interesting compression
* cfg.mk (sc_prohibit_long_lines): Add another test.
* Makefile.am: Fix offenders.
* daemon/Makefile.am: Likewise.
* docs/Makefile.am: Likewise.
* python/Makefile.am: Likewise.
* src/Makefile.am: Likewise.
* tests/Makefile.am: Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Since they make use of file descriptor passing, the remote protocol
methods for virDomainCreate{XML}WithFiles must be written by hand.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Convert the type of loop iterators named 'i', 'j', k',
'ii', 'jj', 'kk', to be 'size_t' instead of 'int' or
'unsigned int', also santizing 'ii', 'jj', 'kk' to use
the normal 'i', 'j', 'k' naming
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Commit id 'ed3bac71' introduced the following:
TEST: libvirtdconftest
........................................ 40 OK
==25875== 690 (480 direct, 210 indirect) bytes in 30 blocks are definitely lost in loss record 18 of 24
==25875== at 0x4A06B6F: calloc (vg_replace_malloc.c:593)
==25875== by 0x4C737DF: virAllocN (viralloc.c:152)
==25875== by 0x403BC8: remoteConfigGetStringList (libvirtd-config.c:74)
==25875== by 0x4042CF: daemonConfigLoadOptions (libvirtd-config.c:382)
==25875== by 0x4052F5: daemonConfigLoadData (libvirtd-config.c:479)
==25875== by 0x40222C: testCorrupt (libvirtdconftest.c:112)
==25875== by 0x40321F: virtTestRun (testutils.c:158)
==25875== by 0x401FEE: mymain (libvirtdconftest.c:228)
==25875== by 0x40385A: virtTestMain (testutils.c:722)
==25875== by 0x37C1021A04: (below main) (libc-start.c:225)
==25875==
PASS: libvirtdconftest
Add an access control driver that uses the pkcheck command
to check authorization requests. This is fairly inefficient,
particularly for cases where an API returns a list of objects
and needs to check permission for each object.
It would be desirable to use the polkit API but this links
to glib with abort-on-OOM behaviour, so can't be used. The
other alternative is to speak to dbus directly
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Add a new 'access_drivers' config parameter to the libvirtd.conf
configuration file. This allows admins to setup the default
access control drivers to use for API authorization. The same
driver is to be used by all internal drivers & APIs
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This is a recurring problem for cygwin :)
For example, see commit 23a4df88.
qemu/qemu_driver.c: In function 'qemuStateInitialize':
qemu/qemu_driver.c:691:13: error: format '%d' expects type 'int', but argument 8 has type 'uid_t' [-Wformat]
* src/qemu/qemu_driver.c (qemuStateInitialize): Add casts.
* daemon/remote.c (remoteDispatchAuthList): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
Change the build process & driver initialization so that the
VirtualBox driver is built into libvirtd, instead of libvirt.so
This change avoids the VirtualBox GPLv2-only license causing
compatibility problems with libvirt.so which is under the
GPLv2-or-later license.
NB this change prevents use of the VirtualBox driver on the
Windows platform, until such time as libvirtd can be made
to work there.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
CVE-2013-1962
remoteDispatchStoragePoolListAllVolumes wasn't freeing the pool.
The pool also held a reference to the connection, preventing it from
getting freed and closing the netcf interface driver, which held two
sockets open.
Automake already passes all CFLAGS to the linker too, so it
is not necessary to set WARN_LDFLAGS in addition to the
WARN_CFLAGS variable.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently the virGetHostname() API has a bogus virConnectPtr
parameter. This is because virtualization drivers directly
reference this API in their virDriverPtr tables, tieing its
API design to the public virConnectGetHostname API design.
This also causes problems for access control checks since
these must only be done for invocations from the public
API, not internal invocation.
Remove the bogus virConnectPtr parameter, and make each
hypervisor driver provide a dedicated function for the
driver API impl. This will allow access control checks
to be easily inserted later.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Since PIDs can be reused, polkit prefers to be given
a (PID,start time) pair. If given a PID on its own,
it will attempt to lookup the start time in /proc/pid/stat,
though this is subject to races.
It is safer if the client app resolves the PID start
time itself, because as long as the app has the client
socket open, the client PID won't be reused.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Since the 'nparams' variable passed to virTypedParametersFree is
supposed to represent the size of the 'params' array, it is bad
practice to initialize it to a non-zero value, until the array
has been allocated.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The source code base needs to be adapted as well. Some files
include virutil.h just for the string related functions (here,
the include is substituted to match the new file), some include
virutil.h without any need (here, the include is removed), and
some require both.
Ensure that the driver struct field names match the public
API names. For an API virXXXX we must have a driver struct
field xXXXX. ie strip the leading 'vir' and lowercase any
leading uppercase letters.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
A number of the remote procedure names did not match the
corresponding API names. For example, many lacked the
word 'CONNECT', others re-arranged the names. Update the
procedures so their names exactly match the API names.
Then remove the special case handling of these APIs in
the generator
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
There are many declared options in gendispatch.pl that were
no longer used. Those which were used were obscure '-b', '-k'
and '-d'. Switch to use --mode={debug|client|server}.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
http://www.uhv.edu/ac/newsletters/writing/grammartip2009.07.01.htm
(and several other sites) give hints that 'onto' is best used if
you can also add 'up' just before it and still make sense. In many
cases in the code base, we really want the two-word form, or even
a simplification to just 'on' or 'to'.
* docs/hacking.html.in: Use correct 'on to'.
* python/libvirt-override.c: Likewise.
* src/lxc/lxc_controller.c: Likewise.
* src/util/virpci.c: Likewise.
* daemon/THREADS.txt: Use simpler 'on'.
* docs/formatdomain.html.in: Better usage.
* docs/internals/rpc.html.in: Likewise.
* src/conf/domain_event.c: Likewise.
* src/rpc/virnetclient.c: Likewise.
* tests/qemumonitortestutils.c: Likewise.
* HACKING: Regenerate.
Signed-off-by: Eric Blake <eblake@redhat.com>
Directories python/tools/examples should include them in <> form,
though this patch allows "" form in these directories by excluding
them, a later patch will do the cleanup.
Detected by a simple Shell script:
for i in $(git ls-files -- '*.[ch]'); do
awk 'BEGIN {
fail=0
}
/# *include.*\.h/{
match($0, /["<][^">]*[">]/)
arr[substr($0, RSTART+1, RLENGTH-2)]++
}
END {
for (key in arr) {
if (arr[key] > 1) {
fail=1
printf("%d %s\n", arr[key], key)
}
}
if (fail == 1)
exit 1
}' $i
if test $? != 0; then
echo "Duplicate header(s) in $i"
fi
done;
A later patch will add the syntax-check to avoid duplicate
headers.
