Simon McVittie [Fri, 24 Mar 2017 19:41:10 +0000 (19:41 +0000)]
Stop opting out of -Wswitch-enum and -Wswitch-default
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
Simon McVittie [Tue, 17 Jan 2017 20:50:08 +0000 (20:50 +0000)]
DBusTransport: be explicit about _dbus_auth_do_work() results
Explicitly enumerate the states where we reset the maybe_authenticated
flag, and assert that _dbus_auth_do_work() doesn't return an impossible
or invalid state (by source code inspection, it doesn't).
This silences -Wswitch-enum. Based on part of a patch from
Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
Philip Withnall [Wed, 22 Feb 2017 10:56:56 +0000 (10:56 +0000)]
dbus: Fix writing off the end of an fd_set when testing with Valgrind
If the test-bus test is run under Valgrind, its code to detect FD leaks
accidentally writes off the end of the fd_set it uses, as Valgrind opens
some high FDs (≥1024) for internal use.
Ignore those FDs. Realistically, they are never going to be leaks — in
order to have a false negative from omitting this check, D-Bus would
have to allocate and not leak all the FDs up to FD_SETSIZE, and then
leak the first FD over that which it allocated. D-Bus never allocates
anywhere near that number of FDs concurrently.
Signed-off-by: Philip Withnall <withnall@endlessm.com> Reviewed-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99839
Simon McVittie [Tue, 17 Jan 2017 21:09:07 +0000 (21:09 +0000)]
bus policy: assert that no invalid rule types are seen
This silences -Wswitch-default.
Based on part of a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
Simon McVittie [Tue, 17 Jan 2017 21:07:00 +0000 (21:07 +0000)]
config-parser tests: explicitly skip non-comparable elements
For these types, the tagged union in the Element struct does not store
anything we could usefuly compare.
Based on part of a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
Simon McVittie [Tue, 17 Jan 2017 20:59:57 +0000 (20:59 +0000)]
config-parser: assert elements are of a known type
This silences -Wswitch-default.
Based on part of a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
Simon McVittie [Tue, 17 Jan 2017 20:58:06 +0000 (20:58 +0000)]
config-parser: treat impossible policy type as IGNORED
This silences -Wswitch-default.
Based on part of a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
Simon McVittie [Tue, 17 Jan 2017 20:54:20 +0000 (20:54 +0000)]
sysdeps: assert that log severity is one we expect
This silences -Wswitch-default.
Based on part of a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
Simon McVittie [Tue, 17 Jan 2017 20:52:25 +0000 (20:52 +0000)]
_dbus_global_lock: move success case up into switch
This silences -Wswitch-default.
Based on part of a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
Simon McVittie [Tue, 17 Jan 2017 20:49:31 +0000 (20:49 +0000)]
DBusTransport: assert that invalid results don't happen
This silences -Wswitch-default.
Based on part of a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
Simon McVittie [Tue, 17 Jan 2017 20:47:16 +0000 (20:47 +0000)]
dbus-launch: clarify signal handler
We only register signal_handler() for the three signals that we want
to handle as "kill dbus-daemon and exit", so there's no point in the
switch. Silence -Wswitch-default by removing it altogether.
The variable name got_fatal_signal and the verbose message are both
misleading, because actually this is a handler for multiple signals,
not just SIGHUP. Rename them to be generic.
Based on part of a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
Simon McVittie [Tue, 17 Jan 2017 20:42:31 +0000 (20:42 +0000)]
dbus-monitor: handle default case for binary mode header
Also comment why it's OK to not do anything for the modes that don't
have a header. We are effectively treating the default case as one
of those, on the assumption that future modes are more likely to
lack a header than to have one.
Based on part of a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
Simon McVittie [Tue, 17 Jan 2017 20:41:17 +0000 (20:41 +0000)]
test, tools: assert impossible values of local enums are not reached
Based on part of a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
Simon McVittie [Tue, 17 Jan 2017 20:28:15 +0000 (20:28 +0000)]
dbus-spawn: assert impossible returns from read functions don't happen
This silences -Wswitch-default.
Based on a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
Simon McVittie [Tue, 17 Jan 2017 20:21:21 +0000 (20:21 +0000)]
dbus-daemon: silence -Wswitch-default
There should be no way signal_handler() can be called for a signal
we didn't ask for. If it somehow happens, ignore it.
Based on a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
Simon McVittie [Tue, 17 Jan 2017 20:18:47 +0000 (20:18 +0000)]
bus dispatch tests: treat impossible message_kind as GOT_SOMETHING_ELSE
check_got_service_info() can't actually return an invalid
GotServiceInfo, but if it somehow does, we want to fail the test.
GOT_SOMETHING_ELSE already has that effect, and a similar meaning.
Based on a patch from Thomas Zimmermann.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=98191
Philip Withnall [Wed, 5 Apr 2017 10:36:12 +0000 (11:36 +0100)]
test: Fix a couple of memory leaks in test-corrupt
Spotted while testing bug #100568.
Signed-off-by: Philip Withnall <withnall@endlessm.com> Reviewed-by: Simon McVittie <smcv@collabora.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=100568
Philip Withnall [Wed, 5 Apr 2017 10:35:27 +0000 (11:35 +0100)]
test: Fix reading off the end of an array in test-corrupt
One level of pointer indirection too many when passing the arguments to
dbus_message_append_args().
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=100568 Signed-off-by: Philip Withnall <withnall@endlessm.com> Reviewed-by: Simon McVittie <smcv@collabora.com>
Initialize SELinux and Apparmor after capabilities are set
avc_init() in the SELinux code path is creating a new thread, we need to
set to capabilities before it gets created so it has the permission to
send audit messages.
It also make more sense to open the audit netlink before the different
logging callbacks are set.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=92832
[smcv: add comments explaining why initialization must happen in this
specific order] Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=857660 Reviewed-by: Stephen Smalley <sds@tycho.nsa.gov>
(cherry picked from commit a3a5935a0a038c3b44c61ce5719f0f7e647b96c6)
Philip Withnall [Tue, 28 Feb 2017 08:56:25 +0000 (08:56 +0000)]
test: Remove an unnecessary NULL check in sd-activation
test_transient_services() was checking whether (config == NULL), then
shortly afterwards dereferencing config->bus_name (which is in fact
dereferenced on all branches of the function). Remove the check and add
some assertions to ensure a Config struct is always passed into the test
(which it currently is).
Coverity ID: 141877
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99999 Reviewed-by: Simon McVittie <smcv@collabora.com>
Philip Withnall [Tue, 28 Feb 2017 08:55:26 +0000 (08:55 +0000)]
test: Fix copy-paste error in waiting for a message in the tests
The parameters to the test_pending_call_store_reply() call should match
those to the dbus_pending_call_set_notify() immediately after.
Coverity ID: 141876
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99999 Reviewed-by: Simon McVittie <smcv@collabora.com>
Ralf Habacker [Fri, 3 Mar 2017 11:39:23 +0000 (11:39 +0000)]
cmake, autotools: Add find package config support for cmake clients
With this support cmake and autotools generates cmake equivalent of
pkgconfig files on configure time named DBus1Config*.cmake. These
files are installed into the related directory where cmake expects
find_package related config files.
For instructions how to use this feature with clients see readme.cmake.
With previous DBus versions each cmake client using DBus as dependency
needed a related FindDBus*.cmake in its source distribution or in
the cmake binary packages. With the 'config' find package style support
provided by this patch this requirement has been removed.
The generated config file uses pkgconfig on unix or autotools to
fetch package build flags, which is the prefered way. On Windows
we do not want to require CMake users to have pkg-config installed
so it uses cmake buildin target export support for exporting all
targets into DBus1ConfigTargets*.cmake.
[smcv: make sure variable substitution works in Autotools too]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99721 Reviewed-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de> Signed-off-by: Simon McVittie <smcv@collabora.com>
Relocatable pkgconfig files are necessary when using packages installed to
a location that does not match the location for which they were compiled.
However, using ${pcfiledir} is problematic for system installations
in standard locations, because it interferes with pkg-config's
ability to filter out -I, -L options that are redundant with compiler
defaults (which is important if you are trying to use a newer version
of a library than the system copy).
In practice operating system vendors installing dbus to standard
locations use Autotools, so we enable relocatable builds by default
when building with CMake.
For simplicity, we're also not relocatable if the library directory
is something more complicated than lib or lib64 (e.g. under Debian
multiarch); we don't want to have to compute how many ../ to add.
This is non-trivial to determine in an Autotools build, so for now
there is no support for relocation when built with Autotools,
even as an opt-in feature.
Going via the ${original_prefix} variable is because under Autotools,
both ${prefix} and ${exec_prefix} technically default to NONE, with
NONE replaced with their real defaults of /usr/local and '${prefix}'
(respectively) later on. If we tried to expand ${prefix} at the time
that we choose the value of ${pkgconfig_prefix}, that would cause
a broken value "prefix=NONE" to be hard-coded.
[smcv: no relocation on Autotools, make it optional in CMake,
expand commit message]
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99721 Reviewed-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Ralf Habacker <ralf.habacker@freenet.de> Signed-off-by: Simon McVittie <smcv@collabora.com>
Philip Withnall [Wed, 22 Feb 2017 13:22:37 +0000 (13:22 +0000)]
doc: Clarify /etc/dbus-1/system.d and /usr/share/dbus-1/system.d
The documentation generally only mentioned the directory in /etc, even
though we actually prefer security policies to be installed in
/usr/share to allow for stateless and volatile systems (i.e. booting up
with an empty /etc).
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99901 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Philip Withnall [Thu, 22 Jan 2015 20:11:07 +0000 (20:11 +0000)]
dbus-socket-set-epoll: initialize all bytes of struct epoll_event
This should be a no-op, but it shuts Valgrind up.
The reason for the warning is that we fill in event.events and
event.data.fd, but the union event.data actually contains more bytes
than that. We'll get the same partially initialized union back from the
kernel in socket_set_epoll_poll(), where we take events[i].data.fd and
ignore the rest. So the current code is safe, but valgrind is right to
worry.
This is an expanded version of an older patch by Simon McVittie.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=88808 Signed-off-by: Simon McVittie <smcv@debian.org>
Simon McVittie [Tue, 21 Feb 2017 14:57:37 +0000 (14:57 +0000)]
Add a simple integration test for transient services
Because this is in a subdirectory, it requires some extra `mkdir -p`
and some `nobase_` variables. Make all the installed-tests `nobase_`
for consistency.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Andre Moreira Magalhaes <andre.magalhaes@collabora.co.uk> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Fri, 17 Feb 2017 20:54:44 +0000 (20:54 +0000)]
dbus-daemon(1): Describe how session and system services are found
For Unix, this partially duplicates the D-Bus Specification, but
provides more detail about the intention of each search path element.
It also documents the non-standardized path elements searched by the
reference implementation.
For Windows, there are no standardized path elements in the D-Bus
Specification (and it isn't clear how useful it would be to standardize
them, since Windows software that uses D-Bus tends to be installed
as an integrated "stack" with a bundled copy of a suitable dbus-daemon),
so we just document what the reference implementation does.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com>
[smcv: fix formatting nitpicks] Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Fri, 17 Feb 2017 20:54:14 +0000 (20:54 +0000)]
spec: Don't say implementation-specific locations must be lowest priority
We're treating transient services as higher-priority than those in
the XDG_DATA_HOME or XDG_DATA_DIRS, which is consistent with systemd.
The specific list used by the standard session dbus-daemon will be
added to dbus-daemon(1) in the next commit.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Tue, 14 Feb 2017 15:10:20 +0000 (15:10 +0000)]
sd-activation test: Exercise transient services
To do this, we have to use the <standard_session_servicedirs/>.
A previous commit ensured that those don't provide any service files
we don't expect.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Mon, 13 Feb 2017 20:58:51 +0000 (20:58 +0000)]
config-parser: Add transient service directories
For configuration purposes these are treated as part of the standard
session service directories, to avoid having to add new configuration
syntax which would prevent an old dbus-daemon from reloading
successfully. From an API perspective, they're separate, though.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Mon, 13 Feb 2017 19:57:03 +0000 (19:57 +0000)]
sysdeps: Add accessor for a list of transient service directories
These directories can be used by service managers like `systemd --user`
and its generators, or by session infrastructure like gnome-session,
to synthesize D-Bus service files at runtime from some more canonical
source of information.
The intention is that this is in the XDG_RUNTIME_DIR as defined by the
freedesktop.org Base Directory Specification, which is private to the
user, and has a lifetime equal to the union of all the user's concurrent
login sessions.
This directory is provided on Linux systems that have systemd-logind and
pam_systemd, on other systems with PAM that have pam-xdg-support (which
has been abandoned by Ubuntu in favour of logind, but could be forked
by non-systemd environments that are interested in this functionality),
or any compatible reimplementation.
In practice this is most likely to be useful on systems that run
`dbus-daemon --session` from `systemd --user`.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Fri, 17 Feb 2017 15:40:34 +0000 (15:40 +0000)]
activation: Add support for enforcing strict naming on .service files
This is done on a per-directory basis.
The use of the BusContext here means we have to make the activation
test a little more realistic, by providing a non-NULL BusContext.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Fri, 17 Feb 2017 15:03:02 +0000 (15:03 +0000)]
config-parser: Store service directories in structs
This lets us give them a flags word, which we immediately use to
track whether this directory should be watched with inotify or
equivalent.
The struct name is unfortunately a bit odd, because I had aimed to
use BusServiceDir, but activation.c already has BusServiceDirectory
so that would have been too confusing.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
I'm about to change the version in the full-fat parser to return
BusServiceDir structs. Name this one with "paths" instead, to avoid
confusion.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Wed, 15 Feb 2017 14:13:05 +0000 (14:13 +0000)]
bus_config_parser_get_watched_dirs: Turn into a helper function
This means we can test it more easily. At the moment it just
contains service directories, because this config file is so
cut-down that it doesn't have any config.d directories.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Tue, 14 Feb 2017 19:03:44 +0000 (19:03 +0000)]
sd-activation test: Create and destroy a temporary XDG_RUNTIME_DIR
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Tue, 14 Feb 2017 19:40:28 +0000 (19:40 +0000)]
test-utils-glib: Wait for the killed process to exit
Otherwise, removing transient service directories that are being
watched by the dbus-daemon can fail with EAGAIN.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Mon, 20 Feb 2017 17:44:09 +0000 (17:44 +0000)]
tests: Consistently don't try to kill pid 0
daemon_pid will still be 0 if any tests were skipped. In particular
this happens during `make installcheck`.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Fri, 17 Feb 2017 18:12:54 +0000 (18:12 +0000)]
config-parser test: Exercise the full config-parser
Don't just exercise _dbus_get_standard_session_servicedirs(), but
also its integration into the BusConfigParser.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Fri, 17 Feb 2017 17:15:34 +0000 (17:15 +0000)]
config-parser: Don't use dbus_setenv() to test service directories
We can rely on the Autotools build system to pass in some safe values
for XDG_DATA_HOME and XDG_DATA_DIRS that match DBUS_TEST_BUILDDIR.
This test will now be skipped when running test-bus manually,
or under the CMake build system. Under CMake it could be reinstated
by setting the right environment variables.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com>
[smcv: add missing newline as requested]
[smcv: align DBUS_TEST_BUILDDIR with G_TEST_BUILDDIR] Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Fri, 17 Feb 2017 17:10:19 +0000 (17:10 +0000)]
config-parser: Simplify test for standard session service dirs
There's little point in asserting that the defaults (without
setting XDG_DATA_HOME, etc.) end with share/dbus-1/services,
because we are about to re-test with known values for XDG_DATA_HOME
etc., at which point we can check exact values which is more strict.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Fri, 17 Feb 2017 17:08:20 +0000 (17:08 +0000)]
config-parser: Remove dead code from system service dirs test
progs was never used, because it was originally only used on
Windows, where this test makes no sense and so is no longer run.
It is unnecessary to check that the system service directories end
with dbus-1/system-services, because we are going to check their
exact values a short time later anyway.
It is also unnecessary to set XDG_DATA_HOME and XDG_DATA_DIRS,
because those variables are no longer respected for system service
directories, only for session service directories.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
We want to be able to use <standard_system_datadirs/> in tests
without picking up someone else's .service files.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Tue, 14 Feb 2017 19:49:46 +0000 (19:49 +0000)]
tests: Wrap file-deletion functions to handle EINTR
The GLib functions we're using don't, and it seems to be possible to be
interrupted during cleanup for our tests.
Windows apparently has and uses ENOENT for _unlink(), so just do the
same on Windows there; but EINTR is very much a POSIX thing, so ignore
that on Windows.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com>
[smcv: add Windows fixes from a later commit, also reviewed by Philip] Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Mon, 13 Feb 2017 17:31:41 +0000 (17:31 +0000)]
test_get_dbus_daemon: Take a custom XDG_RUNTIME_DIR as an argument
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Fri, 17 Feb 2017 12:38:27 +0000 (12:38 +0000)]
activation test: Use more realistic bus names for services
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Fri, 17 Feb 2017 12:21:46 +0000 (12:21 +0000)]
activation: Put activation directories in an ordered list
There are two circumstances in which we load .service files. The first
is bus_activation_reload(), which is given an ordered list of directory
paths, and reads each one in its correct order, highest-precedence
first (normally ~/.local/share > /usr/local/share > /usr/share). This
seems correct.
However, if we are asked to activate a service for which we do not know
of a .service file, we opportunistically reload the search path and
try again, in the hope that it was recently-installed and not yet
discovered by inotify. Prior to this commit, this would iterate through
the hash table in arbitrary hash order, so we might load a service
from /usr/share even though it was meant to be masked by a
higher-priority service file in ~/.local/share or /usr/local/share.
Before I add more elements to the search path, we should make sure
it is always searched in the expected order.
We do not actually make use of the hash table's faster-than-O(n)
lookup by directory path anywhere, so there is no point in using a
hash table, and we can safely replace it with an ordered data structure.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Tue, 14 Feb 2017 12:31:24 +0000 (12:31 +0000)]
sd-activation test: Use a struct for the test context
This is going to be necessary to be able to influence setup() and
teardown() as well as just the individual tests.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com> Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Mon, 13 Feb 2017 20:08:07 +0000 (20:08 +0000)]
config-parser: Fix indentation
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com>
[smcv: also correct the same thing for system service directories] Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Mon, 13 Feb 2017 19:31:39 +0000 (19:31 +0000)]
config-parser: Eliminate duplicate functionality
We had two ways to append a path to the list of service directories.
Collapse them into one.
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99825 Reviewed-by: Philip Withnall <withnall@endlessm.com>
Simon McVittie [Wed, 15 Feb 2017 16:57:00 +0000 (16:57 +0000)]
shell-test: Don't use _dbus_get_tmpdir()
There's no particular reason to be using a temporary directory (it's
just some arbitrary string), and it will be harder for future changes
to eradicate uses of a temporary directory that is shared between users
if we list it here.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99828 Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Simon McVittie [Wed, 15 Feb 2017 16:55:53 +0000 (16:55 +0000)]
test: Delete directories like directories, not files
Directories can't usefully appear in CLEANFILES, we have to delete
them recursively in clean-local.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99828 Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Philip Withnall [Mon, 13 Feb 2017 12:55:40 +0000 (12:55 +0000)]
dbus-hash: Fix memory leaks in internal hash table tests
This includes fixing a memory leak in _dbus_hash_iter_lookup(), which is
not one of the unit tests; but it is only ever called from the unit
tests, so this is not a user-facing leak.
Coverity IDs: 54730, 54740
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99793 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Wed, 15 Feb 2017 17:24:14 +0000 (17:24 +0000)]
activation test: Fix time-of-check/time-of-use bug waiting to happen
Creating a directory is atomic, stat'ing it to see whether to remove
it is very much not.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99828 Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Simon McVittie [Wed, 15 Feb 2017 16:32:04 +0000 (16:32 +0000)]
Change _dbus_create_directory to fail for existing directories
If we don't trap EEXIST and its Windows equivalent, we are unable to
detect the situation where we create an ostensibly unique
subdirectory in a shared /tmp, but an attacker has already created it.
This affects dbus-nonce (the nonce-tcp transport) and the activation
reload test.
Add a new _dbus_ensure_directory() for the one case where we want it to
succeed even on EEXIST: the DBUS_COOKIE_SHA1 keyring, which we know
we are creating in our own trusted "official" $HOME. In the new
transient service support on Bug #99825, ensure_owned_directory()
would need the same treatment.
We are not treating this as a serious security problem, because the
nonce-tcp transport is rarely enabled on Unix and there are multiple
mitigations.
The nonce-tcp transport creates a new unique file with O_EXCL and 0600
(private to user) permissions, then overwrites the requested filename
via atomic-overwrite, so the worst that could happen there is that an
attacker could place a symbolic link matching the name of a directory
we are going to create, causing a dbus-daemon configured for nonce-tcp
to traverse the symlink and atomically overwrite a file named "nonce"
in a directory of the attacker's choice, with new random contents that
are not known to the attacker. This seems unlikely to be exploitable
for anything worse than denial of service in practice. In mainline
Linux since 3.6, this attack is also defeated by the
fs.protected_symlinks sysctl, which many distributions enable by default.
The activation reload test suffers from a classic symlink attack
due to time-of-check/time-of-use errors in its implementation, but as
part of the developer-only "embedded tests" that are only intended
to be run on a trusted machine, it is not treated as security-sensitive.
That code path will be fixed in a subsequent commit.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99828 Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Reviewed-by: Philip Withnall <withnall@endlessm.com>
dbus-daemon[26876]: Activating service name='org.freedesktop.DBus.TestSuiteEchoService' requested by ':1.2415' (uid=1000 pid=26876 comm=".../bus/.libs/test-bus ")
dbus-daemon[26876]: Failed to activate service 'org.freedesktop.DBus.TestSuiteEchoService': timed out (service_start_timeout=25000ms)
dbus-daemon[26876]: Did not expect error org.freedesktop.DBus.Error.TimedOut
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Simon McVittie [Mon, 13 Feb 2017 17:01:04 +0000 (17:01 +0000)]
dbus-object-tree test: tolerate OOM in run_decompose_tests()
This test is run repeatedly, with simulated out-of-memory conditions
at different points. If one of these was during run_decompose_tests(),
the test was recorded as failing. Before Philip fixed it, this was
masked by the failure not being reported correctly (CID: #54711).
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99758 Reviewed-by: Philip Withnall <withnall@endlessm.com>
Ralf Habacker [Mon, 13 Feb 2017 19:21:09 +0000 (20:21 +0100)]
cmake: use default GNU installation layout.
cmake provides a macro named GnuInstallDirs to let install locations
be compatible with GNU's install location layout on several plattforms.
Using that layout makes cmake installs be more compatible to what
autotools use and also supports 32 and 64 bit installations out of the box.
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99721 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Philip Withnall [Mon, 13 Feb 2017 12:55:40 +0000 (12:55 +0000)]
dbus-hash: Fix memory leaks in internal hash table tests
This includes fixing a memory leak in _dbus_hash_iter_lookup(), which is
not one of the unit tests; but it is only ever called from the unit
tests, so this is not a user-facing leak.
Coverity IDs: 54730, 54740 Signed-off-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99793 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Philip Withnall [Fri, 10 Feb 2017 11:21:02 +0000 (11:21 +0000)]
dbus-send: Handle OOM failures by aborting
This is a fairly pointless feature to add, since the current behaviour
was to abort due to a NULL pointer dereference shortly after the OOM
failure. At least now people will get a helpful error message when they
try to use dbus-send on a machine with incurable memory pressure.
Coverity ID: 54710 Signed-off-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99759 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Philip Withnall [Thu, 9 Feb 2017 09:40:51 +0000 (09:40 +0000)]
test: Simplify error checking path in test-service slightly
This should shut up Coverity from complaining about not checking the
return value of dbus_set_error_from_message(), which is equivalent to
the (type == DBUS_MESSAGE_TYPE_ERROR) check.
Coverity ID: 54697 Signed-off-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99723 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Philip Withnall [Thu, 2 Feb 2017 10:14:55 +0000 (10:14 +0000)]
dbus-hash: Fix a potential shift by a negative integer
As a hash table becomes unbelievably large and full, the down_shift
tends towards 0. The overflow detection code in rebuild_table() does not
prevent down_shift becoming negative, which then causes undefined
behaviour in RANDOM_INDEX for int-keyed tables.
Note that this can only happen with approaching INT_MAX entries in the
hash table, at which point we’ve almost certainly hit OOM somewhere, so
this is vanishingly unlikely to happen. This is why I can’t add a test
for the bug.
As always, thanks to Coverity.
Coverity ID: 54682
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99641 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Philip Withnall [Wed, 8 Feb 2017 10:09:04 +0000 (10:09 +0000)]
test: Add some OOM paths to the dbus-daemon-eavesdrop path
This will shut Coverity up. All the paths are checks on the return value
from dbus_message_set_destination().
Coverity IDs: 54822, 54823, 54824, 54825
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99712 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Philip Withnall [Mon, 6 Feb 2017 13:07:21 +0000 (13:07 +0000)]
test: Add missing error handling to some name-test tests
Also take the opportunity to tweak the test-threads-init messages
slightly to make it more TAP-compliant. It is not entirely TAP compliant
because it doesn’t print a test plan before starting its tests.
Coverity IDs: 54701, 54714, 54726
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99694 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Philip Withnall [Mon, 6 Feb 2017 13:02:22 +0000 (13:02 +0000)]
tools: Improve argument validation in dbus-spam
Check that at most one argument which sets the payload is provided, so
the allocated payload is not overwritten and leaked.
Coverity ID: 54759
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=99693 Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>